Skip to content

fix(smem-hygiene): default the Stop hook's session-summary fallback to off - #221

Merged
acidkill merged 1 commit into
acidkill:mainfrom
RobertSigmundsson:fix/stop-hook-session-summary-defaults-off
Sep 7, 2026
Merged

acidkill merged 1 commit into
acidkill:mainfrom
RobertSigmundsson:fix/stop-hook-session-summary-defaults-off

Conversation

@RobertSigmundsson

Copy link
Copy Markdown
Contributor

Summary

  • The Stop hook's session-summary fallback becomes opt-in: auto.capture_session_summary, defaulting to false.
  • The fallback is a truncation, not a summary; on a stock installation the write gate is off, so what it produces is stored rather than filtered.
  • Adds six tests over the default, the round trip, and the behaviour on both settings.

Why

When pattern extraction finds nothing in a transcript, the Stop hook manufactures a candidate from the last few lines and offers that to the write gate. On a real chat transcript those lines are harness markers and half-sentences, not knowledge — so the gate rejects them, and the work of extracting, encoding and judging them is repeated on every turn that produces no patterns.

The repository already says as much in two places, which is what makes this a default worth changing rather than a preference:

  • quality_scorer.py's _NOISE_PREFIXES opens with the literal "session activity:", above a comment describing it as an "Empirical denylist from gate_decision false-accepts (2026-07-23): 180x "Session activity:", 30x , …". The single most common false accept the gate had to learn to reject is the thing this fallback manufactures.
  • capture_state.py's module docstring calls duplicate "Session activity" entries "the dominant source of memory poisoning".

Measured on one production brain over twenty-four hours: 96 of 158 write-gate rejections, a little over three in five, originated in this path alone — entries like Session activity: <task-notification>. That number is from a single brain and I would not lean on it alone; it agrees with the direction of the two notes above, which are the project's own.

Changes

  • unified_config.py: the new auto.capture_session_summary field, defaulting to false, with the reason recorded beside it.
  • hooks/stop.py: the fallback runs only when the flag is set.
  • tests/unit/test_stop_hook_session_summary.py (new) and tests/unit/test_hook_capture_idempotency.py, which turns the fallback back on explicitly.
  • docs/reference/config.md: regenerated with scripts/gen_config_docs.py, since that file is generated from these defaults and the Docs Freshness job checks it.

What this does not change

The gate itself. WriteGateConfig defaults to enabled: False and mode: "off", so on a stock installation the fallback's output is not rejected — it is stored, as a CONTEXT memory. That is the case this default actually addresses, and it is the common one rather than the exception, which is worth being explicit about: the argument here is not "the gate wastes effort" but "on the defaults, nothing is stopping this".

Setting capture_session_summary = true restores the previous behaviour exactly. The #80 idempotency regression tests do that deliberately, since the defect they pin lives on this path and needs it switched on to be observable at all.

Not touched, on purpose

The extraction helper keeps its name. Renaming it to say "tail" rather than "summary" would describe it better, and it has a single call site, so the rename would be small — but a rename is a different change from a default flip and belongs in its own commit rather than riding along.

Test plan

  • pytest tests/unit/test_stop_hook_session_summary.py — 6 passed: the default is off, an absent key loads as off, an explicit true survives the config round trip, save emits the key, and the hook stores nothing for pattern-free text with the flag off and does store with it on.
  • pytest tests/unit/test_hook_capture_idempotency.py — 12 passed. Those tests turn the fallback back on explicitly, because the Auto-capture: no idempotency key (Stop/PreCompact re-capture loop) + PreCompact/task_context hooks still bypass dedup #80 defect they exist to pin is only reachable through it.
  • With hooks/stop.py and unified_config.py reverted to main and the tests kept, 5 of the 6 new tests fail.
  • pytest tests/ -m "not stress" -n 4 — 7289 passed, 48 skipped, 1 xfailed, which is main's 7283 plus the six tests added here. Two tests in tests/unit/test_dashboard_brains_scope.py fail on this branch and on main alike: they want a live database and collide with one another under -n. Both pass when that file is run on its own.
  • ruff check src/ tests/ clean; ruff format --check src/ tests/ reports 740 files already formatted.
  • mypy src/ --ignore-missing-imports — success, no issues found in 354 source files.
  • Coverage under the CI gate: 72.37%, against 72.36% on main.
  • CHANGELOG.md untouched — left to the release entry, as with fix(storage): bind datetimes in time comparisons so they select by value #191fix(memory): refresh content-derived fields on compress, restore, and refine #193.

Verified by

@RobertSigmundsson

…o off

_extract_session_summary() does not summarise -- it joins the last ~10
non-trivial transcript lines verbatim behind a "Session activity: " prefix.
On prose that reads fine; on a real agent transcript it emits harness
markers and half-sentences.

Measured on the production brain over 24 h: 96 of 158 write-gate rejections
(61 %) originated in this one path, e.g. "Session activity: <task-notification>"
and "Session activity: Writing objects:". The gate discarded all of them, so
nothing corrupt was ever stored -- but the whole week's yield from the hook was
8 accepted entries, every one truncated mid-sentence, six of them ownerless
TODOs that came back as session-start context days later.

The fallback fires precisely when pattern extraction found nothing, so it is a
noise generator conditioned on failure. Gate it behind
auto.capture_session_summary, defaulting to false; the write gate already
rejects what it produces, so the only thing lost is telemetry noise.

Not deleted, because the acidkill#80 idempotency regression suite pins exactly this
re-encode path -- that suite now opts in explicitly rather than relying on a
default. Its cross-process race test writes the flag into its own config.toml,
since the in-process opt-in cannot reach a subprocess.

New tests pin both directions plus config round-trip and on-disk emission: a
default that nothing asserts is a default that drifts back.

(cherry picked from commit 9a4e6ed)

The comment on the new field and the test's module docstring are corrected at
the same time. Both said the quality gate already discards this output, which is
only true where the gate is enabled: WriteGateConfig defaults to enabled=False
and mode="off", so on a stock installation nothing rejects it and it is stored as
a CONTEXT memory. That is the case this default actually addresses, and saying
the opposite two lines from the flag would have been the worse kind of wrong.
The same comment also named PreCompact, which has no such fallback.

@acidkill acidkill left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full diff. The measured 61%-of-rejections number justifies the default flip; both directions are pinned, the idempotency suite opts back in explicitly, and the subprocess racer gets config.toml rather than an in-process toggle. Note for the changelog: default behaviour change, opt-out is auto.capture_session_summary=true.

@acidkill
acidkill merged commit 8ff85e7 into acidkill:main Sep 7, 2026
9 checks passed
@acidkill acidkill mentioned this pull request Sep 7, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants