fix(agent): sync logging session context on compaction id rotation - #41726
Merged
Merged
Conversation
alt-glitch
added
type/bug
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-attribute |
1 |
First entries
tests/agent/test_compression_logging_session_context.py:52: [unresolved-attribute] unresolved-attribute: Unresolved attribute `context_compressor` on type `AIAgent`
✅ Fixed issues: none
Unchanged: 5225 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
teknium1
force-pushed
the
hermes/hermes-8e3e1b3d
branch
from
June 8, 2026 05:10
811fc1a to
cae83f6
Compare
When context compaction rotates agent.session_id, it updates the gateway/tools session context (set_current_session_id -> HERMES_SESSION_ID env + ContextVar) but never updates the separate logging session context. The [session_id] tag on log lines comes from hermes_logging._session_context (set once per turn in conversation_loop.py), so post-compaction log lines in the same turn carry the STALE old id while the message/DB/gateway state carry the new one — breaking log correlation exactly at the compaction boundary. Call hermes_logging.set_session_context(agent.session_id) alongside the existing set_current_session_id, guarded so a logging failure can't regress the routing update. Logs-only; no runtime or caching impact. Refs #34089
teknium1
force-pushed
the
hermes/hermes-8e3e1b3d
branch
from
June 8, 2026 05:21
cae83f6 to
1bd5dff
Compare
9 tasks
fabiosiqueira
added a commit
to fabiosiqueira/hermes-engine
that referenced
this pull request
Aug 17, 2026
`interruptible_streaming_api_call` runs the provider call on a worker
thread — `_call` for the OpenAI/Anthropic path, `_bedrock_call` for
Bedrock Converse. The `[session]` tag every log line carries comes from a
`threading.local` in `hermes_logging`, and `_context_thread_target`
carries the caller's ContextVars across the boundary but cannot carry a
thread-local. Both workers therefore start unbound and everything they
log is formatted without a session tag.
The cost lands where it hurts most: each worker is where its own stream
failure is logged, so in a process serving concurrent sessions the one
line that carries the cause is the one line that names no session —
INFO agent.chat_completion_helpers: Streaming failed before delivery:
'dict' object has no attribute 'model_dump'
Bind the agent's session id onto both workers. Sourced from
`agent.session_id` rather than inherited from the calling thread: the
agent is the ground truth, and a caller that reached streaming without
going through `turn_context` is itself unbound. Best effort — a
session-less agent leaves the thread unbound instead of raising inside
the worker.
The tests drive the real `interruptible_streaming_api_call` on both
paths and assert on a record emitted from inside each worker, with the
calling thread deliberately left unbound; placement is the defect, so
exercising the helper alone would not catch a worker that was never
bound.
Same class of gap as NousResearch#41726, which synced the session context on
compaction id rotation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
After a mid-turn context compaction, log lines now carry the new session id instead of the stale pre-rotation one — closing the log-correlation gap from #34089.
When
compress_contextrotatesagent.session_id, it already moves the gateway/tools session context (gateway.session_context.set_current_session_id→ ContextVar +HERMES_SESSION_IDenv). But the[session_id]tag on log lines comes from a separate mechanism —hermes_logging._session_context(athreading.localread by the global LogRecord factory, pinned once per turn in the conversation loop). The rotation block never updated it, so post-compaction log lines in the same turn kept the old id while the message body / session DB / gateway state carried the new one. That is exactly the "started session=X, done session=Y" ambiguity in the #34089 evidence.Changes
agent/conversation_compression.py: after the rotation updates the gateway/tools context, also callhermes_logging.set_session_context(agent.session_id), guarded separately so a logging failure can never regress the routing update.tests/agent/test_compression_logging_session_context.py: regression test asserting the logging context follows the rotation.Validation
[session_id]tag after mid-turn compactiontests/agent/test_compression_logging_session_context.pySalvaged from #39994 by @JimStenstrom; original commit cherry-picked with authorship preserved. AUTHOR_MAP entry added.
Infographic