fix(compaction): plugin engines never received the session store, making skew persistence inert - #554
Merged
Merged
Conversation
…ing skew persistence inert Three PRs shipped the skew-calibration stack -- #529 (survive a restart), #539 (key by provider/model), #541 (calibrate per content class). All three are correct. All three were INERT for the LCM engine, which is the engine this fleet actually runs. Measured on the live tree 2026-08-09: COMPACTION_SKEW ... ratio=1.405 ... class=tool <- loop running fine COMPACTION_SKEW ... ratio=1.353 ... class=tool sqlite> SELECT COUNT(*) FROM compression_skew_calibration; 0 <- nothing persisted agent_init binds the session store with: getattr(agent.context_compressor, "bind_session_state", None) ContextCompressor defines that method; the ContextEngine ABC did not. So every plugin engine skipped the bind SILENTLY -- no exception, no log -- left _session_db unset, and _persist_skew_history() returned at its first guard forever. The learned ratio (measured 1.33-1.41 under-count on tool output) was thrown away on every restart and relearned from raw-rough 1.0, which is precisely the gap #529 existed to close. Fix: define a minimal default bind_session_state on the ABC. Deliberately not a fix to LCM alone -- any future engine now inherits a working bind instead of failing the same silent way. ContextCompressor keeps its richer override (cooldown + failure-streak rehydration), pinned by a test. E2E on the real engine (not a double): load_context_engine("lcm").bind_session_state(db, "s1") _persist_skew_history() -> [('claude-apr', 'claude-opus-5', [1.4, 1.35, 1.33])] 9 tests, RED-proven: removing the method fails 7, including test_every_concrete_engine_can_be_bound (enumerates ContextEngine subclasses so the NEXT engine to miss this fails at test time, not in production) and test_agent_init_binds_through_getattr (pins the call shape the fix depends on, so the ABC default cannot quietly become decorative). The tests assert the WIRING, not just the logic. A persist method that works in isolation while nothing hands it a DB is exactly the failure that shipped three times in this subsystem. 979 passed across the compaction/skew/context-engine surface.
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.
The calibration stack was correct and completely inert
Three PRs shipped it -- #529 (survive a restart), #539 (key by provider/model), #541 (per content class). All three are right. None of them did anything, because the engine this fleet runs never received the session store.
Measured on the live tree 2026-08-09:
Root cause
agent_initbinds the session store through:ContextCompressordefines that method. TheContextEngineABC did not. So every plugin engine skipped the bind silently -- no exception, no log -- left_session_dbunset, and made_persist_skew_history()return at its first guard forever.The consequence is the exact gap #529 was built to close: the learned ratio (a measured 1.33-1.41 under-count on tool output) was discarded on every restart and relearned from raw-rough 1.0.
Fix
Define a minimal default
bind_session_stateon the ABC.Deliberately not a fix to LCM alone -- any future engine now inherits a working bind instead of failing the same silent way.
ContextCompressorkeeps its richer override (cooldown + failure-streak rehydration), pinned by a test so the default can't shadow it.Proof
E2E on the real engine, not a double:
9 tests, RED-proven -- removing the method fails 7. Two are class-level guards rather than instance tests:
test_every_concrete_engine_can_be_boundenumeratesContextEnginesubclasses, so the next engine to miss this fails at test time instead of in productiontest_agent_init_binds_through_getattrpins the call shape the fix depends on, so the ABC default can't quietly become decorativeThe tests assert the wiring, not just the logic. A persist method that works in isolation while nothing hands it a DB is exactly the failure that shipped three times in this subsystem.
979 passed across the compaction / skew / context-engine surface.