fix(llm): tolerant JSON parsing, truncation escalation, provider retries, grounding gate - #210
Open
lfnothias wants to merge 8 commits into
Open
fix(llm): tolerant JSON parsing, truncation escalation, provider retries, grounding gate#210lfnothias wants to merge 8 commits into
lfnothias wants to merge 8 commits into
Conversation
…ervable Found while running Mimosa against ASB capsules on OpenRouter's stealth tier. Temperature. workflow_factory samples random.uniform(0.7, 1.3) for every workflow generation, clamping to 1.0 only for Anthropic. Backends that cap at 1.0 refuse the rest. The retry path already knew how to recover — drop to 1.0, retry — but _is_temperature_error only read error.param, which OpenAI-style APIs set and gateways do not: OpenRouter forwards the refusal as a bare 400 whose metadata.raw is the string "ERROR". So the recovery never fired and the task aborted instead. Measured against stealth/ox-alpha: temperature 1.0 and 0.7 succeed, 1.3 returns that 400 — roughly half of all generations died. Detection now also treats a 400 on a request above the 1.0 ceiling as a temperature rejection; a 400 at or below it is left alone so real bad-requests are not masked. Grounding. Every failure path in query_perspicacite returns None and each caller substitutes a "no relevant scientific context" string, so a run where grounding failed outright produces the same artifacts as a grounded one. Added a per-attempt ledger and grounding_stats(), folded into run_metrics.json, so a run can be shown to have been grounded rather than assumed to have been. Also: - perspicacite_kb_name / _mode / _max_papers config knobs. kb_name was hardcoded None, so retrieval always took the web-search pipeline and the local knowledge bases were unreachable. Default stays None — behaviour unchanged unless set. For benchmarks, never scope it to a KB built from the paper under reproduction; that hands the agent the graded values. - Persist reasoning_effort only when it was actually sent. It is gated to the o1/o3/gpt-5 families, but the configured value was recorded for every model, putting a parameter in the run's provenance the provider never received. - Precheck now names save_logprobs when it is on and every probe failed. The probe pairs logprobs with require_parameters, so an endpoint that simply does not advertise logprobs fails with a routing-shaped 404 and the generic advice sent operators to the provider allowlist instead. - Drop _OVERALL_TIMEOUT: defined, documented, never passed to httpx. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 6499f03)
The flag was read in one place — orchestrator._ground_with_perspicacite. planner.py and the judge's grounding evaluator (via verifier.py and generic.py) call query_perspicacite directly, so turning it off still left two of the four call sites querying the literature. The recorded workaround was to point PERSPICACITE_API_URL at an unreachable host, which is also what the fair-eval notes in the dev hub prescribe for benchmark runs. Gating in the client instead covers every caller from one place. Skipped calls are recorded as "disabled" and excluded from the hit-rate denominator, so a deliberately ungrounded run is not reported as a run whose grounding failed. Default stays enabled; behaviour is unchanged unless the flag is set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit cf34548)
Observed running ASB capsules against stealth/ox-alpha: the planner failed all three attempts with "Failed to extract valid JSON from LLM response" / "Invalid control character at …" and the task was abandoned. The model had pasted a multi-line span straight into a string value. _extract_json_from_code_block called bare json.loads on the fenced block, so an otherwise complete plan was discarded over an unescaped newline. The repair ladder that already exists in onboard_cli.py never covered this path. Adds sources/utils/llm_json.loads_llm_json, ported from ASB's llm_pipeline helper, which solved the same problem against the same model family. It repairs raw control characters, bare interior quotes, and trailing prose after a complete object — but only after a strict parse fails, so valid JSON is returned byte-for-byte unchanged, and it re-raises the original JSONDecodeError when nothing parses so callers still see the true defect. The per-claim verifier and workflow_info parse LLM JSON the same way and are candidates for the same treatment; left alone here because neither was observed failing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 0f056aa)
lane1_t0 (p_iimn) ran with the control-character repair already in place and
still lost its plan after three attempts:
Expecting property name enclosed in double quotes: line 47 column 64
That message reads like a quoting fault; it is a trailing comma — the one JSON
forbids and both JavaScript and Python allow. Six occurrences in that one run.
strip_trailing_commas drops a comma followed only by } or ], tracking string
state so commas inside values survive. The ladder now tries strict, then
string-repair, then comma-strip, then both — a real response can carry a pasted
multi-line span and a trailing comma at once.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 926ae7e)
_extract_json_from_code_block scanned only for ```json fences and returned None otherwise, which the caller turns into "Failed to extract valid JSON from LLM response" and, after three attempts, abandons the task. A model told to answer in JSON frequently just answers in JSON. Verified against the persisted plan_creator response from the failed lane1_t1 run: a valid 7558-character plan object, two top-level keys, three steps, no fence — discarded. Same failure on lane2_t0. That made it the most common terminal failure in the sample, ahead of anything model-quality related. Now tries the fenced block first (unchanged when present), then the bare response, then from the first brace so a leading "Here is the plan:" does not cost the plan either. Still returns None when there is genuinely no JSON. Re-running the extractor over that saved response now recovers the plan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit a2b7598)
Same behaviour, spelled out as the three cases it actually handles: opening quote, closing quote, unescaped interior quote. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 9fb0bca)
A response cut off at max_tokens arrives with finish_reason="length" and no exception, and the caller parses it as if it were complete. Reasoning models make this routine: the budget is spent on reasoning before any content is emitted, so a budget that would comfortably fit the answer still returns a fragment. Observed on the p_iimn re-run against stealth/ox-alpha, with every JSON repair already in place: the planner call returned Completion: 8192 against a max_tokens of exactly 8192, four truncation warnings in one run, and JSON that ended mid-string. The parser failed six times with "Unterminated string" and the task was abandoned. No repair recovers a document that was never finished. The budget now doubles and retries, at most twice and never past 65536. The config value is not mutated, so escalation is per-call. When it is still truncated after the last escalation the warning says so explicitly instead of suggesting the operator "consider increasing max_tokens" — by then that advice has already been taken automatically. This is the same shape as the existing context-window handler, which halves an oversized prompt and retries; that one guards the input side, this one the output side. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 78ecb18)
Once the temperature fallback landed, every temp>1.0 generation recovered: the log shows 1.20, 1.10, 1.24, 1.08 each falling back to 1.0 and then "generated in Ns". The generation failures that remained were all at temperature <= 1.0 — a different cause. That cause is OpenRouter reporting an upstream fault as HTTP 400 with message "Provider returned error" and metadata.raw "ERROR". The 400 makes it look like a malformed request. It is not: six identical calls at temperature 0.85 with a full-size workflow prompt succeeded in isolation while the same shape was failing intermittently under four concurrent lanes. Semantically it is a 502. _is_retryable_error did not match that wording, so it raised immediately with no backoff and each occurrence cost a whole workflow generation — the single largest remaining source of lost work in the sample. The match is deliberately narrow, on the gateway's own phrasing rather than on 400s in general, so a genuinely malformed request is not retried in a loop; it would keep failing and still exhaust the existing retry ceiling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 3aa1b8c)
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
1 participant
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.
Slice 2 of 4 from #197. Eight commits cherry-picked unchanged from
mimosa_v2_lfx, all found while running Mimosa against ASB capsules through OpenRouter. Each carries its own test file; the commit messages hold the measurements.Problem
Four ways a run lost work that the model had already produced:
workflow_factorysamples temperature in [0.7, 1.3]; backends capped at 1.0 refuse the upper half. The fallback existed but only recognisederror.param, which gateways do not set, so about half of all workflow generations died instead of retrying at 1.0._extract_json_from_code_blockdid a barejson.loadson the fenced block and returnedNonewhen there was no fence. A plan with an unescaped newline, a trailing comma, or no fence at all was dropped after three attempts. This was the most common terminal failure in the sample.finish_reason="length"response raised nothing and was handed to the parser, which failed with "Unterminated string" six times per run._is_retryable_errordid not match it, so each occurrence cost a whole generation with no backoff.Separately,
literrature_groundingwas read at one call site only, so switching it off still left two of the four callers querying the literature, and grounding outcomes were not observable.Solution
sources/utils/llm_json.py: strict parse first, then repair of control characters, interior quotes, trailing commas and trailing prose, in that order; valid JSON is returned unchanged and the originalJSONDecodeErroris re-raised when nothing parses. The planner also accepts unfenced JSON and JSON after a leading sentence.max_tokensand retries, at most twice, never past 65536, per call.perspicacite_clientfor every caller; skipped calls are recorded as "disabled" and excluded from the hit-rate denominator.Configgainsperspicacite_kb_name,perspicacite_modeandperspicacite_max_papers(round-trip covered), applied once inmain.pybefore any component builds a query.Testing
Fresh
uv sync --group dev --python 3.11on this branch:The 17 failures are exactly the pre-existing set listed in
.github/known-failing-tests.txton #205. This slice adds 72 passing tests:llm_json_test,temperature_fallback_test,truncation_escalation_test,grounding_telemetry_test.Backwards compatibility
Valid JSON, in-range temperatures and complete responses take the same path as before. New config fields default to the previous behaviour (web-search pipeline, agentic mode, 5 papers).
literrature_groundingdefaults to enabled.🤖 Generated with Claude Code