Repository navigation
fix: parse provider JSON after unmatched prose - #238
goutamadwant wants to merge 1 commit into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs real behavior proof before merge. What this changesThe PR recovers valid provider JSON following unmatched braces in leading prose. Example: The fallback extractor receives
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Blocked before merge - 1 item remains This PR repairs a current provider parsing defect and preserves the existing recovery contract; no actionable code defect was found. Priority: P2 Before merge
FindingsNone. Tests
Agent review detailsHow this fits togetherProvider adapters pass captured response text through the shared JSON extractor, then validate the extracted value before returning review or repair results. flowchart LR
A[CLI command] --> B[Provider adapter]
B --> C[Captured response text]
C --> D[Shared JSON extractor]
D --> E[Schema validation]
E --> F[Workflow result]
Technical reviewBest possible solution: Keep recovery in the shared extractor while preserving strict parsing, balanced-candidate priority, nested-draft exclusion, and downstream schema validation. Do we have a high-confidence way to reproduce the issue? Both reported inputs deterministically reach current main's unmatched-brace termination branch; the PR reports failing-before and passing-after regressions, but no target code was executed during this read-only review. Is this the best way to solve the issue? The shared-owner repair addresses the reported failure while retaining historical candidate-selection safeguards, without adding configuration or a parallel parsing path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 610a26db8437. Provenance checked
TestingProof path: unit tests only. SecurityNone. EvidenceWhat I checked:
LabelsLabel changes:
Label justifications:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 18, 2026, 11:44 PM ET / October 19, 2026, 03:44 UTC. |
Closes #237
What Problem This Solves
Fixes provider result parsing when unmatched braces in leading prose appear before a valid JSON object.
User Impact
Review and repair commands can recover the valid structured result instead of failing with
malformed-output.Why This Change Was Made
The fallback parser now records balanced candidates in one pass, preserves top-level candidate priority, and recovers the final valid direct child of an unmatched prose brace. It also keeps quoted prose out of candidate selection and resets lexical state between unrelated top-level candidates.
Existing handling for strict JSON, fenced JSON, balanced malformed candidates, nested draft objects, and text without JSON remains covered.
Evidence
nullbefore the fix while the other 140 focused provider tests passed.pnpm test src/provider.test.ts(146 passed)pnpm typecheck(passed)pnpm lint(passed)pnpm format:check(passed)pnpm test(970 passed, 2 skipped)pnpm test:coverage(970 passed, 2 skipped)pnpm build(passed)pnpm pack:smoke(passed)git diff --check(passed)Shipped CLI and provider-adapter proof
A deterministic provider executable fixture wrote this exact captured response through the normal adapter path:
The packaged CLI then completed a review of a committed fixture repository without a malformed-output error:
This proof exercises the shipped review command, provider adapter, shared extractor, schema validation, and workflow result. The provider response is deterministic; it does not claim a live external-provider run.
Disclosure: AI was used to understand the codebase and review the fix.