fix: parse structured output wrapped in a markdown code fence - #583
Open
andyli953 wants to merge 1 commit into
Open
fix: parse structured output wrapped in a markdown code fence#583andyli953 wants to merge 1 commit into
andyli953 wants to merge 1 commit into
Conversation
A model asked to reply with JSON frequently wraps it in a ```json fence. `parseStructuredOutput` calls `JSON.parse` on the raw message, so the fence throws `Unexpected token '`'` and a well-formed review is discarded in favour of the raw-text fallback. Observed on 2 of 3 consecutive `/codex:adversarial-review` runs against the same repository: identical prompts, and whether the reply arrived fenced or bare varied between runs. Content was never lost, since the fallback prints the raw message, but the structured path -- verdict, findings, per-finding file and line -- was unavailable whenever it happened. Only a fence enclosing the entire message is stripped, so JSON carrying backticks inside string values is untouched, and malformed output, prose and empty messages still fail exactly as before. The existing graceful degradation for an unexpected review *shape* is unaffected; this only addresses output that never reached the schema check. Adds tests/codex-structured-output.test.mjs. Four of its eight cases fail against the current parser and pass with this change; the other four are guards that pass either way, so the fix cannot be loosening the parser.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
parseStructuredOutputinplugins/codex/scripts/lib/codex.mjscallsJSON.parseon the raw final message. A model asked to reply with JSON frequently wraps it in a fence, so the parse throws:The review then falls back to printing the raw message. Nothing is lost, but the structured path — verdict, findings, per-finding file and line — is unavailable.
I hit this on 2 of 3 consecutive
/codex:adversarial-reviewruns against the same repository. Identical prompts; whether the reply arrived fenced or bare varied run to run, so it is not deterministic and not something the caller can avoid.Fix
Strip a fence, but only one that encloses the entire message:
That keeps JSON carrying backticks inside string values intact, and leaves malformed output, prose and empty messages failing exactly as they do today. This is deliberately narrow: it only recovers output that never reached the schema check. The existing graceful degradation for an unexpected review shape (
renderReviewResult, covered intests/render.test.mjs) is untouched.Tests
New
tests/codex-structured-output.test.mjs, in the existingnode:teststyle.```jsonfencedFour cases fail against the current parser and pass with the change; the other four pass either way, so the fix cannot be silently loosening the parser into one that accepts anything.
Full suite
npm teston macOS / Node v23.11.0, before and after:The failing set is byte-identical before and after:
These four fail on pristine
mainin my environment and are unrelated to this change — I have not investigated them and am not claiming they are broken upstream, only that this PR neither causes nor fixes them.