Harden Grok review output handling - #40
Conversation
📝 WalkthroughWalkthroughGrok review candidates and verification responses now use permissive wire schemas and normalization. Verification retries at high effort after medium-effort failure, reports failure details, and tracks actual high-effort usage. ChangesGrok review resilience
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReviewSwarm
participant Verification
participant GrokAgent
ReviewSwarm->>Verification: Run medium-effort verification
Verification->>GrokAgent: Request structured response
GrokAgent-->>Verification: Result or GrokReviewError
Verification-->>ReviewSwarm: Normalized verification
ReviewSwarm->>Verification: Retry at high effort after failure
Verification->>GrokAgent: Request high-effort response
GrokAgent-->>Verification: High-effort result
Verification-->>ReviewSwarm: Normalized fallback result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:7e10c7f86a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .flatMap((finding) => { | ||
| const normalized = normalizeCandidateFinding(finding); | ||
| return normalized ? [normalized] : []; | ||
| }) | ||
| .slice(0, MAX_REVIEW_FINDINGS); |
There was a problem hiding this comment.
When a verifier returns a finding with a blank required text field, this filter silently removes it; the same helper also truncates outputs beyond 20 findings. Because the downstream partial calculation does not track either case, a response whose only finding is malformed can be reported as pass, and the later “Only the first 20...” limitation is unreachable. Preserve whether normalization discarded findings and use it to mark the report partial and disclose the omission.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@apps/server/src/review/GrokReviewModel.ts`:
- Around line 219-235: Add focused tests for normalizeGrokReviewCandidate and
normalizeGrokReviewVerification that decode their outputs through the canonical
GrokReviewCandidate and GrokReviewVerification schemas. Use adversarial wire
inputs covering oversized text, null line fields, out-of-range confidence, and
oversized escalationReason, asserting the normalized results satisfy every
schema bound and filter.
- Line 65: Update the needsHighEffortReview schema definition to accept both
boolean and null values, matching the nullability of other optional wire fields.
Keep normalizeGrokReviewVerification unchanged because its existing nullish
fallback already converts null to false.
In `@apps/server/src/review/GrokReviewSwarm.test.ts`:
- Around line 393-441: Add focused tests alongside the existing “retries failed
medium verification at high effort” case in the GrokReviewSwarm test suite.
Cover both verification attempts failing and assert a GrokReviewError with
operation “GrokReviewSwarm.verify”; also cover successful medium verification
followed by failed high-effort escalation, asserting a partial report with
escalatedToHigh false and the limitation “High-effort escalation failed; the
medium-effort verification is shown.”
- Around line 381-387: Update the assertions in the report normalization test to
import and use the exported MAX_REVIEW_* bound constants instead of hard-coded
2_000, 8, and 240 values. Replace the summary length, coverage length, and
finding title length literals while preserving the existing assertions.
In `@apps/server/src/review/GrokReviewSwarm.ts`:
- Around line 262-332: Replace each repeated Effect.map plus Effect.catch
wrapper around runVerification in the medium and high-effort verification paths
with Effect.result. Preserve the existing tagged success/failure branching,
error propagation, verification assignment, and high-effort status flags while
using the result produced by Effect.result.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: b9d43dfd-4fdf-41cd-99b8-471a8175cf5d
📒 Files selected for processing (3)
apps/server/src/review/GrokReviewModel.tsapps/server/src/review/GrokReviewSwarm.test.tsapps/server/src/review/GrokReviewSwarm.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:fa089c4e02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Summary
Root cause
The first production review reached the verifier, but Grok output violated the strict presentation schema. That turned a useful run into HTTP 500 even though the durable queue and retry path were healthy.
Verification
pnpm exec vp test run apps/server/src/review/GrokReviewSwarm.test.ts apps/server/src/review/GrokReviewAgent.test.ts— 25 passedT3_GROK_REVIEW_SWARM_PROBE=1 pnpm exec vp test run apps/server/src/review/GrokReviewSwarmProbe.test.ts— real Grok 4.5 swarm passedpnpm exec vp run --filter t3 typecheck— passed (existing Effect suggestions only)vp linton the three changed files — passedpnpm build— passedgit diff --check— passedNo frontend behavior changed.