Conversation
…task death, 25% of verdicts) (#278) * fix(session): give the completion verifier room to answer (ENG-1081) The verdict call is a forced tool call with max_tokens=256. Models that narrate before acting — the open-weight aliases MindsHub serves through Fireworks (mindshub_air/kimi, deepseek) — spend the whole budget on plain prose and never reach the tool call, so `generate_object_code` raises and the fail-safe converts it into a silent "task complete": the turn ends with no message and the agent looks like it died mid-task. Measured in prod Langfuse over 2026-07-14..07-27: 171 of 687 verdict calls returned no tool call (24.9%; 37.1% for external users), 145/147 of them on mindshub_air, every single one with completion_tokens == max_tokens exactly. Not new with ENG-716 — the old free-text verifier also passed max_tokens=256 and 21% of its verdicts were unparseable for the same reason; ENG-716 only changed the consequence from "keep working" to "stop silently". - Verifier budget 256 -> 2048, with one retry at 4096 when truncated. 2048 is measured: mindshub_air spent 1,654 output tokens even with the no-preamble instruction, so 1024 was not enough (1/3 at 1024, 3/3 at 2048). - Verifier prompt now asks for the tool call first, no preamble. Needed alongside the budget, not instead of it (0/3 at 256 even with it). - StructuredOutputError carries `truncated` so a blown budget is retryable while a genuine failure isn't. Detected by output-token count, because the gateway reports finish_reason "stop" at the cap for most aliases (ENG-1082); "length"/"max_tokens" are honoured when reported. - Truncation is logged distinctly from other verifier failures, which all logged identically as "verifier unavailable" before — the reason a 25% failure rate went unseen for 10 days. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(session): bound the verifier retry to once per session (ENG-1081) Self-review follow-up. If the truncation retry is itself truncated, the model won't fit a verdict at any budget we're willing to pay for — so a chronically-narrating model would otherwise cost 2048+4096 wasted output tokens on every turn instead of the old 256. Latch it after the first proof and stop buying the retry for the rest of the session; the first attempt still runs, so a transient truncation still recovers. Also drops a pointless f-string in the verifier prompt concatenation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llm): share the truncation classifier with the scratchpad path (ENG-1081) Adversarial-review follow-up. Four fixes: 1. The detection lived in `client.py`, so the sync twin in the scratchpad subprocess (`_ScratchpadLLM.generate_object`) still raised a blind `ValueError("LLM did not return structured output.")` — the same silent, causeless failure this branch set out to remove, on the path that is handed to model-written scratchpad code with a caller-chosen max_tokens. `structured.py` exists to keep those two paths in lockstep, so the classifier moves there as `raise_missing_tool_call` and both call it. 2. Drop the per-session retry latch. It assumed a double truncation proves the model can never fit a verdict. Measured: 16 identical verdict calls to mindshub_air spent 245..1654 output tokens — a 6.7x spread for the same request — so one truncation is a tail sample, not proof about the next turn, and latching the retry off on that evidence would reintroduce silent stops. The retry is cheap because it only fires when the first attempt truncates (0 of 12 at 2048). 3. The session tests hand-built StructuredOutputError, so they would have passed even with the detection broken. Added an end-to-end test wiring a fake provider through the real LLMClient into the session; verified by mutation (breaking the classifier fails it, and 4 others). 4. The plan_stream fake alternated on `call_count % 2`, coupling it to the tool loop's internal call count; a third call in a turn would desync it and the test would assert the wrong thing. Now keyed off turn state. Also records the measured distribution next to the budget constant, so the 2048 number carries its evidence (median ~290, max 1654, ~1.24x headroom) instead of a single sample. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(llm): treat a tool call truncated mid-arguments as truncation too (ENG-1081) Third review pass found the same bug behind a second door. The budget can run out *inside* the forced tool call's JSON, not only in the prose before it. That arrives as a non-empty but incomplete tool call — safe_parse_tool_input repairs what it can and sets parse_error — so the missing-call check never sees it, unwrap_structured_response raises a bare ValidationError, the verifier's `except Exception` treats it as a hard failure, skips the retry, and lands on exactly the silent-stop fail-safe this branch exists to remove. Given output length varies ~6.7x per call, a model that narrates ~2000 tokens and then starts emitting the call is precisely the case that lands here. - `looks_truncated` extracted so the same evidence rule covers both shapes. - `raise_missing_tool_call` -> `raise_unusable_tool_call`, message adapts to "did not return" vs "returned an unusable", and now typed NoReturn. - Both the async client and the scratchpad's sync twin classify a failed unwrap as truncation *only* when the response shows it; a malformed call with budget to spare still propagates as a validation error, so genuine schema bugs stay visible instead of buying a hopeless retry. - Parity test upgraded from a substring grep (which a comment could satisfy) to an AST check that the call really is inside the sync `generate_object`. Tests: 15 in the file, mutation-verified — breaking `looks_truncated` fails 7 of them, including both end-to-end shapes. Full unit suite 1329 passed with the same 2 pre-existing scratchpad-exec failures; all 36 e2e scenarios pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(e2e): cover the truncation retry end-to-end; log the verifier's error cause Tool-assisted review pass. - The e2e stub could not express a truncated response (usage.completion_tokens was hardcoded to 10), so the retry had no coverage on the real subprocess path. `_Response.output_tokens` + `queue_verification_truncated()` emulate what the gateway actually returns — prose, no tool call, completion_tokens at the cap, and `finish_reason: "stop"` anyway (ENG-1082). New scenario asserts the session makes a second verdict call with a larger budget and honours the retried verdict instead of stopping silently. - The verifier's generic `except Exception` logged a bare `verdict=ERROR` with no cause — the same undiagnosable swallow this branch exists to fix, and the same thing I flagged on #276. Now logs the exception type and message. Full unit suite 1329 passed; all 37 e2e scenarios pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(session): keep conversation content out of verifier logs (review) @pnewsam, Major on #277: logging `str(exc)` can copy conversation content into ordinary application logs. Correct — a pydantic ValidationError embeds the rejected `input_value`, which for the verifier is model-generated text derived from the user's conversation, and provider exceptions can carry response bodies. It also contradicted this PR's own security claim. - `_safe_error_detail(exc)` emits the exception type plus, for validation errors, the failing field locations and error codes only — via `errors(include_input=False, include_url=False, include_context=False)`, and reading only `loc`/`type` even on the v1 fallback, so `msg`/`input`/`ctx` can never be quoted. Provider errors reduce to type + status_code. - Same leak class found in a pre-existing line: the summary log wrote `reason=%s`, the model's free-text justification, on EVERY verdict — a broader exposure than the one reported. Dropped; status and counters stay, and the reason remains in the Langfuse trace where conversation content belongs. - 3 regression tests, including one asserting a planted value never reaches the log detail. Also shortened the long comment blocks flagged in review (structured.py, session.py budget/prompt constants, client.py, scratchpad_boot.py) — same facts, roughly half the lines. 1332 unit tests pass; all 37 e2e scenarios pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(session): _safe_error_detail must never raise (hotfix hardening) It runs inside the verifier's `except` handler, so an exception escaping it would convert a gracefully-handled verifier failure into a dead turn — the exact failure mode this branch exists to remove. A custom exception can expose `status_code`/`errors` as a property that raises, so the attribute reads are wrapped too. Degrades to the exception type name, never a crash and never the message. Found while assessing hotfix risk for #278. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Sync
main→stagingafter the ENG-1081 hotfixStep 2 of the hotfix flow. #278 is merged and released as
v2.26.7.27.2(PyPI publish and live e2e both green), somainnow carries the completion-verifier fix andstagingdoes not. This brings it back.This merge is required, not optional.
staging-unfreeze.yml— which normally runs the post-release sync-back — is gated onhead.ref == 'staging', so it does not fire for a hotfix branch. Without this PR,stagingstays without the fix and the next weeklystaging → mainmerge would carry a base that never had it.What it changes
Dry-run merge: zero conflicts, and it touches exactly the 8 files of the hotfix — nothing else:
No migrations, no dependency changes, no config changes.
staging's own unreleased work (ENG-838,prompts.py) is untouched — the two changesets don't overlap.Verified before opening
mainbase this code was 1335 unit tests / 0 failures and 37/37 e2e scenarios; the release's owne2e-livejob passed against tagv2.26.7.27.2.After this merges
mindshub_air, fix(session): completion verifier fails toward a real message, not silence (ENG-1079) #276 landing on a base without this fix would fire its "an internal check failed…" message on ~99% of turns.Delivery is still pending and separate: hub agents need a
Build Snapshot+ SAM redeploy, desktop needs an installer build (or an updater reinstall). No version bumps — both track the git branch.