Skip to content

fix(272): probe weak/missing tool output schemas, isolate probe blocks (Stage 1+2)#362

Closed
haroldship wants to merge 13 commits into
mainfrom
feat/272-stage2-block-isolation
Closed

fix(272): probe weak/missing tool output schemas, isolate probe blocks (Stage 1+2)#362
haroldship wants to merge 13 commits into
mainfrom
feat/272-stage2-block-isolation

Conversation

@haroldship

Copy link
Copy Markdown
Collaborator

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 and print() 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_content hook, 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 to frozenset()).

Full design rationale: docs/superpowers/specs/2026-06-23-weak-tool-schema-probing-design.md
Implementation plans: docs/superpowers/plans/2026-06-23-weak-tool-schema-probing-stage1.md, docs/superpowers/plans/2026-06-23-weak-tool-schema-probing-stage2.md

Staging

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 only
  • feat/272-stage2-block-isolation (this branch, off Stage 1) — adds Component B

Eval results (Vakra M3, 1 task, 5 runs each)

Branch Commit Pass@1 ExactMatch Avg tokens Avg LLM calls Avg time
main 8d3d359 0.0% 0.20 301,614 15.2 65.2s
feat/272-tool-output-schema-probing (A+C) 2dc4773 0.0% 0.80 205,719 11.2 25.2s
feat/272-stage2-block-isolation (A+B+C) 4f62378 60.0% 1.00 214,005 11.0 24.7s

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

  • Stage 1 unit tests (prompt_utils, prepare_node, sandbox_node, graph_adapter) — new + extended
  • Stage 2 unit tests (code_extraction, graph_nodes, shared_nodes, graph_adapter) — new + extended, including an end-to-end reproduction of the issue's literal original multi-block trace
  • Full regression: cuga_lite + cuga_agent_core + cuga_supervisor suites green (363 passed, 1 skipped, 3 pre-existing unrelated e2b-package-not-installed failures)
  • ruff check clean
  • Broader eval run across the full Vakra(M3) task set (this PR's numbers are a single-task, 5-run sample)

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1b65125f-a484-4a75-94e6-310a984bfa25

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/272-stage2-block-isolation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.
@haroldship

Copy link
Copy Markdown
Collaborator Author

Broader eval (Vakra M3, 10 tasks across 9 domains, 5 runs each) — and a finding

Ran this PR's branches at wider scope to validate the single-task numbers above. Results, with pass@5/pass^5/maj@5/Cons added:

Branch Commit Pass@1 pass@5 pass^5 maj@5 Cons Tokens LLM Time
main 8d3d359 16.0% 30.0% 10.0% 10.0% 1.00 3,730,539.6 157.0 758.1s
feat/272-tool-output-schema-probing (A+C) 2dc4773 16.0% 40.0% 0.0% 10.0% 0.00 4,648,273.6 179.6 845.9s
feat/272-stage2-block-isolation (A+B+C) 4f62378 10.0% 20.0% 0.0% 10.0% 0.00 5,048,238.4 190.2 939.2s

This contradicts the single-task conclusion above ("Adding B is what converts that into actual task success") — at this scope, main actually has the best pass^5/Cons (it's the only branch with a task that passes all 5/5 runs), and both feature branches cost more tokens/time while scoring worse on Pass@1 than main.

Root cause: Component B never actually engages

Traced this through the langfuse traces and the code, not just the aggregate numbers. In the feat/272-stage2-block-isolation trace for beer_factory task a823e527d383-5ee3999c12b4 (run 1), the model wrote three fenced blocks in one turn — a probe call, code assuming the probe's result is a string, and a call to a hallucinated tool name — and all three executed together as one script (confirmed: the eventual NameError came from block 3, proving blocks 1–2 had already run). If block-isolation were active, that turn should have truncated after block 1.

Root cause: prepare_node.py:600-603 computes adapter._weak_schema_tool_names by filtering tools_for_prompt. But whenever the tool catalog exceeds shortlisting_tool_threshold (the normal case — every M3 domain here has 100+ tools against threshold=1), tools_for_prompt is collapsed to just [find_tools] at line ~230, before the weak-schema computation runs. So _weak_schema_tool_names is permanently empty in this environment, which cascades:

  • sandbox_node.py:54-56 (_record_weak_schema_shapes) returns immediately on empty weak_schema_tool_names_observed_tool_shapes never populates (Component C dead).
  • graph_adapter.py:111-112 (get_tools_needing_probing) = _weak_schema_tool_names - _observed_tool_shapes.keys() = always frozenset() → Component B's truncation branch in code_extraction.py never executes.

Component A (the directive text) survives because find_tools' own renderer does an independent is_weak_schema_tool check against the real tool catalog at call time, bypassing this broken cache — which is why ExactMatch/tokens/calls still improved on both feature branches even though B and C were inert.

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 _weak_schema_tool_names from tools_for_execution (full catalog, unaffected by the shortlisting collapse) instead of tools_for_prompt, plus a regression test exercising the shortlisting-active path (the existing test only covered shortlisting-inactive, where the two lists are identical and couldn't have caught this). Re-running the 10-task comparison now to see if B actually moves the numbers once it's wired correctly.

@haroldship

Copy link
Copy Markdown
Collaborator Author

Re-test after the fix (commit d08f6ce)

Re-ran the same 10-task, 5-run comparison on feat/272-stage2-block-isolation after the _weak_schema_tool_names fix:

Branch Commit Pass@1 pass@5 pass^5 maj@5 Cons Tokens LLM Time
main 8d3d359 16.0% 30.0% 10.0% 10.0% 1.00 3,730,540 157.0 758.1s
probing (A+C, broken) 2dc4773 16.0% 40.0% 0.0% 10.0% 0.00 4,648,274 179.6 845.9s
stage2 (A+B+C, broken) 4f62378 10.0% 20.0% 0.0% 10.0% 0.00 5,048,238 190.2 939.2s
stage2 (A+B+C, fixed) d08f6ce 12.0% 20.0% 10.0% 10.0% 1.00 4,567,234 181.6 975.3s

What I could and couldn't confirm

Confirmed: 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 _weak_schema_tool_names is correctly populated during real runs, not just in the unit test.

Not cleanly confirmed either way: whether Component B's truncation fires on a real violation. I tried to verify this by scanning raw ChatGroq generations in the langfuse traces for multi-block responses that call a weak-schema tool and then assume its result in the same turn, then checking whether the executed code was truncated to just the probe block. This turned out to be unreliable: some "raw generation" content actually bundles reflection/planning text with illustrative ```python snippets as advice (not code the model intends to run), and at least one instance contained the literal string "Execution output:" embedded in what's labeled as the model's own output — which the model never generates itself, indicating the recorded content sometimes spans multiple historical turns (likely an artifact of the context-summarization feature also visible in these logs), not a single live generation. I don't have a way to cleanly separate genuine single-turn multi-block violations from this artifact in the available trace data, so I'm not reporting a yes/no on Component B's enforcement from trace inspection — that would overstate what I actually verified. A reliable check would need a direct unit/integration test instrumenting `_truncate_after_first_probing_block` against a real adapter from a live registry session, not post-hoc trace archaeology.

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 soccer_2016 going 4/5→5/5, a task unrelated to any weak-schema tool. main is still ahead on Pass@1 (16.0%). Consistent with the statistical point made earlier: n=5 can't resolve effects this small, and this 10-task mix includes several bug classes (tool-name hallucination, missing functionality, input-parameter discovery) that neither Component B nor C address, which dilutes any real effect at the aggregate level.

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.

@haroldship

Copy link
Copy Markdown
Collaborator Author

Reopened as #417 — GitHub couldn't reopen this PR directly because the branch head was force-moved after closing (recorded head went stale). Same feat/272-stage2-block-isolation branch, unchanged content. Continuing review there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants