[Feature] Adaptive context and robust daemon - #28
Merged
Conversation
P0 + P1 of the compression relaxation plan. The compression *strategy* was fine — its ratio triggers already follow the window — so this fixes the inputs it works from: the denominator, and the serial truncation chain's tightest link. P0 — the capability registry must not shrink the window. llm_context_length is a configured *budget* and the registry holds model *capability*, so min() of the two is the right semantics. The problem was trusting family-wide patterns: r"qwen" carried the 131K that was current when the row was written, so any later 1M-class model in that family ran on 13% of its window — with every compression ratio computed against that wrong denominator. The failure mode was inverted too: an unrecognised model got the full default while a recognised one got clamped by stale data. ModelCapabilities now declares whether its context_length is `authoritative`. Version-specific rows are (they still cap an oversized budget, so configuring past a real limit is still caught); family rows and the default fallback are not, and defer to the configured budget. Family rows no longer assert a length at all — a test guards the table against re-adding one. A length learned from a real response counts as authoritative. Model names always outrun a static list, so this is a deliberate stance: keeping the table current is a convenience, never a correctness requirement. That is also why no entry was invented for newer models — a made-up number is exactly the defect being removed here. Overshooting a real limit is recoverable (the provider reports overflow and recovery compresses); silently running at a fraction of the window is not, because nothing surfaces it. Status reporting now uses the engine's effective budget rather than the configured one, so the bar cannot claim a window compression is not using. P1 — the tool-result budget could only shrink. The call site used min(max_tool_result_chars, context_length // 20), which pins every large window to the 3000-char base: on 1M that is 0.3% of the window, and since the truncation chain is serial it decided what reached the model no matter how much room was left. Replaced with adaptive_tool_result_chars(), which scales both ways — 1M now yields 25K, 128K stays ~3.2K (divisor chosen to avoid regressing existing setups), 32K still contracts, bounded and monotonic throughout. Evidence and trim ceilings were also low enough to bind almost immediately on 1M (8K/50K -> 24K/120K), and shell stdout/stderr capture moved to 40K/20K because build logs routinely exceed 10K and the truncated tail is usually where the failure is. Two governance tests asserted ceiling literals; they now bound against the constants, so the contract stays "bounded and monotonic" as ceilings rise. Verification: the first sabotage run passed, which exposed two blind spots in my own tests — the shipped family rows carry no length so min() was harmless against them, and the formula was tested without its call site. Added a stale non-authoritative entry case, an authoritative/non-authoritative pair that isolates the flag, a table guard, and two tests that drive _sync_engine_runtime_budget itself. Re-running the sabotage now fails 4 tests. Suite 1374 -> 1410 passing, ruff clean. Not addressed here (P2-P4): loss-free stages before LLM summarisation, cache prefix stability vs PCD tool-set churn, and externalising long-task state.
Intended to be P2 (move loss-free stages ahead of LLM summarisation), but that turned out to be already done: TrimStage is the first stage and is explicitly zero-cost (dedup by hash + truncate), Summarize is token-driven behind an anti-thrashing guard, and summarize_fn/archive_fn are both wired. There was no work left worth doing there, so this addresses the next real gap instead — estimate accuracy, which matters more as windows grow. The gate decision runs on a character heuristic (CJK 1:1, Latin 4:1). Its error scales with the window: on a 1M budget a 15% miss is ~150K tokens, enough to either overrun the 0.92 hard gate or waste a large slice of the window. Provider prompt_tokens was already captured and already replaced the estimate for cross-turn use, but nothing fed it back into the estimator itself. ContextBudgetEstimator now learns its own correction from that signal (EMA, clamped to 0.5-2.5, small prompts and outlier ratios rejected because provider-side caching and injected system content distort a single sample). No tokenizer dependency: a per-vendor tokenizer goes stale exactly the way the capability table did, and this adapts to whatever model and language mix is actually in use. With zero observations behaviour is bit-for-bit unchanged. Calibration happens before _record_provider_usage overwrites the snapshot, since that overwrite is what destroys the (estimate, actual) pair, and a snapshot that already holds a provider count is skipped so the estimator cannot calibrate against its own output. One defect found by measuring rather than reasoning: the first version fed the *corrected* estimate back for comparison, so the observed ratio approached 1.0 precisely when calibration began working and the factor drifted back (0.649 -> 0.802 over six rounds). The factor is now divided out first, and a regression test pins that it does not creep toward 1.0. Measured after the fix: residual 54.1% -> 0.0% when the heuristic overestimates, and the underestimating direction converges too. 12 new tests including the wiring and a failure-isolation case; reintroducing either defect fails 4 of them. Suite 1410 -> 1422 passing, ruff clean.
One mistyped attribute name made the agent unusable for every turn, and the
reason it was both unusable and undiagnosable is the path the error took rather
than the error itself.
`_calibrate_budget_estimator` read `self._context_window_controller`, a name the
engine never defines (the controller is `_context_controller`); it appeared
exactly once in the tree, at the point of use. Because the read sat past two
early returns, it only fired once a turn had a real context estimate — which is
every substantive turn, and never in the suite.
From there:
AttributeError("... has no attribute '_context_window_controller'")
-> raised inside the LLM call's try block, which also held the post-response
bookkeeping, so a local bug entered the provider-error path
-> ErrorClassifier matches on message text and saw "context"
-> category context_overflow, recoverability auto_recover
-> three rounds of compression, provider failover and credential rotation
against a context that was fine
-> fourth round: nothing left to try -> "No applicable recovery strategy
found" as the entire user-visible answer
-> that branch logged no traceback, and the audit sink was constructed with
path=None, so nothing survived the turn
Reproduced offline: three context_compress decisions then the exact halt message,
matching the incident log's api_calls=4, tools=0, latency=114s.
Fixes, in the order the failure travelled:
- The attribute name.
- `_record_llm_call_telemetry` now owns usage recording and calibration, absorbs
its own failures into a warning, and is called after the provider call's
try/except rather than inside it. Three call sites narrowed; one of them also
bypassed calibration entirely by assigning `_last_context_tokens` directly.
- Exceptions that mean "LeapFlow has a bug" (AttributeError, TypeError,
NameError, KeyError, IndexError, ImportError, AssertionError,
NotImplementedError) are classified by type into a new non-recoverable
`internal_defect` category before the provider taxonomy is consulted. Matching
Python's exception hierarchy is exact; matching message text is what turned a
typo into a context overflow.
- Both silent recovery entry points now log with exc_info, the audit sink is
constructed with the profile layout's audit path, and every terminal decision
carries an InteractionRequest, so a stopped turn states what failed and what to
do instead of emitting internal jargon.
The test gap mattered as much as the bug: the calibration wiring tests built the
engine with `object.__new__` and assigned `_context_window_controller` themselves,
asserting the same wrong name the code used. Mocking the attribute made the one
thing those tests claimed to cover unobservable. They now use the real name, and
three new guards close the hole: a real-engine calibration test on the production
path, an architecture contract that every `self._x` the engine reads is assigned
somewhere (167 reads checked; reintroducing the typo fails it), and
test_internal_defect_reporting for the classification, budget, message and audit
contracts.
Verified beyond the suite: a single-turn answer and a tool-using turn both
complete in-process (7.4s, 1 tool) where this shape previously halted after
~2 minutes.
1432 passed, ruff clean.
Note: a running leapd predates this fix and must be restarted to pick it up.
Two TUIs in different workspaces left the second one unusable: every turn was
rejected with "Session '...' is bound to workspace A; current request uses B",
and the advice it gave ("start a fresh TUI session") could not work.
The workspace guard was right. What was wrong is how the second client came to
hold the first client's session id:
- `SessionRegistry.most_recent()` returns the most recently active session of any
client, ignoring workspace and client identity.
- `resolve_session_engine(ctx, "")` falls back to it when no session is named.
- `status()` took no session id at all, so every caller got that fallback — and
with it another client's `session_id` and context figures. `_chunk_from_event`
had the same fallback whenever an engine was not passed explicitly.
- The TUI then adopted it unconditionally: `active_session_id =
str(metadata["session_id"])`, on startup, on every refresh, and on every stream
chunk.
So TUI#2 adopted TUI#1's session on its first status poll, sent it with its own
workspace on the next turn, and was refused — permanently, since a fresh client
re-adopts the same id immediately. The second, quieter symptom was TUI#2's status
bar showing TUI#1's context usage.
The fallback arrived with 0291d19 and was wired into status/stream metadata by
8061fc1 to fix a status bar stuck at 0/1M. Both share one root: client-visible
runtime state was resolved by "most recently active" rather than "this caller".
Fixed in the order the identity travelled:
- The client accepts a reported session id only when it matches its own or it has
none yet (first assignment, or an explicit --resume). This alone stops the
outage even if the daemon regresses.
- `status()` takes the caller's `session_id` and resolves that session; with none
named it reports no session identity and no per-session figures rather than a
substitute. Client, protocol, and the TUI's three call sites pass it.
- `_chunk_from_event` requires the producing engine; the fallback is gone. Its
only caller already passed one, so the fallback was pure risk.
- `most_recent()` is now `most_recent_any_client()`, documented as valid only for
aggregate views, so it cannot be mistaken for "the caller's session" again.
- The mismatch message names the one legitimate cause (--resume from another
workspace) instead of telling the user to do what they already did.
Verified with real Settings: with TUI#1 active in workspace A, TUI#2 polling
status keeps its own session, its turn is accepted in workspace B, and both
sessions coexist. Each client's status reports its own usage (5000 vs 99), so the
leak is closed without regressing the status bar to zero — the case 8061fc1 fixed.
Four existing tests asserted the leaky behaviour and were corrected: one expected
a session-less `status()` to report context usage, another exercised the
`_chunk_from_event` fallback. New `test_multi_client_session_isolation` covers
workspace binding, the rename, per-caller status, RPC plumbing, client adoption,
and a source-level guard against the unconditional assignment returning.
AGENTS.md gains nine contracts across architecture, recovery, and testing. The
important one is that concurrent TUIs in different workspaces are a supported
scenario, not an edge case: a change to session routing, status, stream metadata
or the client lease is not verified until two instances in two workspaces have
been exercised together. The recovery entries record the previous commit's
lessons (a local defect is never a provider failure; terminal decisions must be
actionable; recovery must leave evidence), and the testing entries record the two
blind spots that let both incidents ship green — a test may not fabricate the
wiring it claims to cover, and multi-client behaviour needs multi-client tests.
1442 passed, ruff clean. Confirmed by hand in two live TUIs.
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.
No description provided.