ENG-1081: give the completion verifier room to answer (fixes the silent-stop root cause)

[alecantu7]

What & why

The completion verifier asks for its verdict as a forced tool call with max_tokens=256. Models that narrate before acting spend the entire budget on plain prose and never reach the call — generate_object_code raises, and the fail-safe converts that into a silent COMPLETE. The turn ends with no message at all, which is what users report as "Anton dies on the first hurdle, even after restarting".

Measured in prod Langfuse (2026-07-14 → 07-27; all 20,976 Chat Completions Handler v1 generations scanned, 1,241 verifier calls identified by system prompt and split by protocol):

• 171 / 687 post-ENG-716 verdict calls returned no tool call = 24.9% (external users only: 37.1%).
• 145 / 147 of them on mindshub_air (98.6%), and every single failure has completion_tokens == max_tokens exactly, with ~1,300 characters of reasoning prose cut off mid-sentence.
• 33 users, 51 sessions in 10 days. Whole sessions where it never once worked: 15/15, 12/12 (turns 7→21 consecutively), 8/8 — which is why restarting doesn't help. The user's model setting is the same next session.

This is not a regression from ENG-716. The old free-text verifier passed max_tokens=256 too, and 116/554 (21%) of its verdicts were unparseable for the same reason (sampled 8/8 sitting exactly at the cap). ENG-716 changed the consequence of truncation — from INCOMPLETE/"keep working" to except/silent stop — not its frequency. It also isn't a model-capability problem: all nine non-narrating aliases, cheap ones included, answer correctly at 256.

Evidence

Identical verifier payload (real system prompt + real _VerifierVerdict schema, synthetic transcript) against prod api.mindshub.ai, all 14 aliases:

models	verdict at max_tokens=256	output tokens	prose
haiku, sonnet, opus, gpt, gpt-codex, gpt-mini, gpt-nano, gemini, gemini-flash	✅ 2/2 each	43–115	none
kimi, mindshub_air (= kimi-k2p6), deepseek	❌ 0/2 each	256 (capped)	~1,300 chars

The nine that work are first-party APIs where tool_choice is enforced structurally — the model cannot emit anything before the call. The three that fail are open-weight chain-of-thought models on Fireworks, where tool_choice is advisory and the narration lands in ordinary content, eating the same budget. mindshub_air narrates even on "say ok".

The fix numbers come from a measured distribution, not one sample. mindshub_air, identical verdict payload:

variant	result
256 (current)	0/5 — capped at 256 every time
256 + no-preamble prompt	0/3 — capped at 256
1024 alone	1/3 — two runs capped at 1024
2048 + no-preamble	15/15

Across 16 runs at 2048 + no-preamble the output length was:

245 246 247 253 253 254 267 279 287 295 392 407 571 573 610 865 1654
median ~290, max 1654 — a 6.7x spread for the *same* request

Two things follow. 1024 was genuinely too small (a 1,654-token run would blow it). And 2048 is only ~1.24x the observed worst case, so the headroom is thinner than a single number suggests — which is why the 4096 retry exists rather than one larger budget, and why there is deliberately no "this model can't do it" latch: with a 6.7x per-call spread, one truncation is a tail sample, not proof about the next turn. The retry stays cheap because it only fires when the first attempt truncates — 0 of 12 at 2048. All three narrating models (kimi, mindshub_air, deepseek) pass at 2048 + no-preamble.

Latency (the fair question when raising a ceiling 8x), mindshub_air on the verifier payload: 4.8–8.2s at 2048 + no-preamble vs 3.4–5.4s at 256 — so ~+2–4s on the narrating models, and unchanged everywhere else (first-party models emit 43–115 tokens regardless of the ceiling). Today's 256 calls spend that time and return nothing.

What changed

• Verifier budget 256 → 2048, with one retry at 4096 when truncated. Nothing pays for headroom it doesn't use; first-party models still spend ~80 tokens. Today's failures already burn a wasted 256 per turn and return nothing, so this is close to cost-neutral.
• Verifier prompt asks for the tool call first, no preamble. Tool name is derived from _VerifierVerdict.__name__ so it can't go stale.
• StructuredOutputError (a ValueError subclass, so existing catchers are unaffected) carries truncated, output_tokens, max_tokens, stop_reason — so a blown budget is retried while a genuine failure isn't charged a pointless second call.
• Truncation detected by output-token count, not finish_reason: the gateway returns "stop" while handing back exactly max_tokens for 9 of 11 aliases (ENG-1082). "length" and Anthropic's "max_tokens" are honoured when reported, so BYOK-direct users are covered too.
• Truncation now logs distinctly from other verifier failures. Previously all four failure modes logged identically as reason="verifier unavailable" — the reason a 25% failure rate went unseen for 10 days. The rate is now measurable from logs alone.

Testing

• tests/test_verifier_truncation.py — 12 new tests: prose-at-the-cap is reported truncated even when the gateway says "stop"; both provider dialects ("length" / "max_tokens") count as truncation; a short empty response is not truncated (so it isn't retried); a truncated verdict is retried exactly once with the larger budget and the retried verdict is the one that counts; a non-truncated failure costs exactly one call; the no-preamble instruction reaches the model.
• Full unit suite: 1326 passed, 17 skipped. The 2 failures (tests/test_chat_scratchpad.py scratchpad-exec) are identical on clean staging — verified by stashing this diff and re-running. Pre-existing env issue, not this change.
• tests/e2e/scenarios/test_loop_safety.py — 6 passed, including ENG-716's WAITING test.

Relationship to #276 (ENG-1079)

Complementary, not competing — and this one should land first or together. #276 replaces the silent stop with an honest "the check failed, how would you like to proceed?" message. That's the right last-resort behavior and still needed for the failures a budget can't fix (a disabled model 403, a provider 500, malformed args — see ENG-1083). But at ~99% failure on mindshub_air, #276 alone would show that message to those users on nearly every turn. With the ceiling fixed, it stays the rare fallback it's designed to be.

Both touch the same try/except around the verdict call, so expect a small conflict. Deliberately not stacked: this branches from staging. Whichever merges second resolves — this PR's version keeps the fail-safe in an else: branch, so #276's honest-diagnosis body drops into it directly.

Security

Reviewed the diff: no new inputs, endpoints, auth paths or deserialization. The new exception and log line carry only token counts, a budget, and a stop_reason — no conversation content, no credentials. The verifier's LLM payload is unchanged in content (only its token budget and one instruction line). Raising the budget cannot leak anything the same call didn't already send.

Closes ENG-1081.

[alecantu7]

Adversarial self-review

Five findings. Two fixed in 84fa1ea, three kept intentionally with reasoning.

1. Anthropic-dialect truncation was missed — fixed (in 6d7905a before opening)

The first version only checked stop_reason == "length". AnthropicProvider passes Anthropic's own value through raw (anthropic.py:201), and Anthropic says "max_tokens". Direct-Anthropic BYOK users would have relied on the token-count fallback alone. Now both dialects are honoured, with a parametrized test over ["length", "max_tokens"].

2. A chronically-narrating model pays for the retry on every turn — fixed (84fa1ea)

If a model narrates past 4096 too, the old shape spent 2048 + 4096 wasted output tokens per turn, versus 256 before this PR — a 24x cost regression for exactly the users this fix targets, and the verdict still fails. Added a per-session latch: once the retry has itself been truncated, later turns attempt only the first budget. The first attempt still runs, so a transient truncation still recovers. Measured evidence says this path should be rare (all three narrating models fit in ≤1,654 tokens), but "should be rare" is what the original except assumed too, which is precisely why we're here.

3. Missing usage silently disables the retry — kept, noted

Truncation detection needs usage.output_tokens. A provider that reports no usage (output_tokens == 0) yields truncated=False, so no retry. That's the safe direction — we don't pay for a retry we have no evidence will help — and every provider anton ships with does report usage. Not worth a heuristic on response length; flagging so it's a known limit rather than a surprise.

4. A truncated-then-failed call logs twice — kept, intentional

It emits verdict=TRUNCATED … per attempt and then verdict=COMPLETE reason="verifier unavailable" from the existing summary line. Slightly noisy, but the two lines answer different questions (what each attempt did vs. what the loop concluded), and I deliberately did not change the existing summary line's format — it's the one a log query would already be parsing.

5. The retry re-sends the full transcript — kept, deliberate

The retry is a second complete call (~2.5k input tokens), not a continuation. Continuing from a partial narration would mean feeding the model its own truncated prose and hoping it lands the tool call next — strictly less reliable than a clean attempt with room, and the input side is the cheap side. The latch in #2 bounds the worst case.

Not in scope, tracked elsewhere

• ENG-1082 — the gateway reporting finish_reason: "stop" at the cap. Once fixed, the token-count comparison here can be simplified to the standard check.
• ENG-1083 — qwen/fable fail 100% with 403/404, which no budget fixes. That path needs fix(session): completion verifier fails toward a real message, not silence (ENG-1079) #276's honest message.
• ENG-1081 work item 5 — sweeping the other generate_object* callers (cerebellum, identity extraction, scratchpad post-mortem) for the same tight-ceiling exposure. Not done here to keep this diff to the user-visible bug; the new StructuredOutputError.truncated gives them the same handle when someone picks it up.

[alecantu7]

Second, harder adversarial pass on this PR

The first self-review found process-level issues. This one found a real defect and a design mistake in my own change, plus new measurements that weaken a claim in the PR body. Eight findings, ranked. #1 and #2 should be fixed before this merges.

---

1. Parity break — the sync twin still raises blind (defect + convention violation)

anton/core/llm/structured.py exists precisely to keep the two structured-output paths in lockstep; its own docstring says "only the actual provider call differs between the two runtime contexts." This PR puts the truncation detection in client.py, so:

• async (LLMClient._generate_object_with) → knows truncation from genuine failure ✅
• sync (_ScratchpadLLM.generate_object, scratchpad_boot.py:235) → still raise ValueError("LLM did not return structured output.") ❌

The sync path is the more exposed one: it's handed to model-generated scratchpad code via get_llm().generate_object(...) with a caller-supplied max_tokens. A narrating model that picks a small budget in the scratchpad gets the same silent, causeless failure this PR set out to eliminate — and generate_object in the scratchpad already has a history of returning nothing quietly (ENG-1004).

Per convention 7 ("if a new pattern is genuinely better, the same PR must include a migration plan for existing occurrences") this diff owes the migration, not a follow-up. Fix: move the classification into structured.py as a shared raise_missing_tool_call(response, budget, tool_name) and call it from both sites. One helper, both paths, no drift.

2. The retry latch is the wrong shape — output length is stochastic, not per-model

I added _verifier_retry_exhausted on the theory that a double truncation proves this model can't fit a verdict. New measurements say that theory is wrong.

Same payload, same config (mindshub_air, 2048 + no-preamble), 16 runs:

output tokens: 245, 246, 247, 253, 253, 254, 267, 279, 287, 295, 392, 407, 571, 573, 610, 865, 1654
                  ^ min                                                            max ^
median ≈ 290                                                              spread ≈ 6.7x

Output length varies 6.7x run to run for an identical request. So a single truncation past 4096 is a rare tail event, not a property of the model — and my latch responds to one tail sample by permanently disabling the retry for the whole session, where the cost of being wrong is a silent stop on every subsequent turn.

Given the retry barely fires at 2048 anyway (0 of 12 truncations in the latest sample), I now think the right call is to drop the latch entirely — it optimizes a path that rarely runs, using evidence that doesn't support the inference. If it stays, it needs two consecutive proofs or a decay, not a one-shot permanent flip.

3. The PR body overstates the headroom at 2048

The body says mindshub_air "spent 1,654 output tokens even with the no-preamble instruction — so 1024 would not have been enough." True, but incomplete: across the fuller distribution above, 2048 is only 1.24x the observed worst case (1654), and the tail past 1654 is uncharacterized. That's thinner than "measured, not guessed" implies, and it means the 4096 retry is doing more load-bearing work than the text suggests — which is also why finding #2 matters. The body should carry the distribution and the sample size, not the single worst number.

4. The session tests don't actually test the detection

All three session-level tests raise StructuredOutputError(truncated=True) by hand. If client.py's detection were wrong — wrong stop_reason strings, usage absent, an off-by-one on the comparison — every one of them would still pass. The client-level tests cover the classifier in isolation, but nothing exercises provider response → client → session → retry as one chain.

This is the ENG-747 lesson (never hand-build error fixtures; verify the whole chain) and I walked straight into it. Fix: one test with a fake provider returning prose-at-the-cap, through the real LLMClient, asserting the session retries.

5. Test fragility: the fake is coupled to the tool loop's call count

_session_that_uses_a_tool alternates on call_count % 2, assuming exactly two plan_stream calls per turn. If a turn ever makes three (an extra tool round, an internal recovery retry), the alternation desyncs and the multi-turn latch test silently asserts the wrong thing. Should key off history state (does the last entry carry a tool_result?) rather than a call counter.

6. Latency: measured after the fact, should have been in the body

Raising a token ceiling 8x invites "how much slower is the turn?" and I shipped without answering it. Measured now, mindshub_air on the verifier payload:

config	wall time	outcome
256 (old)	3.4s, 5.4s	fails
2048 + no-preamble (shipped)	4.8s, 5.0s, 8.2s	succeeds

≈ +2–4s on narrating models, unchanged elsewhere (first-party models emit ~80 tokens regardless of the ceiling). Fine — but it belongs in the PR body, not in a reviewer's head.

7. Suppressing chain-of-thought may cost verdict accuracy — unmeasured

The no-preamble clause tells CoT-trained models to skip the reasoning they were trained to emit. 12/12 runs returned the identical verdict (WAITING, correct for that scenario), so stability on one case is fine. That is not accuracy across cases, and suppressed reasoning is exactly where a quality regression would hide. Worth a follow-up: run the verifier over a labelled set of turns, with and without the clause, per model — same shape as the ENG-381 ground-truth fixtures.

8. After the latch fires we still buy a doomed call

If the latch survives #2: it drops to one attempt at 2048, which we have proof can't fit — so it spends 2048 tokens per turn to fail. If we know it can't work, skip the verdict call and go straight to the fail-safe (or #276's honest message) rather than paying for a guaranteed failure.

---

Verdict on my own PR: the core fix (budget + prompt, and the retry that makes the budget robust) is sound and evidenced. #1 is a genuine defect, #2 is a design mistake of mine that adds a new silent-failure risk, #4 means part of my test coverage is theatre. I'd fix 1, 2, 4, 5 and re-state 3 and 6 in the body before asking anyone to review this seriously.

[alecantu7]

Review findings addressed — c5a31ea

#	Finding	Outcome
1	Sync twin still raised blind	fixed
2	Retry latch built on stochastic evidence	fixed (latch removed)
3	PR body overstated the 2048 headroom	fixed (body rewritten)
4	Session tests didn't test the detection	fixed (+ mutation-verified)
5	Test fake coupled to internal call count	fixed
6	Latency unmeasured	fixed (measured, in body)
7	Verdict accuracy under CoT suppression	not fixed — follow-up, needs a labelled set
8	Doomed call after the latch	moot — latch is gone

1. Shared classifier (the actual defect)

Detection moved out of client.py into structured.py as raise_missing_tool_call, called by both the async LLMClient._generate_object_with and the sync _ScratchpadLLM.generate_object (scratchpad_boot.py). The blind ValueError("LLM did not return structured output.") is gone. That path is the one handed to model-written scratchpad code with a caller-chosen max_tokens, so it was the more exposed of the two. A test now asserts the sync path uses the shared classifier and that the old raise is absent, so the parity structured.py exists to guarantee is enforced rather than assumed.

2. Latch removed

It assumed a double truncation proves the model can never fit a verdict. The measurement says otherwise — 16 identical calls spent 245…1654 output tokens, a 6.7x spread for the same request. One truncation is a tail sample, so latching the retry off would have reintroduced silent stops on later turns for a model that usually succeeds. Deleted rather than tuned: the retry only fires when the first attempt truncates (0 of 12 at 2048), so there was little to optimise. The reasoning now sits in a comment next to the budget constant so nobody re-adds it.

4. The end-to-end test, mutation-verified

New test_truncation_retry_through_the_real_client wires a fake provider through the real LLMClient into the session, so provider → client → session is the real code path. Proof it has teeth — breaking the classifier (truncated = False) fails it:

FAILED test_no_tool_call_at_the_cap_is_reported_as_truncated
FAILED test_both_provider_dialects_for_truncation[length]
FAILED test_both_provider_dialects_for_truncation[max_tokens]
FAILED test_stop_reason_length_is_honoured_below_the_cap
FAILED test_truncation_retry_through_the_real_client   <-- the new one
5 failed, 5 passed

Also added a defensive test that a response with no usage doesn't blow up the classifier (it reports truncated=False — the safe direction, no retry bought without evidence).

Testing after these changes

12 tests in the file; full unit suite 1326 passed, 17 skipped, with the same 2 pre-existing test_chat_scratchpad failures that are present on clean staging. tests/e2e/scenarios/test_loop_safety.py 6 passed.

Still open

#7 — the no-preamble clause suppresses chain-of-thought on models trained to emit it. 12/12 identical verdicts shows stability on one scenario, not accuracy across scenarios. Measuring that properly needs a labelled set of turns run with and without the clause, per model (same shape as the ENG-381 ground-truth fixtures) — too big for this PR, and it doesn't gate the fix: today those models return no verdict at all, so any correct-ish verdict is an improvement. Worth its own ticket if you want it tracked.

[alecantu7]

Third pass — found the same bug behind a second door (19caf53)

The hole: truncation inside the tool call

Everything so far assumed truncation happens in the prose before the forced call. It can equally run out inside the call's JSON arguments — and that path bypassed the entire fix:

1. safe_parse_tool_input repairs what it can and sets parse_error, so the call arrives non-empty but incomplete (provider.py:203).
2. if not response.tool_calls: is therefore false — the classifier never runs.
3. unwrap_structured_response raises a bare ValidationError.
4. The verifier's except Exception treats it as a hard failure → no retry → fail-safe → silent stop.

Same user-visible bug, same root cause, different door — and my fix walked right past it. Given output length varies ~6.7x per call, a model that narrates ~2000 tokens and then starts emitting the call is exactly the case that lands mid-arguments.

Fixed in both the async client and the scratchpad's sync twin: on a failed unwrap, if the response shows truncation, raise the same retryable StructuredOutputError instead of the validation error. looks_truncated is now extracted so one evidence rule covers both shapes, and raise_missing_tool_call became raise_unusable_tool_call (message adapts, typed NoReturn).

Importantly, the mirror case is guarded too: a malformed tool call with budget to spare still propagates as a validation error. A genuine schema bug must stay visible rather than be relabelled truncation and buy a retry that can't help — test_schema_mismatch_under_budget_is_not_disguised_as_truncation pins that.

My own parity test was weak

test_shared_classifier_is_used_by_both_structured_paths was a substring search over the source — a passing mention in a comment would have satisfied it, which is close to useless for the exact drift it's meant to prevent. Now it parses the AST and asserts the call is inside the sync generate_object function body.

Two things checked, both fine

• Import weight / cycles: structured.py now imports provider at module scope, and it's imported inside the scratchpad subprocess. anton/core/llm/__init__.py is empty and provider.py is dataclasses + abc only — no cycle, no new cost at boot.
• Exception across the subprocess boundary: scratchpad errors cross as formatted tracebacks, not pickles, so no custom-attribute loss. Side benefit: the richer message ("truncated: 4096/4096 output tokens spent before the call was complete") now reaches the model itself, which can act on it — where "LLM did not return structured output." gave it nothing.

Testing

15 tests in the file, mutation-verified: breaking looks_truncated fails 7 of them, including both end-to-end shapes (no_tool_call and damaged_tool_call, now parametrized through the real LLMClient).

FAILED test_tool_call_truncated_mid_arguments_is_retryable
FAILED test_truncation_retry_through_the_real_client[no_tool_call]
FAILED test_truncation_retry_through_the_real_client[damaged_tool_call]
... 7 failed, 8 passed

Full unit suite 1329 passed, 17 skipped, with the same 2 pre-existing test_chat_scratchpad failures present on clean staging. All 36 e2e scenarios pass (ran the whole tests/e2e/scenarios directory this time, not just loop-safety).

Still open

Unchanged from the last pass: verdict accuracy under chain-of-thought suppression is unmeasured. Needs a labelled set run with and without the no-preamble clause, per model. Doesn't gate this fix — today those models return no verdict at all.

[alecantu7]

@torrmal review request — this is the other half of ENG-1079, so a quick orientation on how it relates to your #276.

Short version: your diagnosis was right, but the trigger sits one layer below where the ticket points. The verdict call asks for a forced tool call with max_tokens=256. Models that narrate before acting — mindshub_air/kimi (Fireworks kimi-k2p6) and deepseek — spend the whole budget on prose and never reach the call, so generate_object_code raises and the fail-safe turns it into a fake COMPLETE. Measured in prod Langfuse: 171/687 verdict calls (24.9%), 145/147 on mindshub_air (98.6%), every one with completion_tokens == max_tokens exactly.

Two things that may change how you read the ticket:

1. ENG-716 didn't introduce it. The old free-text verifier also passed max_tokens=256, and 116/554 (21%) of its verdicts were unparseable for the same reason. ENG-716 changed the consequence — from INCOMPLETE/"keep working" to except/silent stop — not the frequency.
2. STUCK isn't inflated — 1.9% post-ENG-716 vs 2.5% before. So the "move the verdict off the cheap model" line isn't supported: all nine non-narrating aliases, cheap ones included, answer correctly at 256. It's budget, not capability.

Merge order — this one first, then #276. Not stacked; both branch from staging. Your fail-safe is still needed (a disabled-model 403 or a provider 500 can't be fixed by a budget — see ENG-1083), but on its own it would show "an internal check failed, how would you like to proceed?" to mindshub_air users on ~99% of turns. With the ceiling fixed it becomes the rare fallback it's designed to be. Both rewrite the same try/except; this PR leaves the fail-safe in an else: branch so your honest-diagnosis body drops straight into it.

One thing to carry into #276: the truncation check can't use finish_reason. The gateway returns "stop" while handing back exactly max_tokens for 9 of 11 aliases (ENG-1082, two-line fix in mindshub_inference). Detection here compares completion_tokens >= max_tokens instead.

Where I'd focus your review:

• session.py — the two-budget retry loop and whether 2048/4096 is the right shape given the measured distribution (245–1654 tokens, 6.7x spread for an identical request; I removed an earlier per-session latch precisely because of that variance).
• structured.py — looks_truncated / raise_unusable_tool_call are shared with the scratchpad's sync twin. That parity is the part I'd most want a second opinion on.
• Whether the no-preamble clause in the verifier prompt risks verdict accuracy on CoT-trained models. 12/12 identical verdicts shows stability on one scenario, not accuracy across cases — the one thing here I haven't measured properly.

I've also left a review on #276. One confirmed item there: the stale-reply history bug you fixed for the new path still exists on the STUCK and budget-exhausted paths, both of which #276 edits — I reproduced it (diagnosis absent from history, pre-verification reply appended twice).

Follow-ups already filed: ENG-1082 (gateway finish_reason), ENG-1083 (qwen/fable advertised but 403/404), ENG-1084 (seven more generate_object* sites with the same ceiling bug at 512–1500 tokens, all silently swallowed — memory and cerebellum are quietly no-ops for these users today).

[pnewsam]

Code Review

Verdict: REQUEST_CHANGES

The truncation recovery itself is well designed and thoroughly tested, but the new generic verifier error log can write model-generated conversation data or provider response details into ordinary application logs. That privacy risk should be removed before merge.

Summary of findings

Severity	File	Issue
Major	anton/core/session.py:2746	Logging an arbitrary exception message can expose conversation content or PII.

What's working well

The retry is bounded, truncation detection covers both provider stop-reason dialects, the sync and async structured-output paths share the classifier, and the provider-to-session behavior has meaningful unit and subprocess-level coverage.

---

Reviewed by Codex · alejandrocantu/eng-1081-completion-verifiers-256-token-ceiling-truncates-narrating → staging · 2026-07-27

[pnewsam]

Major: Avoid logging str(exc) for arbitrary verifier failures.

Pydantic validation errors embed the rejected input_value, which here is model-generated verdict data derived from the user's conversation. Provider exceptions may similarly include response bodies. This can copy conversation content or PII into ordinary application logs and contradicts the PR's security claim that the new logging contains only token metadata.

Please log safe structured fields only—such as exception type and validation locations/error codes—without including rejected inputs or arbitrary exception messages.

[alecantu7]

Agreed, and thank you — this was a real leak and I introduced it while arguing the opposite case on #276 (that a bare verdict=ERROR hides the cause). Fixed in 9122e65.

_safe_error_detail(exc) now emits the exception type plus, for validation errors, only the failing field locations and error codes — errors(include_input=False, include_url=False, include_context=False), and it reads only loc/type even on the pydantic-v1 fallback, so msg/input/ctx can never be quoted. Provider errors reduce to type + status_code. So ValidationError(status:literal_error) instead of a message containing the rejected verdict text.

Your finding also applies to a line I was not touching, and more broadly. The pre-existing summary log at session.py:2794 wrote reason=%s — the model's free-text justification of the verdict, derived from the user's conversation — on every verdict, not just failures. Same exposure class, larger volume. I dropped it in the same commit: status and counters stay (that's what makes the verdict distribution measurable), and the reason is still in the Langfuse trace, where conversation content already lives under the appropriate controls. Shout if you would rather that were a separate change — it is a pre-existing line and I did widen the diff to include it.

Regression test plants a recognisable value in the rejected input and asserts it never reaches the log detail.

[pnewsam]

@alecantu7 Some of these comments are getting very long -- maybe worth a quick prompt to shorten wherever possible?

[alecantu7]

Fair, and the comments were long. Trimmed across the diff in 9122e65 — structured.py, the budget/prompt constants in session.py, client.py and scratchpad_boot.py — same facts in roughly half the lines. The 17-number token distribution is now one line rather than a block, and the "why no latch" rationale is one sentence.

I kept the numbers that justify the constants (2048 vs 1024, the 6.7x spread) because they are the load-bearing part — without them the next reader has no way to tell 2048 from a guess, and it was originally 1024 until measurement said otherwise. Happy to cut further if any specific block still reads as too much.

[entelligence-ai-pr-reviews]

---

Confidence Score: 5/5 - Safe to Merge

Safe to merge — safe to merge.

Key Findings:

• No critical or significant issues found

[alecantu7]

Review addressed — 9122e65

Major (privacy): fixed. str(exc) is gone from the verifier logs. _safe_error_detail emits the exception type plus, for validation errors, only failing field locations and error codes. Details in the thread — including that the same leak class existed in a pre-existing line (reason=%s, the model's free-text justification, logged on every verdict), which I dropped in the same commit and flagged there in case you'd rather it were separate.

Style: done. Comment blocks trimmed across the diff, ~half the lines, numbers that justify the constants kept.

3 regression tests added (18 in the file), 1332 unit tests pass, all 37 e2e scenarios pass.

---

Separately: validated against the full model catalog

Since the review I ran the verifier payload against all 19 advertised MindsHub models (the catalog grew from 14 to 20 today, with labels and new ids).

Valid verdict today: 12/17 reachable → 16/17 with this PR. Four models only work because of it:

model	today at 256	with the fix
MindsHub Air	no tool call (capped)	WAITING in 313t
Qwen3.7 Plus	no tool call (capped)	WAITING in 1054t
DeepSeek V4 Pro	no tool call (capped)	WAITING in 272t
GPT 5.3 Codex	tool call but unparseable args	WAITING in 181t

Two things worth noting for the review:

• GPT 5.3 Codex is the mid-arguments truncation case in the wild — a tool call whose JSON is cut off at exactly 256 tokens. That's the specific hole the third commit on this branch closed; without it Codex stays broken. (New since this morning: the catalog now applies a default reasoning effort to Codex, and reasoning tokens eat the budget.)
• Qwen needed 1054 tokens, independently confirming that 1024 would have been too small — I'd argued that from Kimi's 1,654 alone. Max across the whole catalog is 1054, so 2048 has ~1.9x headroom.
• GLM 5.2 passes today at exactly 256 tokens — a latent narrator that happens to fit, and would start failing on any longer transcript.

Three models this PR does not fix, all outside its mechanism (capability, not budget):

• Kimi K3 → 400 "tool_choice 'specified' is incompatible with thinking enabled" — forced tool choice rejected outright.
• Muse Spark 1.1 → 400 "named tool_choice is not supported".
• Claude Fable 5 → returns arguments: "{}" in 8 tokens, identical at 256 and 2048; works with a simplified schema, so something about the real _VerifierVerdict schema (likely its long enum descriptions) defeats it.

For those three the verifier lands on the fail-safe, which is a concrete argument for landing #276 after this one rather than dropping it. Filing them separately — they need their own fix and no open PR covers them.

[pnewsam]

Code Review

Verdict: APPROVE

The previous privacy finding is resolved. Verifier failures now use safe structured error details without logging arbitrary exception messages or model-generated justification text, and the regression tests explicitly cover rejected values, provider response bodies, and the generic fallback. I found no remaining blocking issues.

Summary of findings

No issues found. Code looks good to merge.

What's working well

The truncation retry remains bounded and evidence-based, sync and async structured-output paths share the classifier, provider-to-session behavior is covered end to end, and all required CI and CodeQL checks are green.

---

Reviewed by Codex · alejandrocantu/eng-1081-completion-verifiers-256-token-ceiling-truncates-narrating → staging · 2026-07-27

[alecantu7]

Retargeted at `main` as a hotfix in #278 (team convention for hotfixes: PR to main, then merge main back into staging, then close the staging PR).

Same 6 commits cherry-picked onto `origin/main` — zero conflicts, and the resulting files are hash-identical to this branch, so #278 is byte-for-byte this change rather than a re-implementation. All files touched here are identical between `main` and `staging` (the two branches differ in exactly one unrelated file), and on the main base it is 1334 unit tests / 0 failures plus 37/37 e2e scenarios.

Leaving this open until #278 lands, then closing it as superseded once the main→staging sync brings the content into staging. Note the automatic sync-back (`staging-unfreeze.yml`) is gated on `head.ref == 'staging'`, so it will not fire for the hotfix — that merge needs doing by hand.

@pnewsam your privacy fix is included (`9122e65`, cherry-picked as `058f705`). This PR remains the review of record — the evidence, the token distribution behind 2048, and the 19-model matrix all live in this thread.

[alecantu7]

Superseded by #278, which is merged and released as v2.26.7.27.2 (auto-release, PyPI publish and live e2e all green). Closing this in favour of the hotfix path.

Provenance: #278 is the 7 commits from this branch cherry-picked onto main, verified hash-identical to this branch across every touched file — session.py, client.py, provider.py, structured.py, scratchpad_boot.py, test_verifier_truncation.py, stub_server.py, test_loop_safety.py. Nothing was reworked on the way to main, and origin/main now matches this branch byte-for-byte on all of them.

This thread stays the review of record for the change: the prod measurement (171/687 verdicts, 98.6% on mindshub_air), the token distribution behind 2048 (245–1654 across 16 calls), the four adversarial passes, @pnewsam's privacy finding and its fix, and the 19-model validation matrix all live here rather than on #278.

staging does not have the fix yet — it arrives via the sync PR #279 (main → staging), which is required because staging-unfreeze.yml is gated on head.ref == 'staging' and will not run for a hotfix branch. If #279 is not merged, staging is left without the fix, so that PR is the thing to watch rather than this one.

@torrmal for #276: rebase onto staging after #279 lands, so the ceiling fix is underneath the honest-diagnosis fail-safe. My review notes on #276 (including the reproduced stale-reply duplication on the STUCK and budget-exhausted paths) are still open.