fix(272): probe weak/missing tool output schemas, isolate probe blocks (Stage 1+2)#362
fix(272): probe weak/missing tool output schemas, isolate probe blocks (Stage 1+2)#362haroldship wants to merge 13 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
prepare_node.py was filtering for weak-schema tools over tools_for_prompt, which collapses to just the find_tools meta-tool whenever the catalog exceeds shortlisting_tool_threshold (~prepare_node.py:230) -- the normal case once an app has more than a handful of tools. That made _weak_schema_tool_names permanently empty in any realistic deployment, silently disabling get_tools_needing_probing() (block-isolation enforcement) and _record_weak_schema_shapes (session shape-memory) for every tool actually reachable through find_tools. Only the advisory per-tool directive rendered inside find_tools' own output (which checks the real tool catalog directly, bypassing this cache) was ever active. Switch the source to tools_for_execution, the full catalog unaffected by the find_tools-shortlisting collapse. Add a regression test that exercises the shortlisting-active path (the previous test only covered shortlisting-inactive, where tools_for_prompt == tools_for_execution anyway and couldn't have caught this). Found while running the Vakra(M3) eval at broader scope (10 tasks across 9 domains) for this PR: traces showed the model writing multi-block turns that combined a probe call with code assuming its shape, with all blocks executing together -- exactly what Component B is supposed to prevent.
Broader eval (Vakra M3, 10 tasks across 9 domains, 5 runs each) — and a findingRan this PR's branches at wider scope to validate the single-task numbers above. Results, with
This contradicts the single-task conclusion above ("Adding B is what converts that into actual task success") — at this scope, Root cause: Component B never actually engagesTraced this through the langfuse traces and the code, not just the aggregate numbers. In the Root cause:
Component A (the directive text) survives because Statistical note on the task1-alone result: since B has zero behavioral effect here, the 60% Pass@1 vs main/probing's 0% on task1-alone is sampling noise, not signal — pooling task1 across both this run and the 10-task run gives 4/10 (40%) successes; P(3/5) and P(1/5) under a true rate of 40% are ~23% and ~26% respectively, i.e. both are unremarkable outcomes from the same distribution. n=5 can't distinguish these branches. Fix pushed in d08f6ce: compute |
Re-test after the fix (commit d08f6ce)Re-ran the same 10-task, 5-run comparison on
What I could and couldn't confirmConfirmed: Component C now fires. The "Observed tool output shapes (this session)" note appears in every fixed-run task trajectory I checked, vs. never before the fix. That's an unambiguous, direct signal Not cleanly confirmed either way: whether Component B's truncation fires on a real violation. I tried to verify this by scanning raw Aggregate impact is small and within noise. Pass@1 moved 10.0% → 12.0%; the Cons/pass^5 improvement (0.00→1.00, 0%→10%) is fully explained by Bottom line: the fix is real and necessary (Component C was silently dead before it, confirmed dead via the regression test and confirmed alive in live traces after), but this re-test doesn't provide strong evidence either way on whether Component B's enforcement is now doing useful work — the eval at this scope and sample size isn't sensitive enough to tell, and trace-based verification of B specifically needs a more direct method than what I used here. |
|
Reopened as #417 — GitHub couldn't reopen this PR directly because the branch head was force-moved after closing (recorded head went stale). Same |
Summary
Implements a staged fix for #272 (CugaLite struggles with weak/missing tool output schemas), combining elements of candidate solutions #2, #3, and #5 from the original design deck (attached on the issue) into the smallest change set that closes the gap.
Component A — weak-schema detection + per-tool probing directive
Detects tools with no real declared output schema (both the genuinely-empty OpenAPI case and MCP's synthetic
{"success": {"type": "string"}}placeholder) and replaces their rendered schema block with a directive telling the model to call the tool alone andprint()the raw result before writing code that assumes its shape.Component C — post-probe session enrichment
Captures the first observed shape of a weak-schema tool's result per session and surfaces it back into the prompt on every subsequent turn via the existing per-turn
prepare_system_contenthook, so the model stops re-probing a tool it has already seen.Component B — structural enforcement of the block split
A+C alone are advisory: nothing stopped the runner from combining a probe call and a slicing call written in the same completion into one executed script, which is the literal mechanism behind the issue's failure trace. B adds a
CoreGraphAdapter.get_tools_needing_probing()hook and truncates a multi-block response after the first block that references a still-unobserved weak-schema tool, discarding the rest so the model is forced to see the real result before writing dependent code. CugaSupervisor is unaffected (hook defaults tofrozenset()).Full design rationale:
docs/superpowers/specs/2026-06-23-weak-tool-schema-probing-design.mdImplementation plans:
docs/superpowers/plans/2026-06-23-weak-tool-schema-probing-stage1.md,docs/superpowers/plans/2026-06-23-weak-tool-schema-probing-stage2.mdStaging
This PR is the union of two branches, kept separate during development so each component's contribution could be evaluated independently:
feat/272-tool-output-schema-probing— Stage 1, Components A+C onlyfeat/272-stage2-block-isolation(this branch, off Stage 1) — adds Component BEval results (Vakra M3, 1 task, 5 runs each)
A+C alone already cuts tokens/calls/time roughly in half and raises ExactMatch substantially, but doesn't move Pass@1 — consistent with the design doc's prediction that advisory-only probing wouldn't close the gap on its own. Adding B (structural enforcement) is what converts that into actual task success.
Test plan
prompt_utils,prepare_node,sandbox_node,graph_adapter) — new + extendedcode_extraction,graph_nodes,shared_nodes,graph_adapter) — new + extended, including an end-to-end reproduction of the issue's literal original multi-block tracecuga_lite+cuga_agent_core+cuga_supervisorsuites green (363 passed, 1 skipped, 3 pre-existing unrelated e2b-package-not-installed failures)ruff checkclean