feat(hooks): add hooks.transcript_mining config knob - #1940
Draft
Lcstyle wants to merge 1 commit into
Draft
Conversation
Adds `hooks.transcript_mining` (env: MEMPALACE_HOOKS_TRANSCRIPT_MINING) to let operators disable the in-hook transcript-mining subprocess without losing the save-reminder / silent-diary-checkpoint behavior. Motivation ---------- In multi-agent deployments (multiple concurrent Claude Code / Codex sessions filing through the same palace) an in-hook transcript mine can either starve interactive MCP writes queued behind it in the daemon (MemPalace#1497), or contend with concurrent processes for the palace storage lock. Downstream users have been carrying out-of-tree "mine off" patches (@anastasiiaanfimova's hooks_cli patch, @jphein's palace-daemon at the gateway layer) for months. This upstreams the pattern as a first-class config knob so no fork is required. Design ------ - New property `MempalaceConfig.hooks_transcript_mining` defaults to True (matches pre-PR behavior). - Env override `MEMPALACE_HOOKS_TRANSCRIPT_MINING={false,0,no,true,1,yes}`. - Master `hooks.auto_save=false` still masks it (back-compat). - Env override wins over the auto_save mask (mirrors how the existing MEMPALACE_HOOKS_AUTO_SAVE env override works). Belt-and-suspenders guards short-circuit at the top of each mining entry point rather than at each call site — one source of truth, no risk of a future caller forgetting the outer gate: - `_maybe_auto_ingest` (async background MEMPAL_DIR mine) - `_mine_sync` (synchronous precompact mine) - `_ingest_transcript` (transcript-into-convo-drawer ingest) Composes with MemPalace#1826's daemon: with mining out of the hook path, the daemon queue only holds ~200ms interactive writes and the daemon becomes a viable single-writer for multi-session deployments without the mine-starvation problem that motivated Choice 1 pure workarounds. Recommended multi-agent config (unlocked by this PR) ---------------------------------------------------- { "hooks": { "auto_save": true, "silent_save": false, "transcript_mining": false, "daemon": true } } - Stop/PreCompact still fire and emit save reminders - Assistant responds by calling `mempalace_diary_write` / `mempalace_add_drawer` - Interactive writes serialize through the daemon (safe under N sessions) - No mining subprocess is ever spawned by a hook fire - Bulk mining runs out-of-band via a scheduled `mempalace mine --daemon` when no session is active Tests ----- - Config truth table (8 tests): default/from-config/env-overrides/ auto_save-mask/env-wins-over-mask - Guard tests (4 tests): each entry point verified as no-op when transcript_mining=false, plus a regression test that mining still fires when transcript_mining=true (default) - All 275 existing tests in the touched files continue to pass.
Author
|
Reporting dogfood experience with this knob, in case it helps the PR land. We have been running this change as a local patch on several hosts since it was No regressions observed in that time. The knob defaults to existing behaviour, so Would be glad to see this merged, and can provide before/after mine timings from a |
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.
Summary
Adds a
hooks.transcript_miningconfig knob (env:MEMPALACE_HOOKS_TRANSCRIPT_MINING) that lets operators disable the in-hook transcript-mining subprocess while keeping the save-reminder / silent-diary-checkpoint behavior. Zero-breaking-change default (mining stays on).Motivation
Multi-agent deployments (multiple concurrent Claude Code / Codex sessions filing through the same palace) currently have to choose between:
hooks.auto_save: false— turns off both the reminder AND the mine. Operator loses the assistant-nudge that drives interactive saves.The gap: users want reminders without the mine. Downstream has been carrying out-of-tree patches for months:
hooks_climine-off gist patch — literally an earlyreturnin_maybe_auto_ingest/_mine_sync.palace-daemonat the gateway layer — same principle, gateway-scale ("For anything heavy (transcript ingestion), append to a plain text file from the hook and let a batch process pick it up later").This PR upstreams the pattern as a first-class config knob so no fork is required.
Design
MempalaceConfig.hooks_transcript_mining(defaults toTrue→ matches pre-PR behavior).MEMPALACE_HOOKS_TRANSCRIPT_MINING={false,0,no,true,1,yes}.hooks.auto_save=falsestill masks it (backwards compat: existingauto_save: falsedeployments stay silent).MEMPALACE_HOOKS_AUTO_SAVEalready behaves.Belt-and-suspenders guards short-circuit at the top of each mining entry point rather than at each call site — one source of truth, no risk of a future caller forgetting the outer gate:
_maybe_auto_ingest(async background MEMPAL_DIR mine)_mine_sync(synchronous precompact mine)_ingest_transcript(transcript → convo drawer ingest)Composes with #1826's daemon
With mining out of the hook path, the daemon queue only holds ~200ms interactive writes — the daemon becomes viable for multi-session deployments without the mine-starvation problem that motivated the workarounds above.
Recommended multi-agent config unlocked by this PR:
{ "hooks": { "auto_save": true, "silent_save": false, "transcript_mining": false, "daemon": true } }mempalace_diary_write/mempalace_add_drawermempalace mine --daemonwhen no session is activeTests
tests/test_config.py): default / from-config / four env overrides /auto_save-mask / env-wins-over-mask.tests/test_hooks_cli.py): each entry point verified as no-op whentranscript_mining=false, plus a regression test that mining still fires whentranscript_mining=true(default).test_hooks_cli.py+test_config.pycontinue to pass.Test plan
uv run pytest tests/test_config.py tests/test_hooks_cli.py -vuv run ruff check .+uv run ruff format --check .cleanhooks.transcript_mining: falseon a live deployment, confirm hooks fire but no_spawn_minesubprocess appears; confirm assistant still gets Stop reminder and calls MCP tools; confirm scheduledmempalace minestill populates the palace at 04:00.Deliberately out of scope
PRECOMPACT_BLOCK_REASONwiring — the constant is defined athooks_cli.py:110but never emitted (hook_precompactat line ~1353 always does_output({})). Wiring it up would revisit Compaction Blocking #955 / PreCompact hook unconditionally blocks /compact, making it unusable #1172 (PreCompact-block-caused-deadlock) which were closed with a specific "don't block on PreCompact" fix. That's a separate design decision better handled in a follow-up PR that specifically considers those closed issues.Related: #1497 (multi-writer safety discussion), #1826 (write daemon), #1828 (daemon hardening).