Skip to content

fix: strip markdown fences and prose from opencode batch payloads - #738

Open
dopamine-pixels wants to merge 1 commit into
peteromallet:mainfrom
dopamine-pixels:fix/strip-fences-from-opencode-payloads
Open

dopamine-pixels wants to merge 1 commit into
peteromallet:mainfrom
dopamine-pixels:fix/strip-fences-from-opencode-payloads

Conversation

@dopamine-pixels

Copy link
Copy Markdown

Problem

desloppify review --run-batches --runner opencode reports valid batches as failed when the model wraps its JSON output in markdown fences or prefixes it with prose. The run log shows "Runner returned 0 but output file is missing" and the batch lands in the failed list even though a complete, valid payload exists in the stream.

Fix

Normalise model output before parsing in _persist_opencode_payload_text:

  • Strip surrounding ` fences when present.
  • Fall back to the outermost {..} slice when prose surrounds the JSON.

Verification

  • Added tests: fenced-stream recovery through
    un_opencode_batch and direct _extract_json_payload_text cases (fenced, prose-prefixed, non-JSON).
  • pytest desloppify/tests/commands/review/test_review_runner_helpers_direct.py — 12 passed.
  • Observed in a real 20-batch review run: batches whose output was fenced JSON were the only failures; with this fix the retry round recovered them.

OpenCode frequently wraps the requested review JSON in `json fences or
prefixes it with prose. The payload parser required bare JSON, so those
batches were reported as failures ('Runner returned 0 but output file is
missing') even though the model produced a complete, valid payload.

Normalise model output before parsing: strip surrounding fences, then
fall back to the outermost {..} slice of the text.
github-actions Bot pushed a commit to citizenadam/desloppify that referenced this pull request Sep 15, 2026
github-actions Bot added a commit to citizenadam/desloppify that referenced this pull request Sep 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny coverage nit: the fenced tests look like they’re actually exercising the {...} fallback since there’s prose before the fence. Could be worth adding one pure json ... case so the fence-stripping branch gets hit directly.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant