Skip to content

feat(memory): a session purge retracts what was learned from the chat - #854

Open
OAraLabs wants to merge 2 commits into
mainfrom
fix/purge-retracts-derived-memory
Open

OAraLabs wants to merge 2 commits into
mainfrom
fix/purge-retracts-derived-memory

Conversation

@OAraLabs

Copy link
Copy Markdown
Owner

What this fixes

POST /api/sessions/{id}/purge removed a chat's messages and summaries from the conversation database, but left what the assistant had derived from the chat: the facts the extractor mined from it, the wiki pages compiled from those facts, and the insight pages synthesized from them. A secret pasted into a chat and then purged survived as a fact and a wiki line, even though the route reports "reversible": false and its docstring frames it as the answer to "that must not be on the disk".

The rule

Decided by the owner:

  • A fact learned only from the purged chat is deleted.
  • A fact a surviving chat also supports stays, and the purged chat stops being one of its sources.
  • The owner is shown what will be removed before confirming.

One fact about the data that shapes the design

A fact's source_event_ids is batch-level, not exact. The extractor sweeps new messages across every chat in row order, cuts them into batches, and every fact from a batch cites every message in it (MemoryExtractor._process_batch). One batch can mix chats, so "this fact still has a surviving source" does not mean "a surviving chat supports it": the surviving id may only be a batch-mate of the purged message. Read literally, the rule would keep a fact learned only from the purged chat, which is the one outcome a purge must not allow.

So a surviving source counts as support only if that message actually mentions the fact's entity (_supports in memory/purge_derived.py, one small function). The cost is stated plainly: a fact that a surviving chat expressed without naming the entity (a pronoun, an alias) can be removed although that chat supports it. That is deliberate, because the point of a purge is that the purged content does not survive, and the preview lists every removal with its reason so the owner sees it first. The real fix is to record the source chat per fact at write time; that is a separate change to extraction and is not in this PR.

What changes

Retraction (new memory/purge_derived.py):

  • Facts learned only from the chat are deleted. Facts it shares with a surviving chat keep their text and lose the purged chat as a source; mention_count scales by the surviving share of sources (floor 1), so the purged chat's mentions stop propping up a wiki page (the existing two-mention threshold).
  • The wiki is rebuilt from the store (it is a pure projection of it, so the facts' text and the short source-id fragment each fact line carries both go).
  • Insight pages in wiki/queries/ whose recorded entities include a removed fact's entity are deleted; they regenerate from what is left on the next synthesis cycle. Filed answers that mention a removed entity are listed for review and never touched: they record no source, so they can only be flagged.
  • The deprecated messages/summaries tables in memory.db and the chat's extractor cursor row are cleaned too.

Order and failure rules. The chat's message ids are how a fact names its sources, and the id-to-chat mapping disappears with the rows, so the plan is made and applied before the conversation rows are deleted. Then the existing purge runs, then a sweep with the same ids catches a fact the extractor wrote while the purge was running.

  • If retraction raises, the route answers 500 and purges nothing; a retry is safe (every step is idempotent).
  • The one non-atomic step is the wiki rebuild (files). The memory transaction records a mark in store_marks, and a rebuild that failed after the facts committed is finished by the next purge even though that purge's own plan finds nothing left to retract.
  • With no memory store running, the purge still proceeds and the response says derived.available: false (and the log warns), rather than reporting a retraction that did not happen.

The file is scrubbed, not just the rows. MemoryStore.retract applies everything in one transaction, then does what the conversation purge already does for its own file: secure delete, FTS5 optimize, WAL checkpoint, VACUUM. A test proves the text is absent from the file, its WAL, and the FTS index. The strip of a kept fact is done against the row as it is inside the transaction, not against a list computed earlier, so a source the extractor merged in between planning and applying is not erased.

Not changed: automatic retention. Old forgotten chats purged by the retention job never retract facts: what was learned is meant to outlive chat retention. Only the owner's explicit purge retracts (a test pins this).

API (for the client)

  • New GET /api/sessions/{id}/purge-preview (operator only, read-only; classified in web/route_access.py): {session_id, messages, branches, derived: {available, facts_removed[{id, entity_name, entity_type, fact, manual, reason}], facts_kept[...], wiki_pages_removed, wiki_pages_rebuilt, insight_pages_removed, queries_to_review, totals, truncated}, not_covered, reversible: false}. Lists are capped at 200 with the true totals alongside.
  • POST .../purge (still needs {"confirm": "<session_id>"}) gains derived and not_covered in its response.
  • not_covered says what a purge does not reach: notes the assistant wrote into its own MEMORY.md/USER.md, skills or drafts the learning loop wrote from the chat, telemetry rows, forks (already reported as branches), and text in other chats that quotes it.

The screen that shows the preview before the owner confirms lives in the client and is a separate change; this PR is the daemon side it needs.

Tests

Written first and watched fail. tests/test_purge_retraction.py (23) covers the rule case by case: only-source, supported, batch-mate, ids that exist in no chat, manual notes (treated like any other fact and flagged), untouched facts byte for byte, plan is read-only, apply is idempotent, plan and apply agree, the late sweep, the file/WAL/FTS scrub, the wiki pages, insight pages and filed answers, a failed wiki rebuild finished on retry, the cursor, the legacy tables, and retention leaving facts alone. tests/test_api_purge_preview.py (8) is the HTTP contract, including: the preview changes nothing; a failed retraction answers 500 with the chat still present; a failed final sweep is reported, not hidden; no memory store still purges and says so. tests/test_scoped_devices_default_deny.py gains the scoped-device 403 on the preview.

I broke the code three ways on purpose and confirmed the right test went red each time: counting any surviving source as support, skipping the FTS/VACUUM scrub, and dropping the rebuild mark.

  • Full suite, run as CI runs it (-m "not network"): 14,374 passed, 9 failed. The same 9 fail identically on an untouched main at 763e76d (URL-guard and web-download tests that need DNS, the deferred-tool reachability tests, and a real-daemon TLS restart), so they are not from this change.
  • ruff check src/ clean; scripts/mypy_gate.py OK (333 modules clean).
  • docs/reference/routes.md regenerated; docs/guide/api.md and docs/guide/memory.md describe the behaviour.

For review

  1. Manual notes (/note): a note whose only source is the purged chat is removed with it, per the rule, and is flagged manual: true in the preview. Say if notes should be exempt.
  2. The guard's trade-off above. It errs toward removal; the preview shows the reason for each.
  3. A page can disappear although its fact stays. A kept fact's mention count scales down, so an entity that had one mention in each of two chats drops under the wiki's two-mention page threshold and its page goes; the fact is still in the store and recall.
  4. This adds an extra GET route. memory_store/wiki_compiler are handed to the app through launch_web, like the other late-built objects.

Suggested line for the release notes: Purging a conversation now also removes what the assistant learned only from it (facts, wiki pages, insight pages), and GET /api/sessions/{id}/purge-preview shows what will go before you confirm.


Generated by Claude Code

Purging a chat removed its messages and summaries but left what the
assistant had derived from it: the facts the extractor mined, the wiki pages
compiled from them and the insight pages synthesized from them. A secret
pasted into a chat and then purged survived as a fact and a wiki line.

An explicit purge now also retracts, by the owner's rule:
- a fact learned only from the purged chat is deleted;
- a fact a surviving chat also supports stays, and the purged chat stops
  being one of its sources (mention_count scales by the surviving share,
  floor 1);
- the wiki is rebuilt from the store, insight pages that used a removed
  entity are deleted, and filed answers that mention one are listed for
  review and never touched.

A fact's source_event_ids is batch-level: every fact from an extraction batch
cites every message in it, and a batch can mix chats. So a surviving source
only counts as support if that message mentions the fact's entity; otherwise
it was only a batch-mate and the fact goes. The preview gives each removal's
reason.

Order: plan and retract BEFORE the conversation rows are deleted (the
message-id to chat mapping disappears with them), then the existing purge,
then a sweep with the same ids for a fact written during the purge. A failed
retraction answers 500 and purges nothing. The memory file is scrubbed like
the conversation file (secure delete, FTS optimize, WAL checkpoint, VACUUM),
and a wiki rebuild that fails after the facts commit is finished by the next
purge through a mark in store_marks. Automatic retention is untouched.

New: GET /api/sessions/{id}/purge-preview (operator only, read-only) so a
client can show what will go before the owner confirms. The purge response
gains a derived block. Without a running memory store the purge still runs
and says derived.available is false.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018UjrXNciMTQJ8Ms46sH4fu
…es (CVE-2026-114603)

The quality job's pip-audit step fails on main (opentelemetry-api 1.42.1,
CVE-2026-114603, fixed in 1.45.1), so it fails every PR. #844 is the fix; this
is the identical uv.lock change so this PR's check can go green. It is the same
bytes as #844, so whichever merges second is a no-op.

Verified: pip-audit on the exported lock reports no known vulnerabilities;
ruff and the mypy gate were already clean; the purge tests pass on the bumped
lock.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018UjrXNciMTQJ8Ms46sH4fu

Copy link
Copy Markdown
Owner Author

CI note: the first quality failure was not this change.

On the first run quality was red. Ruff and the mypy gate both passed on this PR (type gate OK — 333 module(s) clean); the failing step is pip-audit, which flags opentelemetry-api 1.42.1 (CVE-2026-114603, fixed in 1.45.1). The same check is red on main at 763e76d (completed 2026-10-22), so it fails every PR until the lock moves.

#844 is the fix (a one-file uv.lock bump, its own quality and test checks green). I carried its identical uv.lock change in bc18e1d so this PR's check can go green. It is the same bytes as #844, so whichever merges second is a no-op. Checked locally: pip-audit on the exported lock now reports no known vulnerabilities.

If you would rather keep this PR free of lock changes, drop bc18e1d and merge #844 first; nothing else here depends on it.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

CI note 2: all 14 checks are green on bc18e1d. One attempt of test-macos (3.14) failed on tests/test_daemon_shutdown.py::test_daemon_exits_promptly_on_sigterm[SIGTERM] (a real daemon must exit within 15 s of SIGTERM) in a 19-minute run, and it passed on a single re-run of that job. Everything else in that run passed (14,529), and the other 11 open PRs' macOS jobs are green. This change adds nothing to boot or shutdown (two keyword arguments to launch_web and two app.state assignments), so I am treating it as a runner-load flake of a time-budgeted test, not a regression; flagging it in case it shows up again.


Generated by Claude Code

This branch has not been deployed

No deployments
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