Every API call in the tool loop persisted its token/cost delta by
calling SessionDB.update_token_counts() synchronously on the turn
thread — a BEGIN IMMEDIATE sessions UPDATE plus a session_model_usage
upsert, measured in production at p50 3.3ms / p95 70.4ms per call and
up to 299ms against a cold multi-GB state.db. The tool loop stalls for
that long between calls, N times per multi-tool turn.
SessionDB gains queue_token_counts(): same signature and semantics as
update_token_counts(), but the critical path is a deque append plus a
condvar notify. A lazily started daemon thread applies deltas in
enqueue order through the existing update_token_counts ->
_execute_write path, so the established self._lock / BEGIN IMMEDIATE /
jitter-retry discipline is unchanged. When a backlog forms, adjacent
same-route incremental deltas coalesce into one UPDATE: token and
api-call fields sum, cost fields sum None-preservingly (an all-None
run stays None so COALESCE keeps the stored value), and absolute=True
deltas never merge and act as ordering barriers. Route equality is
required for a merge because those fields feed COALESCE backfill, the
last-non-None-wins status fields, and the per-model usage attribution
key — a merged apply is row-equivalent to sequential applies.
Correctness and durability:
- flush_token_counts() gives read-your-writes to token/cost readers
(get_session, list_sessions_rich, _get_session_rich_row,
list_gateway_sessions, InsightsEngine.generate) — a plain attribute
check when nothing is queued. The writer sets its busy flag before
popping the queue so the lock-free fast path can never miss an
in-flight batch.
- update_session_model / update_session_billing_route /
update_session_meta write the sessions row synchronously, bypassing
the queue, so they flush it first: a still-queued first-of-session
delta carries the pre-switch route, and applying it after the switch
UPDATE would trip the first_accounted_route branch (api_call_count
== 0 plus a route mismatch) and resurrect the old model/provider.
- AIAgent._persist_session flushes at turn finalize and every
error-exit persist point; close() stops and drains the writer before
the WAL checkpoint; an atexit hook (registered on first enqueue,
unregistered on close so closed instances are not pinned until
interpreter exit) drains at shutdown. Worst-case crash loss is the
in-flight call's delta — the same window as the old inline write.
- A flush trusts a live stop-flagged writer (its loop drains before
exiting) and only drains on the caller's thread when the writer is
dead or never started, claiming the same busy flag so concurrent
flushes wait instead of racing an in-flight batch.
- After close() has stopped the writer, queue_token_counts applies the
delta inline instead of parking it on a queue nothing will drain; a
closed-connection failure then raises at the call site, which
already guards for it, exactly like the old synchronous path.
- Writer apply failures are logged and never raise into a turn; the
writer thread survives and keeps applying.
Call sites switched to the queue: the per-call site in
agent/conversation_loop.py and both codex app-server sites in
agent/codex_runtime.py. In-memory per-turn counters
(agent.session_estimated_cost_usd etc.) stay synchronous, so live turn
displays never see the queue.
Tests: tests/agent/test_async_token_accounting.py (19 tests: enqueue
ordering, absolute-as-barrier, backlog coalescing with exact sums,
coalesced-vs-sequential row equivalence, merge unit rules, None-cost
preservation, read-your-writes, flush vs stop-flagged/concurrent
drains, inline apply after writer stop, close/atexit durability,
_persist_session drain, writer failure isolation);
tests/run_agent/test_token_persistence_non_cli.py updated to the
queue_token_counts contract.
The auto-continue recovery note was typed only after run_conversation
returned, so its row sat untyped for the whole turn — and permanently
when the continuation was itself killed, which is the case it exists
for. persist_user_display_kind stamps the type on the live message
before the crash persist writes it, in the same insert as the content.
The flush also carries display_metadata through, which it was dropping.
A mid-stream steer persists an interrupted-turn checkpoint so the model knows
its reply was cut off. That scaffolding — "[This response was interrupted by a
user correction.]" and the "Visible response before the interruption:" header —
was written straight into message content, so every reload painted the raw
machinery as an assistant bubble (and merged it into the preceding tool-call
bubble). Steered transcripts became unreadable.
Reuse the existing display/replay split instead of inventing new surface:
- Carry the scaffolded form in the server-only api_content sidecar (the exact
bytes replayed to the provider), keep content the user's/agent's real words.
- When nothing reached the screen there is no clean form, so mark the row
display_kind=hidden — replayed to the model, dropped by every transcript
surface, exactly like compaction-reference rows.
- Honor display_kind=hidden in the gateway's _history_to_messages projection
(it only sniffed the [System: convention), so the checkpoint can't leak
through the live/resume path to the TUI/CLI either.
The model still receives the full interrupted context on the wire; the
transcript shows the partial reply and the user's correction.
A mid-stream steer/redirect cancels only the live model request and queues
the correction for a rebuild. But the retry-wait, error-handling, and
backoff-wait paths all treated the cancellation bit as a hard stop:
clear_interrupt() destroyed the pending correction and the turn died with
"Operation interrupted…" — the user's message silently lost. All three
sites now preserve the redirect and rebuild the iteration from it, exactly
like the InterruptedError handler.
tui_gateway also gets the two suppressions its sibling surfaces already
had: the "Operation interrupted: waiting for model response (…)" sentinel
is cancellation metadata and no longer ships as assistant prose in
message.complete (gateway/run.py and ACP already suppress it), and a
leftover pending_steer returned by the turn is requeued as the next prompt
instead of dropped (cli.py and gateway/run.py already do this).
session.steer now records the correction on the inflight turn like
session.redirect does, so a resume/reconnect mid-turn rebuilds the steered
user bubble instead of losing it.
The concept 'never send a turn that strict wire validation rejects as
empty' was forked across four sites, each with its own predicate and its
own blind spots:
1. build_assistant_message write-time ' ' pad — broke codex commentary
turns (content:'' is a designed state), and a DB-side pad can't
survive _rows_to_conversation's whitespace strip anyway. REMOVED.
2. conversation_loop send-time ' ' pad — main-loop only (summary path
uncovered), ordering-fragile (had to run after whitespace
normalization), assistant-only. REMOVED.
3. stream-stub '[response interrupted]' substitution — defeated the
loop's empty-stub guard (the stub no longer looked empty, entered
history, and the placeholder leaked into the stitched final
response via truncated_response_parts). REMOVED.
4. repair_empty_non_final_messages in sanitize_api_messages — the
unconditional pre-send chokepoint shared by the main loop AND the
summary path, covers user and assistant turns, non-final only,
copy-on-write. This is now the SINGLE OWNER.
The owner's payload predicate (_msg_has_payload) is extended to treat
codex_message_items / codex_reasoning_items as payload, so
designed-empty codex commentary turns are never rewritten on any
api_mode — the failure shape that broke site 1 in CI is encoded in the
owner, not special-cased at a call site.
Tests updated to pin the new contracts: builder stores textless turns
as-is; the empty stream stub stays recognizably empty for the loop
guard; poisoned resumed histories are repaired to the placeholder at
the send boundary; codex item carriers are never rewritten.
Sabotage-verified: unwiring the owner fails 3 regression tests.
Commentary-phase Codex turns persist with content:'' by design (their
text is delivered via the interim assistant callback), and the Responses
wire has no 'assistant must not be empty' validation — padding them
broke test_run_conversation_codex_continues_after_commentary_phase_message
in CI. Both the builder pad and the send-time pad now skip
api_mode=codex_responses. Keying on the ACTIVE api_mode preserves the
repair for codex-written sessions replayed through a strict
chat-completions provider.
A mid-tool-call stream drop with no delivered text produces a
partial-stream stub carrying content:'' and tool_calls=None. The
conversation loop's truncation path appended it to history as
{"role":"assistant","content":""} before the continuation nudge, and
strict providers (Moonshot/Kimi via OpenRouter) reject empty assistant
content with HTTP 400 ("the message at position N with role 'assistant'
must not be empty") on the next replay. Because the message is
persisted, every subsequent turn re-failed — the session was
unrecoverable.
Three layers, smallest blast radius first:
1. conversation_loop (length path): an EMPTY partial-stream stub is no
longer appended as an interim assistant message; only the
continuation user-message is. Stubs that delivered partial text are
still persisted so continuation stitching is unchanged.
2. chat_completion_helpers.build_assistant_message: never serialize a
textless assistant turn with content:'' — pad to a single space, the
same trick as the reasoning_content pad (#15250, #17400). Tool-call
turns are exempt (content:'' alongside tool_calls is accepted
everywhere).
3. conversation_loop send boundary: pad a textless assistant turn's
empty content to a single space AFTER all content-mutating passes
(surrogate sanitize, whitespace normalization, thinking-only drops),
before token estimation. This is the durable repair for sessions
ALREADY poisoned by older builds: the persisted stub rows are rebuilt
to '' on every reload (_rows_to_conversation strips whitespace, so a
DB-side pad can't survive) and only a send-time pad repairs them.
Verified: 485 tests pass across the four affected files; live replay of
a real poisoned session's resumed history against Moonshot via
OpenRouter returns HTTP 200 (was HTTP 400).
A /steer redirect during a thinking phase serialized the streamed
reasoning into the persisted assistant checkpoint ('Reasoning shown
before the interruption: ...'). An assistant turn exposing its own
chain-of-thought reads to Anthropic's output classifier as
reasoning-injection/prefill jailbreak, so every subsequent call on the
session deterministically returned 'Provider returned an empty
response' — and because the checkpoint is persisted and replayed, no
retry, nudge, or empty-recovery branch could ever escape it. Four
sessions were permanently bricked this way in the week of Jul 21-27
(42+ blocked calls; every reasoning-free checkpoint that week was
untouched — same mechanism as the prefill.json incident).
Class fix: streamed reasoning is now display-only state. The
_current_streamed_reasoning_text accumulator is removed entirely
(producer in _fire_reasoning_delta, resets, and init), so no future
path can serialize CoT into replayable content. The checkpoint keeps
only the visible response text; the model regenerates its reasoning on
the retried turn. Invariant documented in _apply_active_turn_redirect.
Regression tests: CoT never appears in either checkpoint shape,
reasoning-only interrupts produce a bare checkpoint, reasoning deltas
stay display-only.
The call-block decoration reads agent._use_prompt_caching / _cache_ttl /
_use_native_cache_layout directly; the redecoration helper wrapped each in
getattr with divergent defaults (e.g. or-'5m' vs verbatim _cache_ttl).
The flags are unconditionally initialized on AIAgent, so the defaults
served only test fixtures and would mask a real init bug as silent
cache-off. Align with the house style.
_peel_moa_guidance hand-implemented the inverse of moa_loop's
_attach_reference_guidance from a different module — a drifting separator
or shape would make the peel silently no-op and put the last cache
breakpoint on the turn-varying guidance block (the #72626 bug class).
Move the inverse into moa_loop.peel_reference_guidance directly adjacent
to the attach, keep a thin wrapper in conversation_loop, and pin the
contract with a round-trip test over all three attach shapes.
Also fix the empty-list residue: peeling a guidance-only content part now
drops the whole message (mirroring the appended-user-message shape)
instead of leaving an empty-content user turn behind.
guidance=None is a real prepared shape (all references failed / silent
degraded policy builds prepared_request without attaching guidance), and
the MoA facade sends prepared['messages'] — not api_kwargs['messages'].
Gating the rebase on 'and guidance' left the stale decoration in the
prepared object for the no-guidance MoA sub-path, so #72626 persisted
there. rebase_prepared_request already handles falsy guidance (copies
messages, skips the attach).
The static-prefix reconstruction pattern (build_system_prompt_parts ->
['stable'] -> startswith gate -> fail-open) existed in three copies:
session restore (conversation_loop), compression keep-prompt path
(conversation_compression), and the new failover redecoration helper.
Hoist it into agent/system_prompt.reconstruct_static_prefix and call it
from all three sites.
Also memoize failed rebuilds per stored prompt (_static_rebuild_failed_for):
the redecoration chokepoint runs at the top of every retry attempt, and a
persistent stable-tier mismatch (restored session whose SOUL.md/skills
changed since save) would otherwise re-run the full prompt build — SOUL.md,
context files, memory I/O — on every attempt of every API call for the
life of the session. A legitimately changed stored prompt retries once.
try_activate_fallback refreshes the cache policy flags for the new
provider, but the retry loop reused the primary's decorated api_messages.
Cache-off→cache-on shipped zero breakpoints; cache-on→cache-off left
stale markers. Strip and re-render at each retry attempt (same chokepoint
as reasoning-echo reapply), peel/rebase MoA guidance so the last marker
stays off the turn-varying block, and rebuild the static system prefix
when caching becomes active mid-turn (#72626).
When the empty-response ladder is fully exhausted (thinking-prefill
continuation, empty-content retries, provider fallback) and the model
produced structured reasoning but never any visible text, deliver a
clearly labeled excerpt of that reasoning instead of a bare '(empty)' —
the reasoning frequently contains the actual answer.
Delivery-only by design: raw chain-of-thought is never promoted to a
normal answer earlier in the ladder (prefill continuation still gets
first crack, retries and fallback still run), transcript persistence
semantics are untouched (the '(empty)' sentinel scaffolding keeps its
replay-safety behavior), and a truly empty exhaustion still returns the
existing terminal.
Idea credit: PR #48795 (@ligl0325) proposed falling back to
reasoning_content on empty content; this lands the safe kernel of that
idea at the one point in the ladder where it is strictly an improvement.
Port from openclaw/openclaw#110518 / #110956: some models reuse one call id
for different tool calls in a single batch (native Kimi Responses replays,
Ollama-compatible endpoints, degraded models at long context). Hermes kept
both calls but the pre-API sanitizer then dropped the later call/result pair
per id (#58327), so the second call's output silently vanished from every
replayed payload — the model never saw it and confabulated.
_uniquify_tool_call_ids renames later collisions to a deterministic <id>_d<n>
suffix at ingestion, before validation/dispatch/history build, so both pairs
survive. Composite Responses ids collide on the call half and keep their
response-item half. Deterministic suffixes preserve prompt-cache prefix
stability (no random UUIDs).
`apply_anthropic_cache_control` runs once per call block, before the retry
loop, and splits the system prompt into `[static prefix, volatile tail]` text
blocks carrying the cache_control breakpoints.
A failover fires *inside* that retry loop, and `_sync_failover_system_message`
assigns a bare string over `api_messages[0]["content"]` to refresh the
`Model:`/`Provider:` identity lines. That drops the block list and both
breakpoints. `convert_messages_to_anthropic` only emits system cache blocks for
list content (`isinstance(content, list)`), so the retried request ships
`system` as one plain string: zero breakpoints, nothing written to cache, and
the whole system prompt re-billed at full write price. The next call misses
too, so a failover costs two full-price prompts instead of one write + one
read.
Measured with the real functions, same history, native Anthropic layout:
normal turn system=BLOCK LIST(2) system breakpoints=2
after failover system=plain string system breakpoints=0
`_sync_failover_system_message`'s own docstring notes this fires on "every
gateway turn, since fallback re-activates per message while the primary is
down" -- so a gateway running on a degraded primary pays it on every message.
Fix: rewrite the decorated blocks in place instead of flattening them.
`rewrite_prompt_model_identity` only touches the LAST `Model:`/`Provider:`
lines, and those live in the volatile tail, so the static prefix stays
byte-identical and its cache entry keeps matching -- the failover retry keeps
both breakpoints AND the warm prefix. Shapes we cannot safely patch fall back
to the existing plain-string assignment.
This is the same class the retry loop already guards against eight lines below
the gap, for reasoning fields:
# api_messages is built once, before this retry loop, while the primary
# provider is active. [...] so the fallback request isn't sent with stale,
# primary-shaped reasoning fields.
agent._reapply_reasoning_echo_for_provider(api_messages)
There was no equivalent for cache decoration.
The whole-prompt scan for "Current working directory:" matched USER project
content, not just Hermes' own host-info line. The prompt embeds AGENTS.md /
CLAUDE.md / .cursorrules in the context tier, which sits AFTER the host-info
block, and line_value() takes the LAST match — so any project file containing
a line starting with that label won the scan.
The comparison then ran runtime state against project prose, a mismatch that
never clears: the stored prompt was rejected on EVERY turn, rebuilding the
system prompt each message and destroying the prefix cache for the whole
session. Strictly worse than the staleness the check exists to catch, and
invisible to CI because no test encoded the invariant.
"last match wins" was only ever safe because Model/Provider/Platform live in
the volatile tier at the very END of the prompt. The cwd line is the one field
read from the STABLE tier near the start, so it needs an anchored read:
locate Hermes' own "User home directory:" line and take the working-directory
line that follows it.
Verified against the real builder on a real AIAgent, not a stub:
- stable cwd, clean project -> prompt REUSED (0 rebuilds)
- stable cwd, AGENTS.md naming the
field -> prompt REUSED (was: rebuild/turn)
- drifted cwd -> rebuilt
- drifted cwd, AGENTS.md naming the
NEW cwd -> rebuilt (prose can't mask drift)
Also re-verified no spurious rebuild across all five cwd-resolution paths
(pinned contextvar, TERMINAL_CWD, launch dir, symlinked workspace,
trailing-slash cwd).
The contributor's fixtures omitted the host-info anchor, so they silently
stopped exercising the cwd path once the read was anchored; reshaped them to
match real prompt structure via a _host_block() helper.
Follow-up to the cherry-picked #68258 base: the cross-session-stable
prefix (_cached_system_prompt_static) was only recorded on fresh
builds, so two paths silently degraded to the legacy single-breakpoint
layout (flagged in review of #68258/#69341/#69704):
- Session restore: gateway surfaces build a fresh AIAgent per turn and
restore the persisted prompt verbatim from the session DB; the static
prefix stayed None from turn 2 onward, flip-flopping the wire layout.
- Post-compression cached-prompt reuse: _invalidate_system_prompt()
clears the static prefix, and the keep-cached-prompt branch never
restored it.
Both sites now reconstruct the stable tier and adopt it ONLY when the
authoritative prompt string literally startswith() it — stable-tier
drift (skills edited, identity changed) falls back to the legacy layout
with the stored bytes untouched. Fail-open on any builder error. The
restore-path rebuild is gated on _use_prompt_caching so non-Anthropic
routes skip it entirely.
Refs #68191
Co-authored-by: JonthanaHanh <92574114+JonthanaHanh@users.noreply.github.com>
Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
The conversation loop normalizes message text right before the API call so
the request prefix is byte-identical across turns -- the stated reason is
KV cache reuse on local inference servers and better cache hit rates on
cloud providers. Cache breakpoints were injected *before* that pass, which
defeats it.
`_apply_cache_marker` rewrites a plain-string `content` into a
`[{"type": "text", ...}]` block. The normalization pass is guarded on
`isinstance(content, str)`, so every message that just got marked is
silently skipped by it and keeps its raw leading/trailing whitespace. A
message is only marked while it sits in the last-3 window, so:
turn N in the window -> marked, content "file1\nfile2\n"
turn N+1 rolled out -> plain, content "file1\nfile2"
The same logical message is sent with different bytes on consecutive
turns. The prefix stops matching at that position -- which is inside the
span the breakpoints were placed to protect -- so the reusable prefix
collapses back toward the system breakpoint on every turn. Tool results
carry a trailing newline almost by default (any shell command output), so
this is the common case, not an edge case.
Move the injection below every message mutation. Besides fixing the
whitespace divergence this stops breakpoints from being spent on messages
that the orphan sweep or the thinking-only drop is about to remove or
merge away -- a marker on a dropped message is a wasted breakpoint out of
the four available.
Nothing between the old and new call sites reads `cache_control`, and the
mutators now see the plain-string shapes they were written against.
Review follow-up for the dropped tool-call recovery (#69630): the
re-prompt pair was tagged _dropped_toolcall_nudge, but that marker was
not part of the ephemeral-scaffolding contract. _persist_session /
_flush_messages_to_session_db would therefore write the synthetic
'issue the actual tool call now' user message (and the narration-only
interim assistant turn) as real transcript rows — a resumed session
could replay the internal retry instruction as user-authored context
and prompt unsolicited tool use.
- Add _dropped_toolcall_nudge to _EPHEMERAL_SCAFFOLDING_FLAGS
(run_agent.py) so both SQLite and JSON persistence skip the pair.
- Add it to _SYNTHETIC_USER_FLAGS (conversation_compression.py) so the
compressor never treats the nudge as human intent.
- Flag the interim assistant half of the pair too, and include the
marker in the finalization scaffolding pop so a genuine turn end
strips the pair from the live transcript (mirrors the
_empty_recovery_synthetic pattern).
- Regression tests: flagged messages classify as ephemeral, the
returned transcript contains no scaffolding, and the turn tail stays
on the real assistant answer.
The initial fix guarded the no-tool-calls else branch, but that branch only
SETS final_response — the turn actually finalizes later, in a separate block
after final_msg is built. Runs that reached finalization via that path exited
without the guard ever running (observed live: a scheduled PR reviewer stalled
at tool_turns=1-2 with zero recovery nudges).
Move the recovery to the finalization chokepoint (right after final_msg is
built), so it catches every path that ends a turn. Single guard now:
- increments a consecutive-stall counter and re-prompts (bounded to 3),
- resets on any successful tool round, and
- resets on a genuine (non-mismatch) turn end,
so it guards each stall independently without capping the whole run and
without looping forever.
Verified live: the scheduled reviewer now recovers through the stalls and
submits real reviews — PR 57800 APPROVED, PR 54826 COMMENTED — one PR per run.
Some providers (observed: claude-opus-4.8 / claude-sonnet-4.5 on GitHub
Copilot, ~2026-07) return finish_reason="tool_calls" while the parsed
tool_calls array is empty — the model signalled it wanted to act but the
payload shipped no call. The conversation loop took the no-tool-calls
else branch, treated the turn's narration as the final answer, and exited
with the task unstarted.
On unattended multi-step jobs this is silent failure: a scheduled PR
reviewer, for example, would narrate "Let me verify the PR..." and stop
at tool_turns=0 every run, never submitting a review, while the job still
reported success.
Fix: in the no-tool-calls branch, detect the provider contract violation
(finish_reason == "tool_calls" with zero tool_calls) and re-prompt the
model to emit the call instead of exiting. Bounded to 3 consecutive
stalls; the budget resets after any successful tool round so it guards
each stall independently rather than capping the whole run. The narration
may live in content or only in the reasoning field (empty content) — the
guard keys on the finish_reason/tool_calls mismatch, so both are covered.
A genuine finish_reason="stop" text turn is unaffected.
Verified live: a scheduled reviewer that died at tool_turns=0 every run
now recovers through 20+ stalls per run and submits real reviews.
Tests: tests/run_agent/test_dropped_tool_call_recovery.py covers the
re-prompt, the empty-content case, the clean-stop control, and the
bounded-loop guard.
After a tool call completes, the conversation loop does to
start the next API call. Between the last (from tool
completion in tool_executor.py) and the next one (at the start of the
next API call), there is a gap. If anything during this gap takes time
— context compression, slow provider prefill, or other post-tool
processing — the combined inactivity window can exceed the gateway's
inactivity_timeout (default 120s). The gateway kills the session and the
user sees 'agent never returns a final response', even though the tool
call itself succeeded in 0.1-0.2s.
Fix: call right before so the gateway
sees a fresh timestamp immediately after tool results are posted,
regardless of how long the follow-up API call takes.
Fixes#69559
Verification follow-up for the #51226 salvage: the host call site guarded
select_context with hasattr(), but the ABC defines a default on every
engine, so the built-in ContextCompressor (and any non-implementing
engine) still paid per-request shallow copies of the conversation
history plus a hook call on every provider request. Identity-check the
bound method against ContextEngine.select_context and return the
request untouched — mirroring the existing base-method short-circuit in
_notify_context_engine_turn_complete — so the default path does zero
work, not just produces an identical result.
Adds two pins: the base no-op is never invoked (patched-to-raise base
stays silent), and ContextCompressor.__dict__ contains neither new verb.
Also registers the contributor email mapping for @chaos-xxl.
Addresses the hermes-sweeper review on #51226:
- _apply_context_engine_selection now passes shallow copies of the read-only
conversation_messages / incoming_message to the hook, so an engine mutating
them in place cannot corrupt persisted transcript state (enforces the
request-only contract, not just documents it). Adds a mutation-regression
test asserting persisted history + incoming message are untouched.
- on_turn_complete docstring: scope the coverage claim to the standard
finalization seam. Some abnormal early-return paths (content-policy block,
provider terminal failure) currently persist+return without finalization and
don't emit the hook; documented as best-effort with a shared-seam follow-up,
rather than over-promising a guaranteed callback for every early exit.
- _apply_context_engine_selection: reject an empty list. all([]) is True, so
a [] returned by a failing/buggy engine previously replaced a valid request
with an empty message list the downstream sanitizers can't restore; now it
falls open to the unmodified request (honors the fail-open contract).
Thanks @johnnykor82 for catching this on #41918's review.
- test: empty list keeps the original request (fail-open regression).
- docs: document select_context()/on_turn_complete() in the public
context-engine plugin guide (were still describing only the old contract).
The on_turn_complete() observation hook is the engine's post-turn signal,
so it should receive the completed turn's canonical token usage when the
host has it, not a hardcoded None. Per @johnnykor82's #41918 contract: the
engine uses prompt/completion + cache_read/write/reasoning buckets to judge
how large/expensive the selected context was before the next select_context().
- conversation_loop.py: stash the most recent provider response's usage_dict
(the same canonical shape fed to update_from_response) on the agent as
_last_turn_usage; reset to None at turn start so turns that never reach a
provider response (early failure / interrupt) forward None, not a stale
prior turn's usage.
- turn_finalizer.py: forward agent._last_turn_usage instead of usage=None.
- context_engine.py: document the usage param contract on the ABC hook.
- tests: cover both ends through the real finalize_turn path — completed turn
forwards the full canonical bucket set intact; no-response turn forwards None.
Co-authored-by: johnnykor82 <johnnykor82@users.noreply.github.com>
Adds the post-turn observation verb as the companion to select_context():
an optional, no-op-default on_turn_complete() called once after the
assistant/tool loop finishes, with the finalized transcript snapshot. Lets
an engine ingest/index/summarize the completed turn to inform the next
select_context(). Wired via _notify_context_engine_turn_complete() from
turn_finalizer.finalize_turn(); fail-open, base no-op short-circuited so
non-implementing engines (incl. the built-in compressor) pay nothing.
This is the request-assembly + observation pair from #41918; with this
commit the PR fully subsumes #41918's two hooks (prepare_request_messages
-> select_context, on_turn_complete) rather than only the selection half.
Co-authored-by: johnnykor82 <johnnykor82@users.noreply.github.com>
Adds an optional, no-op-default select_context() hook to the ContextEngine
ABC, called every turn after the request messages are assembled and before
provider dispatch — independent of should_compress(). Lets an engine select
or replace which context enters the prompt for a single request (retrieval,
topic routing, role/branch switching) without mutating persisted history,
removing the need to abuse should_compress()=True as a per-turn callback.
The host call site (_apply_context_engine_selection) is fail-open: a missing
hook, an exception, or an invalid return value leaves the assembled request
untouched. Additive and non-breaking: the built-in compressor and every
existing engine are unaffected.
Consolidates the per-turn request-assembly surface proposed across #41918,
Related: #36765#41918#24949#47109#50053#23837#25115#29370
agent/tool_executor.py's concurrent tool batch checks agent._interrupt_requested
and aborts the wait early; agent/moa_loop.py's _run_references_parallel had
no equivalent, so a MoA-enabled turn blocked on ThreadPoolExecutor.result()
until every reference model finished or hit its own individual
auxiliary.moa_reference timeout -- there was no way for the user to abort a
live turn mid-fanout.
Thread an optional `agent` parameter through aggregate_moa_context ->
_run_references_parallel (used when MoA references run alongside the main
model) and MoAClient/MoAChatCompletions (used when the MoA preset itself is
the acting model), then poll concurrent.futures.wait() in
_REFERENCE_POLL_INTERVAL_S slices instead of blocking on future.result() per
reference, checking agent._interrupt_requested each cycle.
Deliberately scoped to interrupt/cancel only -- no new or changed timeout
value, so this doesn't overlap open PRs #53784/#53875 (which lower the
per-reference timeout default but don't add interrupt support). `agent` is
optional and defaults to None, so any caller that doesn't pass it keeps
today's uninterruptible blocking behavior unchanged.
Adds test_moa_gemini_aggregator_sanitize_uses_real_model: drives a full MoA
tool-call turn (virtual-provider mode) with a Gemini aggregator and asserts
the strict-API sanitize pass is invoked with the resolved aggregator model
(gemini-3-pro-preview), never the virtual preset name once a slot is
resolved — the exact path that stripped extra_content/thought_signature and
made Gemini aggregators 400 (#65092).
Writing the test surfaced a gap in the salvaged #66212 fix: in virtual-
provider MoA mode (provider=moa, no moa_config threaded through
run_conversation) the conversation-loop branch never fired because it only
consulted moa_config. Extend it to fall back to the facade's
last_aggregator_slot — the same source the handle_max_iterations fix uses —
so both MoA entry modes resolve the real aggregator model.
Also adds the contributors/emails mapping for the #15676 credit base.
When MoA mode is active with a Gemini model as the aggregator,
agent.model holds the virtual preset name (e.g. "closed"), not the
actual aggregator model name. The _sanitize_tool_calls_for_strict_api
call uses agent.model to decide whether to keep extra_content
(thought_signature) on tool_calls — since "closed" doesn't contain
"gemini", the thought_signature is stripped and the Gemini aggregator
rejects the next request with HTTP 400 (INVALID_ARGUMENT):
"Function call is missing a thought_signature in functionCall parts."
Fix: resolve the actual aggregator model name from moa_config
(conversation_loop) or last_aggregator_slot (chat_completion_helpers)
and pass it to _sanitize_tool_calls_for_strict_api so the
_model_consumes_thought_signature check sees the real Gemini model.
Closes#65092
The phase-1 tool-result prune only runs inside compress(), which fires
near 50% of the context window, so it never triggers on large-window
models; old tool outputs then ride in history and are re-sent every turn.
Add prune_tool_results_only(): the same no-LLM prune on a separate, low
proactive_prune_tokens trigger, run as an elif to the compression branch.
Opt-in (default 0), protects the recent tail by message count.
Add the method to the ContextEngine base as a no-op default so pluggable
engines inherit it safely (the post-tool-call path never AttributeErrors on
a non-built-in engine); the built-in compressor supplies the real prune.
Register both keys under the top-level compression config with defaults and
document them.
Interrupted turns can seed a speculative display token count before the provider receives the request. Restore that display-only seed when interruption wins the race, while preserving completed post-compaction state and treating a successful provider response independently of optional usage metadata.
Constraint: #54776 remains reproducible on current main, while review #4702305384 identifies anti-thrashing rollback as stale and usage receipt as an unreliable response-completion signal.
Rejected: Restore anti-thrashing counters from a preflight snapshot | current main derives their verdict from real provider usage after a completed compaction boundary.
Confidence: high
Scope-risk: narrow
Directive: Keep interrupted preflight rollback display-only, and never infer provider completion from the presence of usage metadata.
Tested: ./.venv/bin/python -m pytest -q tests/run_agent/test_413_compression.py (29 passed); turn-finalizer/conversation-loop tests (31 passed); context-compressor targeted tests (12 passed); infinite-compaction targeted tests (3 passed); ruff; git diff --check.
Not-tested: End-to-end interactive interrupt through CLI or gateway transport.
