fix(llm): recover malformed JSON in the openai-compatible provider (#3683) - #3685
fix(llm): recover malformed JSON in the openai-compatible provider (#3683)#3685chiruu12 wants to merge 3 commits into
Conversation
r266-tech
left a comment
There was a problem hiding this comment.
At head SHA 65161aa3e21fc5f3dba3d95b1bc8d3a67181a86a, the retry short-circuit treats any two failures with the same JSONDecodeError message, line, and column as a deterministic repeat without checking that the response text is unchanged. Two different stochastic outputs can therefore be classified as a deterministic repeat and trigger parse_llm_json on the second attempt; if repair cannot recover that output, the exception exits the retry loop and skips the remaining generation budget. A later valid response is therefore dropped instead of being attempted.
Please only short-circuit when the failed response content is also identical, or otherwise retain a retry when content differs, and add a regression test with two distinct responses sharing the same JSONDecodeError signature followed by a valid response.
The bound verification observed head SHA 65161aa3e21fc5f3dba3d95b1bc8d3a67181a86a, and git diff --check passed. The focused pytest command was not independently run because uv was denied permission to create its cache directory in the read-only environment.
|
Right, that holds. Two different malformed responses can share a JSONDecodeError message, line and column, so the check called them a repeat and spent the rest of the retry budget on repair instead of on the generation that would have worked. The tracker now holds the response content, and the short-circuit only fires when the text comes back unchanged. Identical content produces an identical decode error, so the position fields carried nothing the content did not already carry. The dataclass is gone with them. Added |
koriyoshi2041
left a comment
There was a problem hiding this comment.
Verified the update at 2927e8e164aa6c33fcf06aefa1e80d4ffc0a1fc9. The repeat check now keys on response content in both compatible and native Ollama paths, so distinct malformed outputs with the same decode position retain the remaining generation attempt. The focused response-hardening suite passes 13/13; Ruff check/format and the exact update diff check are clean.
|
Verified the update at |
Fixes #3683.
openai_compatible_llm.pyparsed structured output with barejson.loadsand, on failure, retried by re-sending a byte-identical request.parse_llm_jsonwas never reached, so a recoverable response was never repaired and a deterministic one costmax_retries + 1full generations before the chunk was dropped.What changed
Both parse sites in the file now fall back to
parse_llm_jsononce a fresh generation can no longer help: thechat.completionspath and the native Ollama/api/chatpath. That mirrors whatlitellm_llm.pyhas done since #2547/#2544, including its preference for a clean re-roll before repair.The retry ladder also stops early when a parse failure repeats at the same
(msg, lineno, colno). Every attempt re-sends the same request, so a failure at the same position will keep repeating. Two attempts establish that, and the rest of the budget goes to repair instead of more generations. A failure at a new position still earns a fresh generation, so flaky output keeps the behaviour it has today.Tests
Added to
tests/test_openai_compatible_response_hardening.py. All mocked, no network:Three of those fail on current
main. The fourth pins the re-roll behaviour this change keeps.ruff check,ruff format --checkandty checkare clean. 295 passed and 5 skipped across every test file that touches the provider.Not here
The issue also suggests feeding the invalid output back as a repair turn, and a per-document dropped-chunk counter on the retain response. Both change behaviour past parsing, so I left them on the issue.