fix(judge): retry unusable json mode responses - #220
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe OpenAI-compatible completion path validates response structure and JSON acceptability. It retries once without JSON mode when validation fails. The judge rejects ChangesJSON retry flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Judge
participant OpenAICompatibleClient
participant Provider
Judge->>OpenAICompatibleClient: Request JSON with validator
OpenAICompatibleClient->>Provider: Send request with JSON mode
Provider-->>OpenAICompatibleClient: Return completion response
OpenAICompatibleClient->>OpenAICompatibleClient: Validate response and JSON
OpenAICompatibleClient->>Provider: Retry without JSON mode
Provider-->>OpenAICompatibleClient: Return fallback response
OpenAICompatibleClient-->>Judge: Return extracted JSON
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Hi @adity982, appreciate you contributing to Agent Opfor. |
|
Thanks for checking in. The implementation and full local/CI validation are complete, so I have marked the PR ready for review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@core/src/llm/openaiCompatible.ts`:
- Around line 157-159: The empty-content error thrown in the OpenAI-compatible
response handling must tell users how to resolve the issue. Update the error
message in the content validation block to instruct them to configure the
provider or prompt to return a non-empty JSON completion.
- Around line 153-156: In core/src/llm/openaiCompatible.ts at lines 153-156,
replace the unsafe type assertion on response.json() with proper Zod validation
by first parsing the response as unknown, defining a Zod schema that describes
the expected structure with choices array containing message objects with
content, validating the parsed data against the schema, and throwing an
actionable error that includes details when validation fails. In
core/tests/openaiCompatible.test.ts at line 26, ensure the mocked request body
is similarly parsed as unknown and validated with the same Zod schema to
maintain consistency between test data and production validation logic.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dc1ccdf7-b552-40db-9464-d52069af0c43
📒 Files selected for processing (3)
core/src/llm/openaiCompatible.tscore/src/run/judge.tscore/tests/openaiCompatible.test.ts
|
Can you resolve the conflicts and address Coderabbit's review comments @adity982 |
|
Refreshed this branch against current upstream master and resolved the overlap with the new token-accounting work. The JSON-mode retry behavior is preserved while TokenTracker/parseUsage are now wired through the same OpenAI-compatible and judge paths.\n\nValidation after resolution:\n- core test suite: 185 passed, 3 skipped, 0 failed\n- core TypeScript typecheck passed\n- full pre-commit checks passed: workspace typechecks, ESLint, Prettier, catalog builds, skill validation, and gitleaks\n\nUpdated head: |
|
Addressed both new CodeRabbit findings in 8783b2e: OpenAI-compatible provider responses are now Zod-validated with an actionable malformed-response error, and empty content now includes remediation guidance. Validation: core suite 186 passed / 3 skipped / 0 failed, core typecheck and Prettier passed, and the repository pre-commit gate passed. Both threads are resolved. |
- Loosen the response schema so provider-supplied usage:null and
content:null (both legal on the wire) can't fail an otherwise
usable completion; the existing emptiness check already handles
null content correctly.
- Route the JSON-mode fallback request through the same 400/429
recovery path as the first attempt, so a rate-limited retry no
longer throws away the fix this PR is making.
- Retry once when JSON-mode content fails to parse as JSON, not
just when it round-trips to {}.
- Log the fallback so double LLM spend is diagnosable.
- Add regression coverage for each of the above.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@core/tests/openaiCompatible.test.ts`:
- Line 150: Update the test cases in the usage loop of textCompletion to include
undefined, covering the absent usage field alongside null and the partial usage
object.
- Line 45: Update the assertion message in the response-queue check to state the
corrective action: queue one response for every expected fetch call, while
retaining the existing context about excess fetches.
- Line 43: In the line where bodies.push() is called with JSON.parse, refactor
to parse the init?.body to unknown first instead of directly casting. Then
validate the parsed unknown value using a Zod schema such as z.record to ensure
it conforms to the expected structure. Only push the validated result to the
bodies array after the Zod validation succeeds. This separates the parsing step
from the type validation and ensures type safety without unvalidated casts.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b8ccdd76-bba3-4d2b-80f1-4eff2062c110
📒 Files selected for processing (3)
core/src/llm/openaiCompatible.tscore/src/run/judge.tscore/tests/openaiCompatible.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- core/src/run/judge.ts
- core/src/llm/openaiCompatible.ts
Address CodeRabbit nits on the openaiCompatible fetch stub: parse the captured request body through a Zod schema instead of an unvalidated cast, make the queue-exhaustion assertion state the fix, and add the undefined case so "omits usage telemetry" actually exercises a body with no usage key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 participants
Summary
{}, retry exactly once withoutresponse_formatValidation
Closes #165
Summary by CodeRabbit