Fix parser tool inference defaults - #75
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Lost in the diff? Review this PR in Change Stack to follow the change map from intent to exact ranges. Warning Review limit reached
More reviews will be available in 25 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 (2)
📝 WalkthroughWalkthroughThe PR adds automatic tool-type inference to the agent output parser. A new internal helper analyzes the raw transcript for Codex and Claude patterns, then ChangesTool Inference for Agent Output Parser
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/agent-output-parser-fixtures.test.ts (1)
63-93: ⚡ Quick winAdd one mixed-marker regression to lock inference correctness.
These tests cover clean transcripts, but not a Claude transcript whose response text mentions “OpenAI Codex” (or vice versa). That case is exactly where keyword-based false positives can slip in.
Suggested test addition
describe("agent output parser tool inference", () => { @@ it("infers Claude mode when callers omit tool context", () => { const fixture = getParserFixture("claude-real-prompt-that-matches-codex-suggestion"); const parsed = parseAgentOutput(fixture.raw); expect(parsed.parser.tool).toBe("claude"); expect(parsed.blocks.map((block) => block.type)).toEqual(fixture.expected.map((block) => block.type)); }); + it("does not switch to Codex when Claude transcript text mentions OpenAI Codex", () => { + const fixture = getParserFixture("claude-real-prompt-that-matches-codex-suggestion"); + const raw = `${fixture.raw}\n\n⏺ I previously used OpenAI Codex for this workflow.`; + + const parsed = parseAgentOutput(raw); + + expect(parsed.parser.tool).toBe("claude"); + }); + it("preserves explicit non-unknown tool context", () => {🤖 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 `@src/agent-output-parser-fixtures.test.ts` around lines 63 - 93, Add a regression test that exercises parseAgentOutput on a transcript where the speaker is Claude (or codex) but the response text contains the other tool's name (e.g., a Claude transcript that mentions "OpenAI Codex"), to ensure keyword-only detection doesn't flip parser.tool; create a new it(...) in the same describe block (e.g., "does not misinfer tool when response mentions the other tool") that loads a fixture via getParserFixture (a new mixed-marker fixture like "claude-with-codex-mention"), calls parseAgentOutput(fixture.raw) and asserts parsed.parser.tool is the expected tool (and optionally that block types match fixture.expected) to lock inference correctness.
🤖 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 `@src/agent-output-parser.ts`:
- Around line 50-63: The current inferAgentOutputTool function (variables
hasCodexChrome and hasClaudeChrome) matches "OpenAI Codex"/"Claude Code"
anywhere in the transcript, so free-text mentions flip the inferred tool;
tighten the detection by changing those regexes to only match UI chrome/header
occurrences (anchor to start-of-text or start-of-line with optional whitespace
and require header/chrome punctuation or separators such as ":" "|" "—" or
parentheses or other UI chrome markers) instead of arbitrary inline mentions;
update the hasCodexChrome/hasClaudeChrome checks in inferAgentOutputTool to use
those anchored/header-aware patterns so casual conversation no longer triggers
codex/claude inference.
---
Nitpick comments:
In `@src/agent-output-parser-fixtures.test.ts`:
- Around line 63-93: Add a regression test that exercises parseAgentOutput on a
transcript where the speaker is Claude (or codex) but the response text contains
the other tool's name (e.g., a Claude transcript that mentions "OpenAI Codex"),
to ensure keyword-only detection doesn't flip parser.tool; create a new it(...)
in the same describe block (e.g., "does not misinfer tool when response mentions
the other tool") that loads a fixture via getParserFixture (a new mixed-marker
fixture like "claude-with-codex-mention"), calls parseAgentOutput(fixture.raw)
and asserts parsed.parser.tool is the expected tool (and optionally that block
types match fixture.expected) to lock inference correctness.
🪄 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: e83432fe-db00-4415-9b75-5055fbe18645
📒 Files selected for processing (2)
src/agent-output-parser-fixtures.test.tssrc/agent-output-parser.ts
Summary
Verification
Summary by CodeRabbit
New Features
Tests