feat(stage-router): Extend retrieval cmd identification - #906
grahamking wants to merge 2 commits into
Conversation
- Stricter shell parsing to identify file reads. We identify an error by looking for strings such `MemoryError` in tool output. If that string appears in a log file we read for example, then it should not count. Previously we skipped it correctly if the output was Claude and Pi "Read" tools, but Codex does everything with bash which we did not skip. Now we parse that and categorize if it is a read. On the DeepSWE runs I checked, this reduces capable model use from 19% of requests to 15%. The classification takes ~265 ns. - Missing file reads are not failures but normal exploration. They occur at the same rate (~38%) in successful and failed tasks. Applies to both Codex and Claude. - Empty text blocks occupy no slots. We look at the last three tool calls. Previously a tool with no output (e.g. `mkdir`) didn't occupy one of those three slots, but two together did because we join on `\n`. That caused errors to age out even though we don't have any new evidence. Now empty blocks are skipped. The shell parsing was built by extracting commands from three DeepSWE runs (one Luna, two Sol). It was prototyped in Python before porting to Rust. Both Astra xhigh and Opus 5.5 xhigh worked on it. I think it is remarkably readable for what it is. There are more tests than usual because of this. Assisted-by: Pi:GPT 6 Astra xhigh Assisted-by: Claude:Opus 5.5 xhigh Signed-off-by: Graham King <grahamk@nvidia.com>
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA-NeMo/Switchyard/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughTool-result extraction now recognizes built-in read tools, editor ChangesTool retrieval signal handling
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This change refines how tool-error signals are detected for file-read commands. It only affects heuristic routing signals. No concrete defects were identified, so the change appears ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 1 files. (1 skipped: 1 unsupported.)
A rabbit checks commands with care, Comment |
|
Great contribution!
|
Thanks Tina! Signed-off-by: Graham King <grahamk@nvidia.com>
Done! |
This branch has not been deployed
2 participants
We identify an error by looking for strings such
MemoryErrorin tooloutput. If that string appears in a log file we read for example, then
it should not count.
Previously we skipped it correctly if the output was Claude and Pi "Read" tools,
but Codex does everything with bash which we did not skip.
Now we parse that and categorize if it is a read.
On the DeepSWE runs I checked, this reduces capable model use from 19% of
requests to 15%. The classification takes ~265 ns.
They occur at the same rate (~38%) in successful and failed tasks.
Applies to both Codex and Claude.
We look at the last three tool calls. Previously a tool with no output
(e.g.
mkdir) didn't occupy one of those three slots, but two togetherdid because we join on
\n. That caused errors to age out even thoughwe don't have any new evidence. Now empty blocks are skipped.
The shell parsing was built by extracting commands from three DeepSWE
runs (one Luna, two Sol). It was prototyped in Python before porting
to Rust. Both Astra xhigh and Opus 5.5 xhigh worked on it. I think it is
remarkably readable for what it is.
There are more tests than usual because of this.
Assisted-by: Pi:GPT 6 Astra xhigh
Assisted-by: Claude:Opus 5.5 xhigh
Signed-off-by: Graham King grahamk@nvidia.com
Summary by CodeRabbit