fix(compression): break the infinite compression loop (salvage of #31730) - #31930
Conversation
🔎 Lint report:
|
| Rule | Count |
|---|---|
not-iterable |
1 |
First entries
tests/gateway/test_compression_session_id_persistence.py:75: [not-iterable] not-iterable: Object of type `object` is not iterable
✅ Fixed issues: none
Unchanged: 4872 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
CI Test Fix: borrowed-source credential pruning
Root cause: The test writes a Fix: Added monkeypatch for Fix available at git fetch https://github.com/talwayh1/hermes-agent.git salvage/pr31730-compression-loop
git cherry-pick d32cad3fcVerified: |
Three compounding root causes: A) run_conversation() result dict missing session_id — gateway's dead-code guard at gateway/run.py:8700 never triggers B) preflight compression bypasses should_compress() anti-thrashing — re-triggers every turn when tool schemas dominate token budget C) gateway updates session_entry.session_id in memory but doesn't persist via session_store._save() Fixes: #29335
… persistence Follow-up to @someaka's fix. Polish: - Drop the redundant `_preflight_tokens >= threshold_tokens` clause. `should_compress(tokens)` already short-circuits when tokens < threshold, so the explicit comparison was dead code on the True branch. Tests: - Preflight: pin that should_compress() is called (anti-thrash has a vote). Mocks should_compress to return False even with tokens past the raw threshold and asserts no compression runs — exact bug shape from #29335. - Gateway: AST scan of gateway/run.py asserts every `session_entry.session_id = ...` assignment is followed by a `session_store._save()` call within the same block. Three sites mutate the session_id after compression; all three must persist or the next turn loads the pre-compression transcript and re-loops. Empirically verified the test catches the bug (drops the new _save() line → red). AUTHOR_MAP: - Map ed@bebop.crew -> someaka so the salvaged commit resolves to @someaka in release notes.
76fa6f0 to
2b5043b
Compare
Summary
Compression no longer infinite-loops. Salvage of #31730 (@someaka) with a polish + two invariant-pinning tests on top.
Root cause (from #29335)
Three coupled defects cause an infinite compression loop:
_compress_context()rolls the agent forward into a new session, butrun_conversation()never putsession_idin its result dict — the gateway's guard for "did the session change?" was dead code._preflight_tokens >= threshold_tokensdirectly, bypassingshould_compress()which carries the anti-thrash logic. When the system prompt + tool schemas dominate token count, compression saves <10% but preflight re-fires every turn anyway.session_entry.session_idin memory after compression but never calledsession_store._save()— the two sibling sites in the same file (hygiene rewrite at L8437, manual/compressat L12122) already persisted; this third site was missed.Changes
agent/conversation_loop.py: add"session_id": agent.session_idto the result dict; gate preflight onagent.context_compressor.should_compress(_preflight_tokens)instead of a raw threshold compare. (Contributor commit, authorship preserved.)agent/conversation_loop.py(polish): drop the redundant_preflight_tokens >= threshold_tokensclause the contributor PR added next to the newshould_compress()call —should_compress()already short-circuits whentokens < threshold_tokens, so the explicit comparison was dead on the True branch.gateway/run.py: callself.session_store._save()after the post-compressionsession_entry.session_id = ...assignment. (Contributor commit.)tests/run_agent/test_413_compression.py: one new test pinning that preflight callsshould_compress()so anti-thrash has a vote — mocksshould_compressto return False even with tokens past the raw threshold and asserts no compression runs.tests/gateway/test_compression_session_id_persistence.py: new file, one test. AST scan overgateway/run.pyasserts everysession_entry.session_id = ...assignment is followed by asession_store._save()call within the same block. Empirically verified the test catches the bug: drop the new_save()line → test reportsline 8694 missing _save(), restore it → green.scripts/release.py: AUTHOR_MAP entry mappinged@bebop.crew→someaka.Validation
Notes
session_entry.session_idassignments ingateway/run.py— adding a fourth site in the future without_save()will fail this test, not silently break compression a year from now.Infographic