Skip to content

fix: parse provider JSON after unmatched prose - #238

Open
goutamadwant wants to merge 1 commit into
openclaw:mainfrom
goutamadwant:fix/provider-json-unbalanced-preamble
Open

goutamadwant wants to merge 1 commit into
openclaw:mainfrom
goutamadwant:fix/provider-json-unbalanced-preamble

Conversation

@goutamadwant

@goutamadwant goutamadwant commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

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

  • Regression proof: both reported unmatched-preamble inputs returned null before 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)
  • Independent correctness and security reviews found no patch-introduced actionable issues.

Shipped CLI and provider-adapter proof

A deterministic provider executable fixture wrote this exact captured response through the normal adapter path:

Plan: wrap keys in { braces.
{"findings":[],"inspected":{"files":["src/index.ts"],"symbols":[],"notes":["provider-adapter-proof"]}}

The packaged CLI then completed a review of a committed fixture repository without a malformed-output error:

$ PATH="./provider-proof/bin:$PATH" node dist/cli.js --root ./provider-proof/repo --state-dir ./provider-proof/state --plain --no-color review --provider codex --feature feat_library_f8bf0b65a1 --jobs 1
clawpatch review start run=20261019T050746-62224c features=1 jobs=1
clawpatch review feature-start index=1 total=1 feature=feat_library_f8bf0b65a1 title=Node source src
clawpatch review feature-done index=1 total=1 feature=feat_library_f8bf0b65a1 findings=0 elapsed=0s
clawpatch review done run=20261019T050746-62224c reviewed=1 findings=0
run: 20261019T050746-62224c
reviewed: 1
findings: 0
jobs: 1

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.

@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 19, 2026
@clawsweeper

clawsweeper Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

What this changes

The PR recovers valid provider JSON following unmatched braces in leading prose.

Example: The fallback extractor receives Plan: wrap keys in { braces.\n{"ok":true}.

  • Before: Returns null despite the valid JSON object.
  • After: Returns the object {"ok":true}.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The focused patch has no identified correctness defect, but test-only evidence does not satisfy the external-PR behavior-proof gate.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Review or repair can fail with malformed-output even when the provider returned valid structured JSON after prose containing an unmatched brace.
Reason: Recovering the reported responses extends the established tolerant-extraction contract and benefits every adapter using the shared helper.

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
Reviewed head: 20008175b096dc883ad865994727d53dbf8ade9e

Before merge

  • Add real behavior proof - The supplied evidence exercises extractJson through deterministic tests and reports passing checks, but does not demonstrate recovery through the shipped CLI and provider adapter; provide terminal output or a terminal screenshot of that scenario, redacting credentials and non-public endpoints. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.

Findings

None.

Tests

  • Missing end-to-end proof: No supplied evidence shows a shipped review or fix command successfully consuming a schema-valid provider result after unmatched leading prose.
Agent review details

How this fits together

Provider 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]
Loading

Technical review

Best 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

  • Shared fallback candidate selection keeps the original intent (429cc9c: Skip balanced malformed preambles while excluding their nested draft objects; earlier commits extracted the helper and connected Codex to tolerant parsing.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

  • Current main still contains the defect: An unmatched opening brace leaves depth nonzero and terminates extraction without examining later objects. (src/provider-json.ts:49, 610a26db8437)
  • Introduced repair and focused coverage: The pinned diff retains strict and fenced parsing, prioritizes balanced top-level candidates, and recovers valid children of unmatched braces; tests cover both reported inputs and preserve nested-draft exclusion. (src/provider-json.ts:14, 20008175b096)
  • Existing parsing intent: Commit 429cc9c added retries after balanced malformed candidates and explicitly tested that nested draft JSON is skipped. (src/provider.test.ts:200, 429cc9cbb9fd)
  • Production failure boundary: Pi passes stdout to extractJson and throws malformed-output on null; Codex and several other adapters also consume the shared extractor. (src/providers/pi.ts:113, 610a26db8437)
  • Linked report and supplied proof: The concrete inputs in extractJson gives up when provider output has an unbalanced "{" before the JSON object #237 match the repair; the complete PR body reports deterministic tests and checks, while its comments supply no shipped-command demonstration. (20008175b096)
  • History coverage: History identifies steipete's helper extraction, Codex fallback integration, and malformed-preamble repair; local historical blob reads encountered missing objects, and REST commit patches supplied the extraction and integration details. (src/provider-json.ts:3, 611103ab59d5)

Labels

Label changes:

  • add P2: A specific provider response pattern interrupts review and repair despite usable JSON; no urgent regression is established.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Label justifications:

  • P2: A specific provider response pattern interrupts review and repair despite usable JSON; no urgent regression is established.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Supply a redacted shipped-command trace demonstrating successful recovery of a schema-valid result after unmatched leading prose.

Rating scale

6/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.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 18, 2026, 11:44 PM ET / October 19, 2026, 03:44 UTC.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extractJson gives up when provider output has an unbalanced "{" before the JSON object

1 participant