SCOUT->FORGE: 7 verified H21 tools + bridge/panel hardening#4
Merged
Conversation
4 new MCP tools + 2 schema extensions + 1 render-walk fix, each V1-verified on Houdini 21.0.671 and unit-tested standalone (50 new tests; behavioral checks deferred until the live bridge is up): - houdini_set_payload_loadstate (C2): USD payload load/unload + activation - houdini_create_point_instancer (C5b): UsdGeom.PointInstancer authoring - houdini_shot_render_ready (C6): shot-template composite orchestrator - cops_create_copnet (C4): modern Copernicus copnet, distinct from legacy cop2net - houdini_reference_usd +karma_visible/purpose/kind (C1): non-clobbering Karma visibility metadata on import (completes the BL-008 advisory-only partial) - houdini_modify_usd_prim +instanceable (C5a) - handlers_render upstream Karma-LOP walk now branch-aware + path-keyed (C3) Registry 104 -> 108 tools. Verified: 50 FORGE + 53 cops tests green, 2924 tests collect cleanly. Produced by a SCOUT recon + FORGE MOE build run (.scout / .forge run logs, gitignored). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…1/3 mcp_protocol tests)
Bundled bridge-layer fixes from forge-bridge-fix. CRUCIBLE cleared: no
showstopper, production consent gating preserved.
- server.py (dispatch fix): the read-only fast-path of
MCPServer._handle_tools_call returned dispatch_tool's result directly,
so a handler failure leaked through as a JSON-RPC success result with
{isError:True} buried inside instead of a top-level JSON-RPC error. The
isError->JsonRpcError conversion existed only in the non-read-only
branch. Added the same conversion to the read-only branch and extracted
the shared message-extraction into a module-level _isError_text() helper
(byte-identical logic; no behavior change on the non-read-only path).
Greens tests/test_mcp_protocol.py::TestToolsCall::
test_tool_error_propagated_as_jsonrpc_error (synapse_ping is read_only).
- bridge_adapter.py (id fix): execute_through_bridge() omitted
id=command.id on the failure-path SynapseResponse, so bridge-rejected
commands returned a response with no correlation id. Added id=command.id
to the failure branch to match the success path.
- consent finding (NO CHANGE — test-is-wrong, production gating untouched):
TestSafeRenderMCPTools::test_safe_render_dispatch and
::test_render_progressively_dispatch remain red. In this repo's test env
synapse.core.gates IS importable, so bridge.py sets _GATES_AVAILABLE=True
and _check_consent takes Path 1 (production HumanGate). APPROVE-gated
submit_render correctly times out to safe-default rejection because no
artist decides -> "Bridge: Operation requires approve consent". The
standalone auto-approve tier (Path 3) fires only when _gate is None AND
_consent_callback is None, which is unreachable here. Any edit that
flips these two tests green would alter Path 1 production APPROVE/CRITICAL
gating, forbidden by the hard constraint. Bridge consent logic is correct
as written; zero edits made to shared/bridge.py. The fix belongs in the
test/dispatch layer (inject a consent_callback, null the bridge_adapter
singleton, or supply a HumanGate auto-decision in the fixture) and is
noted separately for the test owner.
Honest outcome: 1 of 3 target tests green. The 2 consent tests are
expected-red (test expectation contradicts the gating architecture), not
regressions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_safe_render_dispatch / test_render_progressively_dispatch verify dispatch of non-read-only tools, which route through the LosslessExecutionBridge. In CI synapse.core.gates is importable, so the bridge auto-wires a real HumanGate and _check_consent takes the gate path (Path 1) before the injected callback (Path 2) -- an unattended APPROVE-gated op then times out to safe-default rejection (~240s). Add an _auto_approve_bridge fixture that swaps in a process-local bridge with the gate nulled + an auto-approving consent_callback, so the tests exercise dispatch (their intent) rather than the production gate. Production consent gating is untouched -- shared/bridge.py is unchanged. test_mcp_protocol.py: 91->93 passing, 240s->0.4s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rror
The panel's ClaudeWorker (and the panel's text-only fallback worker) read the
key straight from os.environ and failed with a cryptic "ANTHROPIC_API_KEY not
set in environment" when empty -- missing keys stored via hou.secure and
offering the user no guidance.
Resolve through host.auth.get_anthropic_api_key() (hou.secure -> env var,
whitespace-stripped, never raises) and emit an actionable message: set
ANTHROPIC_API_KEY at the SYSTEM level and relaunch Houdini (a terminal-scoped
`set` does not propagate to Houdini on Windows), or use
hou.secure.setPassword('synapse_anthropic', ...).
Root cause of the reported failure was an unset persistent key (Houdini
inherits only User/Machine env vars); this commit makes the failure
self-service rather than cryptic and honors keys stored in hou.secure.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 43 minutes and 22 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR extends Synapse's FORGE command suite with four new capabilities (USD payload state management, PointInstancer creation, multi-step shot render orchestration, and modern Copernicus copnet creation), improves upstream Karma-LOP discovery via breadth-first traversal, consolidates API key retrieval across workers, enhances MCP error handling, and validates all changes through comprehensive test coverage. ChangesFORGE Handler Suite and Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
mcp_tools_cops.py (1)
8-25: 💤 Low valueConsider updating GROUP_KNOWLEDGE to distinguish
cops_create_copnetfromcops_create_network.The GROUP_KNOWLEDGE currently mentions "cops_create_network creates a COP2 container" but doesn't mention the new
cops_create_copnettool. Since the tool registry describescops_create_copnetas creating a "modern Copernicus 'copnet' network (H21 Copernicus, distinct from legacy cop2net)", agents consuming this knowledge would benefit from understanding when to use each tool.📝 Suggested GROUP_KNOWLEDGE update
GROUP_KNOWLEDGE = ( "COPERNICUS (COPs) TOOLS: GPU-accelerated image processing in Houdini 21. " - "FOUNDATION: cops_create_network creates a COP2 container, cops_create_node " + "FOUNDATION: cops_create_network creates a legacy COP2 container, " + "cops_create_copnet creates a modern H21 copnet container, cops_create_node " "adds COP nodes, cops_connect wires them, cops_set_opencl sets GPU kernels, "🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp_tools_cops.py` around lines 8 - 25, Update the GROUP_KNOWLEDGE string to explicitly mention both cops_create_network and the new cops_create_copnet, clarifying that cops_create_network creates a legacy COP2 container (cop2net) while cops_create_copnet creates the modern Copernicus "copnet" network (H21 Copernicus) and note when to prefer each (legacy compatibility vs modern rendering/features); keep existing mentions of other functions (e.g., cops_create_node, cops_connect, cops_set_opencl, cops_read_layer_info, cops_to_materialx, cops_composite_aovs, cops_analyze_render, cops_slap_comp, cops_create_solver, cops_procedural_texture, cops_growth_propagation, cops_reaction_diffusion, cops_pixel_sort, cops_stylize, cops_wetmap, cops_bake_textures, cops_temporal_analysis, cops_stamp_scatter, cops_batch_cook) and preserve the note that all mutations wrap in hou.undos.group for safe rollback so consumers know the transactional behavior.houdini/python_panels/synapse_panel.pypanel (1)
208-212: 💤 Low valueConsider matching the error message from
claude_worker.py.The error message here is shorter and omits the
hou.securealternative mentioned inclaude_worker.py(lines 105-106). Since both workers serve the same panel context, users would benefit from consistent, complete guidance including thehou.secure.setPassword('synapse_anthropic', 'sk-ant-...')option.📝 Align error messages for consistency
self.stream_error.emit( - 'No Anthropic API key found. Set it at the SYSTEM level and ' - 'relaunch Houdini: setx ANTHROPIC_API_KEY "sk-ant-..." ' - "(a terminal-scoped `set` won't carry into Houdini on Windows)." + "No Anthropic API key found. Set it at the SYSTEM level so " + "Houdini inherits it, then relaunch Houdini: " + 'setx ANTHROPIC_API_KEY "sk-ant-..." ' + "(a terminal-scoped `set` won't carry into Houdini on Windows). " + "On builds exposing hou.secure you can instead run, in Houdini's " + "Python shell: hou.secure.setPassword('synapse_anthropic', 'sk-ant-...')." )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@houdini/python_panels/synapse_panel.pypanel` around lines 208 - 212, Update the error string emitted by self.stream_error.emit in the SynapsePanel (the call shown) to match the more complete guidance used in claude_worker.py: include the hou.secure.setPassword('synapse_anthropic','sk-ant-...') option in addition to the SYSTEM-level setx instruction and the note about Windows terminal-scoped set not carrying into Houdini, so users get consistent, complete instructions across workers.tests/test_forge_usd.py (2)
265-278: ⚡ Quick winPrefix unused unpacked variables with underscore.
The unpacked variables
ref_lopandkv_lopare never used in this test method. Prefix them with_to indicate they are intentionally unused.♻️ Suggested fix
- parent_node, ref_lop, kv_lop = self._setup_parent() + parent_node, _ref_lop, _kv_lop = self._setup_parent()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_forge_usd.py` around lines 265 - 278, In test_payload_mode_default_karma_visible, the unpacked variables ref_lop and kv_lop from the call to self._setup_parent() are unused; rename or prefix them with an underscore (e.g., _ref_lop, _kv_lop) so the test signals intentional unused variables and avoids lint warnings when running test_payload_mode_default_karma_visible and other tests using _setup_parent.
249-264: ⚡ Quick winPrefix unused unpacked variables with underscore.
The unpacked variables
ref_lopandkv_lopare never used in this test method. Prefix them with_to indicate they are intentionally unused.♻️ Suggested fix
- parent_node, ref_lop, kv_lop = self._setup_parent() + parent_node, _ref_lop, _kv_lop = self._setup_parent()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_forge_usd.py` around lines 249 - 264, In the test method test_karma_visible_string_false_keeps_advisory, the unpacked variables ref_lop and kv_lop returned from self._setup_parent() are unused; rename them to _ref_lop and _kv_lop (i.e., change "parent_node, ref_lop, kv_lop = self._setup_parent()" to "parent_node, _ref_lop, _kv_lop = self._setup_parent()") to signal intentional unused variables and satisfy the lint rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/synapse/mcp/_tool_registry.py`:
- Around line 545-557: The schema for the "houdini_create_point_instancer" /
"create_point_instancer" action marks "prototypes" and "positions" as optional
but the description implies they will be set; inspect the handler function
_handle_create_point_instancer to determine actual behavior (does it create an
empty instancer, apply protoIndices defaults, or fill positions) and then make
the schema and description consistent: either require "prototypes" and
"positions" in the schema if the handler needs them, or update the description
to state that these fields are optional and describe the
defaults/empty-instancer behavior that the handler implements (and if necessary,
modify _handle_create_point_instancer to apply sensible defaults such as empty
prototypes and zeroed protoIndices when fields are missing).
---
Nitpick comments:
In `@houdini/python_panels/synapse_panel.pypanel`:
- Around line 208-212: Update the error string emitted by self.stream_error.emit
in the SynapsePanel (the call shown) to match the more complete guidance used in
claude_worker.py: include the
hou.secure.setPassword('synapse_anthropic','sk-ant-...') option in addition to
the SYSTEM-level setx instruction and the note about Windows terminal-scoped set
not carrying into Houdini, so users get consistent, complete instructions across
workers.
In `@mcp_tools_cops.py`:
- Around line 8-25: Update the GROUP_KNOWLEDGE string to explicitly mention both
cops_create_network and the new cops_create_copnet, clarifying that
cops_create_network creates a legacy COP2 container (cop2net) while
cops_create_copnet creates the modern Copernicus "copnet" network (H21
Copernicus) and note when to prefer each (legacy compatibility vs modern
rendering/features); keep existing mentions of other functions (e.g.,
cops_create_node, cops_connect, cops_set_opencl, cops_read_layer_info,
cops_to_materialx, cops_composite_aovs, cops_analyze_render, cops_slap_comp,
cops_create_solver, cops_procedural_texture, cops_growth_propagation,
cops_reaction_diffusion, cops_pixel_sort, cops_stylize, cops_wetmap,
cops_bake_textures, cops_temporal_analysis, cops_stamp_scatter, cops_batch_cook)
and preserve the note that all mutations wrap in hou.undos.group for safe
rollback so consumers know the transactional behavior.
In `@tests/test_forge_usd.py`:
- Around line 265-278: In test_payload_mode_default_karma_visible, the unpacked
variables ref_lop and kv_lop from the call to self._setup_parent() are unused;
rename or prefix them with an underscore (e.g., _ref_lop, _kv_lop) so the test
signals intentional unused variables and avoids lint warnings when running
test_payload_mode_default_karma_visible and other tests using _setup_parent.
- Around line 249-264: In the test method
test_karma_visible_string_false_keeps_advisory, the unpacked variables ref_lop
and kv_lop returned from self._setup_parent() are unused; rename them to
_ref_lop and _kv_lop (i.e., change "parent_node, ref_lop, kv_lop =
self._setup_parent()" to "parent_node, _ref_lop, _kv_lop =
self._setup_parent()") to signal intentional unused variables and satisfy the
lint rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 260cd177-7514-48d9-8aa3-328b76af2edc
📒 Files selected for processing (18)
.gitignorehoudini/python_panels/synapse_panel.pypanelmcp_tools_cops.pymcp_tools_usd.pypython/synapse/core/protocol.pypython/synapse/mcp/_tool_registry.pypython/synapse/mcp/server.pypython/synapse/panel/bridge_adapter.pypython/synapse/panel/claude_worker.pypython/synapse/server/handlers.pypython/synapse/server/handlers_cops.pypython/synapse/server/handlers_render.pypython/synapse/server/handlers_usd.pytests/test_cops.pytests/test_forge_copernicus.pytests/test_forge_render.pytests/test_forge_usd.pytests/test_mcp_protocol.py
Comment on lines
+545
to
+557
| ("houdini_create_point_instancer", "create_point_instancer", _identity, | ||
| "Author a UsdGeom.PointInstancer: scatter prototype prims across positions. " | ||
| "Minimal valid setup -- defines the instancer, sets the prototypes relationship, " | ||
| "protoIndices (defaults to zeros), and positions.", | ||
| {"type": "object", "properties": { | ||
| "node": {"type": "string", "description": "LOP node to wire after (optional)"}, | ||
| "prim_path": {"type": "string", "description": "USD prim path for the PointInstancer"}, | ||
| "prototypes": {"type": "array", "items": {"type": "string"}, | ||
| "description": "Prototype prim paths to instance"}, | ||
| "positions": {"type": "array", "items": {"type": "array", "items": {"type": "number"}}, | ||
| "description": "Instance positions as [[x,y,z], ...]"}, | ||
| }, "required": ["prim_path"]}, | ||
| False, True, False), |
There was a problem hiding this comment.
Verify that optional prototypes and positions is intentional.
The description states "Minimal valid setup -- defines the instancer, sets the prototypes relationship, protoIndices (defaults to zeros), and positions", which implies these fields will be set. However, the schema makes both prototypes and positions optional (only prim_path is required).
Clarify what happens when these fields are not provided:
- Does the handler create an empty point instancer?
- Are there sensible defaults applied?
- Should the description be updated to reflect that these are optional?
Run the following to verify the handler implementation:
#!/bin/bash
# Search for the create_point_instancer handler implementation
rg -n -A 20 'def _handle_create_point_instancer' --type=py🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/synapse/mcp/_tool_registry.py` around lines 545 - 557, The schema for
the "houdini_create_point_instancer" / "create_point_instancer" action marks
"prototypes" and "positions" as optional but the description implies they will
be set; inspect the handler function _handle_create_point_instancer to determine
actual behavior (does it create an empty instancer, apply protoIndices defaults,
or fill positions) and then make the schema and description consistent: either
require "prototypes" and "positions" in the schema if the handler needs them, or
update the description to state that these fields are optional and describe the
defaults/empty-instancer behavior that the handler implements (and if necessary,
modify _handle_create_point_instancer to apply sensible defaults such as empty
prototypes and zeroed protoIndices when fields are missing).
- README: registry tool count 104 -> 108; new "v5.9.0 - SCOUT -> FORGE" section with an on-point pipeline mermaid diagram and the 7 shipped capabilities; setup docs now point to the set_anthropic_key.bat helper. - Bump __version__ 5.8.0 -> 5.9.0. - Add set_anthropic_key.bat: prompts for the key, persists it with setx, and reminds you to relaunch Houdini -- removes the system-vs-shell-scope gotcha. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements all 7 opportunities surfaced by a read-only SCOUT recon of the Houdini 21.0.671 capability surface vs the SYNAPSE tool registry, plus adjacent bridge/panel robustness fixes found along the way. Every capability was V1-verified against the exact target build (21.0.671 hython) before any code was written; all changes are unit-tested standalone.
What's here
Tools / capabilities (
5287006) — registry 104 → 108:houdini_set_payload_loadstate— USD payload load/unload + activationhoudini_create_point_instancer—UsdGeom.PointInstancerauthoringhoudini_shot_render_ready— shot-template composite orchestratorcops_create_copnet— modern Copernicuscopnet(distinct from the legacycop2netthe 20 existing COPs tools build on)houdini_reference_usd+karma_visible/purpose/kind— non-clobbering Karma-visibility metadata on import (completes the BL-008 advisory-only partial)houdini_modify_usd_prim+instanceablehandlers_renderupstream Karma-LOP walk is now branch-aware + path-keyed (C3)Bridge / panel hardening:
ea32f79) read-only tool failures now surface as JSON-RPC errors instead of success-with-isError;SynapseResponseidfix on the bridge-failure path76d8fca) SafeRender dispatch tests exercise the standalone consent tier (a realHumanGateis wired in CI) — no production-gating changebb5819a) panel Anthropic-key resolution routed throughhost/auth.py(hou.secure→ env var) with an actionable "set it + relaunch Houdini" error messageVerification
test_cops.py: 53.test_mcp_protocol.py: 93/93 (was 91/2, and 240s → 0.4s). Full suite collects 2924 tests, 0 import errors.shared/bridge.pyhas zero diff.Deferred (live bridge was offline during the build)
Behavioral checks to run on a live 21.0.671 session: Karma cook of
copnet, EXR landing for the render path, and USD editableStage round-trips for the metadata / load-state / instancer tools.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
copnetnetwork creation toolBug Fixes
Tests