Skip to content

fix(api): extract_json transform panics on model output with a preceding code fence#136

Open
santhreal wants to merge 2 commits into
graniet:mainfrom
santhreal:fix/extract-json-panic
Open

fix(api): extract_json transform panics on model output with a preceding code fence#136
santhreal wants to merge 2 commits into
graniet:mainfrom
santhreal:fix/extract-json-panic

Conversation

@santhreal

Copy link
Copy Markdown

The extract_json response transform locates the closing fence with resp.find("\``"), which matches the first triple-backtick anywhere in the response rather than the one that closes the ```json block. When the model emits any code fence before the JSON block (e.g. a ```python example, then a ```json result — common output), the closing index lands before the opening one and &resp[start..end]` panics in the request handler.

It also never worked on the happy path: from_str::<String> parses a {...} block as a JSON string literal, so it always returned "Invalid JSON response".

Fix anchors the close-fence search after the ```json opener and returns the trimmed inner text (falling back to the trimmed response when no fence is present). Four regression tests cover the panic repro, object extraction, the no-fence fallback, and an unterminated fence.

… JSON

The extract_json response transform located the closing fence with
resp.find("```"), which returns the first triple-backtick anywhere in
the response. When any code fence precedes the ```json block (common
model output, e.g. a ```python example first), the opening index lands
after the closing index and &resp[json_start..json_end] panics with
'byte range starts at N but ends at M'. This is reachable from the HTTP
API via the response_transform request field.

It also parsed the fenced content with serde_json::from_str::<String>,
which expects a quoted JSON string, so a ```json object block always
fell through to 'Invalid JSON response' — the feature never worked.

Anchor the closing-fence search after the opening ```json marker,
return the trimmed inner content, and fall back to the trimmed response
when no fenced block is present. Add regression tests for the panic
case, object extraction, no-fence fallback, and an unterminated fence.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2cbd72c78

ℹ️ 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".

Comment thread src/api/handlers/helpers.rs Outdated
Comment on lines +50 to +51
let end = inner.find("```").unwrap_or(inner.len());
inner[..end].trim().to_string()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Require closing fences to start a line

When the JSON payload contains Markdown code fences inside a string field, such as a model returning { "answer": "```python\n..." } within the outer json block, this `find("")` treats the embedded backticks as the end of the JSON fence and truncates the extracted response. Because this transform feeds downstream chain steps, those inputs become malformed even though the outer fenced JSON block is valid; look for a closing fence only on a fence line after the opener rather than any backticks in the payload.

Useful? React with 👍 / 👎.

Embedded ``` inside JSON string values must not truncate the extracted payload.
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