fix(mental-models): trace delta-ops call and decouple its completion cap (#3421) - #3424
Open
nicoloboschi wants to merge 1 commit into
Open
fix(mental-models): trace delta-ops call and decouple its completion cap (#3421)#3424nicoloboschi wants to merge 1 commit into
nicoloboschi wants to merge 1 commit into
Conversation
…cap (#3421) A delta-mode refresh whose structured-delta LLM call fails to parse wedges: the #3112 window guard (correctly) preserves content and refuses to advance the watermark, so the next trigger re-reads the identical window, and at temperature 0 the delta call reproduces the identical malformed output — parse fails identically forever with no self-recovery. Two root-cause fixes, deliberately NOT a fall-back to full re-synthesis (that abandons delta mode's purpose and #3112 already rejected it): - Decouple the delta transport cap from the document budget. Passing the doc-sized delta_max_tokens as max_completion_tokens truncates the ops JSON on thinking models (reasoning tokens eat the budget); the cut-off JSON then fails the parse deterministically. Use reflect_max_completion_tokens (uncapped by default), same decoupling reflect's synthesis got in #3365/#3389. delta_max_tokens stays as the prompt-level budget hint. - Trace the delta call. It ran on the raw _reflect_llm_config outside reflect_async's trace context, so its LLM calls were never written to the trace table — the blind spot that made these failures impossible to diagnose. Wrap it in with_config(bank_id, operation, mental_model_id).
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
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.
Problem
Fixes #3421. A delta-mode mental-model refresh whose structured-delta LLM call fails to parse wedges permanently:
parse_delta_operation_listraises (truncated/malformed JSON, or every op rejected →DeltaAllOpsInvalidError) →mode_fallback_reason = "delta_ops_failed".last_refreshed_at, so no observations are skipped.temperature = llm_temperature_consolidation(default 0.0), so the failure is deterministic. The next trigger re-reads the same window, reproduces the same malformed bytes, and fails identically — the "27 repeated errors onuser-preferences" with no automatic recovery.We deliberately do not fall back to full re-synthesis (the issue's suggested fix): that abandons the whole point of delta mode, and #3112 already rejected it for transient errors. Advancing the watermark on a partial apply is exactly the data loss #3112 exists to prevent. Instead this fixes the two things that actually cause the deterministic parse failure and its invisibility.
Changes
1. Decouple the delta transport cap from the document budget.
The delta call passed the doc-sized
delta_max_tokensasmax_completion_tokens. On thinking models the provider's output budget is consumed by reasoning tokens first, truncating the ops JSON mid-string — and at temperature 0 that truncation is deterministic. Now the transport cap isreflect_max_completion_tokens(uncapped by default), the same decoupling reflect's synthesis received in #3365 / #3389.delta_max_tokensstays as the prompt-level budget hint (max_output_tokens), never the transport cap. Operators who want a hard ceiling setHINDSIGHT_API_REFLECT_MAX_COMPLETION_TOKENS(applies to synthesis + delta, consistent).2. Trace the delta call.
It ran on the raw
_reflect_llm_configoutsidereflect_async's trace context, so its LLM calls were never written to the trace table — the blind spot that made #3421's failures impossible to diagnose post-mortem (and why we can't be 100% sure whether truncation or schema-reject fired in the reported incident). It's now wrapped inwith_config(bank_id, operation="mental_model_delta_ops", metadata={mental_model_id}), so upgrading a wedged deployment will surface the raw failing output.The intent is to ship both, upgrade, and let the now-visible traces confirm the failure class — while the completion-cap fix removes the most likely deterministic cause immediately.
Test
test_delta_call_is_traced_and_uses_decoupled_completion_capasserts the delta call (a) runs inside amental_model_delta_opstrace bound to the bank + mental model, and (b) usesreflect_max_completion_tokensas its transport cap, notdelta_max_tokens.Ran
TestDeltaRefreshPlumbing+ dry-run + per-op-concurrency suites (68 passed); ruff + ty clean on changed files.