feat(retention): policy-driven data retention for entities (#275) - #294
Conversation
#275) Adds altk_evolve/retention/: a RetentionPolicy of ordered rules (age via created_at, disuse via metadata.last_accessed) that flag or delete entities, plus a session -> derived cascade keyed on provenance metadata. Driven through the EvolveClient public API, so it is backend-agnostic and every delete flows through the memory_pre_delete hook (a legal hold can veto one). Builds on the merged hook seam (#287) in two ways: - last_accessed is now actually populated by AccessStampPlugin, so unused rules have a real signal. Where an entity carries no stamp the engine falls back to created_at and SAYS SO — per-item `detail` plus a run-level RetentionReport.warning — instead of the prototype's silent substitution. - MetadataNormalizerPlugin makes trace_id canonical for MCP-saved sessions; the engine also falls back to task_id at read time so sessions written before the normalizer still cascade. EvolveClient.record_access is kept as the explicit path for callers not running hooks, but now shares the plugin's build_access_stamps core so the two mechanisms cannot drift. `evolve retention run --policy <file> [--apply]` is dry-run by default and reports the evidence behind every decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a stdlib-only plugin-source/lib/retention.py and a `retention` skill, rendered to all four platform variants. Same rule schema, same flag/delete actions, same dry-run default and cascade concept as the package, running over the .evolve/ file store; every applied action appends an event: "retention" row to .evolve/audit.log. The signals are weaker than package-side and the code and skill now say so rather than implying parity: age comes from file mtime (there is no created_at in the store, so an edit resets the clock), disuse comes from the model-invoked recall audit log, and the `trajectory:` cascade link — while supported and preserved by entity_io — is written by nothing in the shipped plugin today, so cascade_derived is effectively inert there. A test pins that last gap so it cannot be mistaken for working. Subscribed entities (git clones owned by the sync skill) stay out of scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…emo (#275) Adds docs/guides/retention.md in the house style of the memory-hooks guide: policy format, flag vs delete, dry-run-by-default and how to apply, the session cascade and the MCP/Phoenix convention split the normalizer closed, and an honest account of what the unused rule depends on (AccessStampPlugin or record_access — without either it is an age rule wearing a different name). Documents plugin-side parity and its three real gaps in a table (mtime as age, model-invoked recall audit, and the unwritten `trajectory:` cascade link), plus what the plugin sweep excludes (entities/subscribed/, public/, symlinks). Also wires Memory Hooks and Data Retention into the mkdocs nav, cross-links the two guides, and ships examples/retention.example.yaml and a runnable examples/retention_demo.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRetention support is added across the core client and evolve-lite plugins. Policies can flag or delete entities using age or access signals, cascade session deletions, run through CLI and skills, report warnings and errors, and default to dry-run behavior. ChangesData retention
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
altk_evolve/retention/engine.py (1)
102-102: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSilent truncation at
FETCH_LIMIT.If a namespace has more than 100k entities,
get_all_entitiestruncates silently and the sweep won't see the remainder — no warning is emitted in this case, unlike the "no last_accessed" degraded-signal warning. For a compliance-driven retention feature, consider surfacing a warning whenlen(entities) == FETCH_LIMITso operators know the run may be incomplete.Also applies to: 172-172
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@altk_evolve/retention/engine.py` at line 102, Update get_all_entities to emit a warning whenever the fetched entity count equals FETCH_LIMIT, indicating that results may be truncated and the retention sweep may be incomplete; apply the same check at the other FETCH_LIMIT usage noted in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@altk_evolve/retention/engine.py`:
- Around line 204-220: Restrict the cascade block in the retention engine to
session/trajectory entities by adding the same type guard used by the mirrored
implementation before processing derived records. Keep the existing delete and
cascade_derived checks and all downstream tracing/recording behavior unchanged
for eligible trajectory entities.
---
Nitpick comments:
In `@altk_evolve/retention/engine.py`:
- Line 102: Update get_all_entities to emit a warning whenever the fetched
entity count equals FETCH_LIMIT, indicating that results may be truncated and
the retention sweep may be incomplete; apply the same check at the other
FETCH_LIMIT usage noted in the comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bc44d10-b9bf-4283-a8f5-174748cc2d7f
📒 Files selected for processing (30)
altk_evolve/cli/cli.pyaltk_evolve/frontend/client/evolve_client.pyaltk_evolve/retention/__init__.pyaltk_evolve/retention/engine.pyaltk_evolve/retention/policy.pydocs/guides/memory-hooks.mddocs/guides/retention.mddocs/reference/cli.mdexamples/retention.example.yamlexamples/retention_demo.pymkdocs.yamlplatform-integrations/bob/evolve-lite/commands/evolve-lite-retention.mdplatform-integrations/bob/evolve-lite/lib/evolve-lite/retention.pyplatform-integrations/bob/evolve-lite/skills/evolve-lite-retention/SKILL.mdplatform-integrations/bob/evolve-lite/skills/evolve-lite-retention/scripts/run_retention.pyplatform-integrations/claude/plugins/evolve-lite/lib/evolve-lite/retention.pyplatform-integrations/claude/plugins/evolve-lite/skills/evolve-lite/retention/SKILL.mdplatform-integrations/claude/plugins/evolve-lite/skills/evolve-lite/retention/scripts/run_retention.pyplatform-integrations/claw-code/plugins/evolve-lite/lib/evolve-lite/retention.pyplatform-integrations/claw-code/plugins/evolve-lite/skills/evolve-lite/retention/SKILL.mdplatform-integrations/claw-code/plugins/evolve-lite/skills/evolve-lite/retention/scripts/run_retention.pyplatform-integrations/codex/plugins/evolve-lite/lib/evolve-lite/retention.pyplatform-integrations/codex/plugins/evolve-lite/skills/evolve-lite/retention/SKILL.mdplatform-integrations/codex/plugins/evolve-lite/skills/evolve-lite/retention/scripts/run_retention.pyplugin-source/lib/retention.pyplugin-source/skills/evolve-lite/retention/SKILL.md.j2plugin-source/skills/evolve-lite/retention/scripts/run_retention.pypyproject.tomltests/platform_integrations/test_retention_plugin.pytests/unit/test_retention.py
…scan_limit
FIX 1: an empty-string (or otherwise falsy) trace_id / source_task_id no
longer forms a cascade bucket, so a session with no real provenance link can
no longer cascade-delete every entity that merely lacks provenance. Both sides
of the match are normalized to str and required non-empty, so an int trace_id
still matches a str source_task_id of the same value but never an empty one.
FIX 2: new per-rule RetentionRule.on_missing_access_signal
("skip" default / "flag" / "delete"). An `unused` DELETE that matched only
because it fell back to created_at (no real last_accessed stamp) now skips by
default (fail-safe), recorded in the new RetentionReport.skipped list; "flag"
downgrades it to a non-destructive flag; "delete" is the explicit opt-in to the
old behavior. Only bites deletes — flag rules and stamped/age matches are
unaffected. Wired through the engine, report, and CLI (skipped table).
FIX 3: RetentionEngine.apply()/evaluate() take a scan_limit param
(default FETCH_LIMIT=100_000); a scan that comes back holding exactly the limit
emits a RetentionReport.warnings entry that entities beyond it were not
evaluated (and not cascaded).
FIX 5: adds a filesystem-backend dry-run test asserting a real store is not
mutated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ports FIX 2 to the plugin-side store (plugin-source/lib/retention.py): rules
accept on_missing_access_signal ("skip" default / "flag" / "delete") validated
against VALID_MISSING_SIGNAL, an `unused` DELETE that fell back to file mtime
(no recall row) skips by default into a new report["skipped"] list, and
run_retention.py prints SKIP lines. SKILL.md documents the knob and the
first-match-shadowing ordering caveat.
FIX 1 note: the plugin cascade was already falsy-safe (empty `trajectory:`
frontmatter is dropped at parse time and guarded by `if link:`); added a
regression test that a literal empty trajectory link does not cascade.
Re-rendered to all four variants (claude / claw-code / codex / bob).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…h shadowing FIX 4: reorder examples/retention.example.yaml so the long-threshold `unused-guidelines` DELETE precedes the `stale-guidelines` FLAG — first-match otherwise made the delete unreachable — and set on_missing_access_signal: skip explicitly so an unstamped guideline is spared, not deleted from created_at. Guide updates: on_missing_access_signal table, a "When the unused signal is missing" section, a "First-match shadowing" section explaining the trap, and the scan_limit / boundary-warning operational note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guides/retention.md`:
- Around line 39-40: Update the retention guide sample so the output for the
unused-guidelines rule matches its on_missing_access_signal: skip configuration:
show the unstamped delete match as skipped rather than FLAG. Keep the documented
policy and surrounding examples unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 78c466e6-7315-448f-bd86-4ce6b9cdb3dc
📒 Files selected for processing (23)
altk_evolve/cli/cli.pyaltk_evolve/retention/engine.pyaltk_evolve/retention/policy.pydocs/guides/retention.mdexamples/retention.example.yamlexamples/retention_demo.pyplatform-integrations/bob/evolve-lite/lib/evolve-lite/retention.pyplatform-integrations/bob/evolve-lite/skills/evolve-lite-retention/SKILL.mdplatform-integrations/bob/evolve-lite/skills/evolve-lite-retention/scripts/run_retention.pyplatform-integrations/claude/plugins/evolve-lite/lib/evolve-lite/retention.pyplatform-integrations/claude/plugins/evolve-lite/skills/evolve-lite/retention/SKILL.mdplatform-integrations/claude/plugins/evolve-lite/skills/evolve-lite/retention/scripts/run_retention.pyplatform-integrations/claw-code/plugins/evolve-lite/lib/evolve-lite/retention.pyplatform-integrations/claw-code/plugins/evolve-lite/skills/evolve-lite/retention/SKILL.mdplatform-integrations/claw-code/plugins/evolve-lite/skills/evolve-lite/retention/scripts/run_retention.pyplatform-integrations/codex/plugins/evolve-lite/lib/evolve-lite/retention.pyplatform-integrations/codex/plugins/evolve-lite/skills/evolve-lite/retention/SKILL.mdplatform-integrations/codex/plugins/evolve-lite/skills/evolve-lite/retention/scripts/run_retention.pyplugin-source/lib/retention.pyplugin-source/skills/evolve-lite/retention/SKILL.md.j2plugin-source/skills/evolve-lite/retention/scripts/run_retention.pytests/platform_integrations/test_retention_plugin.pytests/unit/test_retention.py
🚧 Files skipped from review as they are similar to previous changes (18)
- platform-integrations/codex/plugins/evolve-lite/skills/evolve-lite/retention/SKILL.md
- platform-integrations/claw-code/plugins/evolve-lite/skills/evolve-lite/retention/SKILL.md
- platform-integrations/claude/plugins/evolve-lite/skills/evolve-lite/retention/SKILL.md
- platform-integrations/claw-code/plugins/evolve-lite/skills/evolve-lite/retention/scripts/run_retention.py
- platform-integrations/bob/evolve-lite/skills/evolve-lite-retention/scripts/run_retention.py
- examples/retention.example.yaml
- examples/retention_demo.py
- plugin-source/skills/evolve-lite/retention/scripts/run_retention.py
- altk_evolve/cli/cli.py
- platform-integrations/codex/plugins/evolve-lite/skills/evolve-lite/retention/scripts/run_retention.py
- platform-integrations/bob/evolve-lite/skills/evolve-lite-retention/SKILL.md
- tests/platform_integrations/test_retention_plugin.py
- altk_evolve/retention/engine.py
- plugin-source/skills/evolve-lite/retention/SKILL.md.j2
- platform-integrations/claude/plugins/evolve-lite/skills/evolve-lite/retention/scripts/run_retention.py
- platform-integrations/claude/plugins/evolve-lite/lib/evolve-lite/retention.py
- plugin-source/lib/retention.py
- platform-integrations/claw-code/plugins/evolve-lite/lib/evolve-lite/retention.py
A cascade_derived delete fanned out along source_task_id for ANY deleted entity, so a rule with entity_type=None (or a non-trajectory type) that matched a non-trajectory carrier holding a trace_id/task_id would mass-delete every entity linked by source_task_id. Only a trajectory owns a provenance tree, so gate the cascade on e.type == TRAJECTORY_TYPE, matching the plugin side's guard. The existing falsy-trace guard (if not trace: continue) is preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The dry-run sample showed an unstamped `unused` match as FLAG under the `unused-guidelines` delete rule, but that rule ships on_missing_access_signal: skip, so such a match renders as skipped, not flagged. Regenerated the block from a real engine run of the shipped example policy: age-delete -> DELETE, cascade -> DELETE, stale age -> FLAG (stale-guidelines), unstamped unused-delete -> SKIP. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…keep always-on hooks # Conflicts: # altk_evolve/cli/cli.py # docs/guides/memory-hooks.md
What
Adds data retention — the half of #275 that main still lacks. A declarative policy selects entities by type and age and either flags them for review or deletes them, with a session → derived cascade. Dry run everywhere by default.
PII is deliberately out of scope for this PR.
Why
Memories accumulate: some go stale, some are never read again, and session transcripts often carry data with a bounded agreed retention. Deleting a session without deleting the memories extracted from it just leaves the same data behind under another name — hence the provenance cascade.
Policy
Rules are evaluated top-to-bottom, first match wins; a cascade
deletesupersedes an earlierflag. The engine drives theEvolveClientpublic API, so it is backend-agnostic and every delete flows throughmemory_pre_delete— a legal-hold plugin can veto a retention delete, and the veto lands inreport.errorswithout aborting the sweep.Synergy with the merged hook seam (#287)
This is the substantive improvement over the earlier prototype. The seam changed two things in retention's favour and both are wired up here:
1.
last_accessedis now actually populated.AccessStampPluginstamps it onmemory_post_read, somax_unused_daysfinally has a real signal — in the prototyperecord_accesshad zero callers, so the unused rule silently degraded into an age rule for every entity. Now the fallback is explicit rather than silent: an entity with no stamp gets a per-itemdetailnaming the fallback, and the run carries aRetentionReport.warning:2.
trace_idis now reliably stamped.MetadataNormalizerPlugincopiestask_id→trace_id, closing the MCP-vs-Phoenix convention split that previously broke the cascade for MCP-saved trajectories. The engine additionally falls back totask_idat read time so sessions written before the normalizer still cascade (trace_idwins when both are present). Both conventions are covered by tests, including one that runs the actualnormalize_entitiescore over an MCP-shaped payload.record_accessdecision — kept, but no longer a second disjoint mechanism. It now shares the plugin'sbuild_access_stampscore, so key, format and one-stamp-per-batch behaviour cannot drift. The plugin is the automatic path;record_accessis the explicit path for callers not running hooks, or for recording a use that was not a store read (a memory pulled from cache and acted on). A test asserts the two produce byte-identical stamps. Dropping it would have left non-hook users with no way to feed the unused rule at all.Plugin-side parity, and its gaps
evolve-litegets a stdlib-onlyretentionlib + skill rendered to all four variants (claude / claw-code / codex / bob), with the same schema, actions, dry-run default and cascade concept over the.evolve/file store, auditing every applied action. The signals are genuinely weaker, and the code, the skill and the guide all say so rather than implying parity:created_atcreated_atexists in the storemetadata.last_accessed(automatic,AccessStampPlugin)recallrow in.evolve/audit.logmetadata.source_task_id == trace_idtrajectory:frontmatter keyentity_iosupports and preserves it, but neither the save flow noradapt-memorypopulates it — socascade_derivedis effectively inert plugin-side. A test pins this so it cannot be mistaken for working.Out of scope for the plugin sweep:
entities/subscribed/(sync-owned git clones — a local delete would just be restored),public/, symlinks,.git.Safety
RetentionEngine.apply()defaults todry_run=Trueand the CLI requires an explicit--apply. A dry run computes every decision and mutates nothing; the report is identical in shape to an enforced one.deletehas no undo —flagexists so you can stage a review queue first.Test evidence
uv run --no-sync pytest tests -q).tests/unit/test_retention.py) and 16 plugin tests (tests/platform_integrations/test_retention_plugin.py), covering age/unused/dry-run/cascade, the degraded-signal reporting, both trace-id conventions, first-match-wins, delete-supersedes-flag, per-entity failure isolation, and therecord_access↔ plugin equivalence.uv lock --checkclean;pre-commit run --all-filesgreen (mypy, ruff, detect-secrets, plugins-rendered).examples/retention_demo.pyrun end-to-end: dry run shows every decision with its "why" and mutates nothing,--applyflags the stale guideline, deletes the 400-day-old session together with the memory derived from it (matched viatask_id), and leaves the guideline recalled yesterday alone.Closes part of #275 (retention half).
🤖 Generated with Claude Code
Summary by CodeRabbit
retention runCLI with YAML/JSON policies, namespace sweep, dry-run default, and--applyenforcement.Post-review fixes (tiered internal review)
A tiered review (fast-model diff mapping → higher-tier adversarial review with repro-verified fixes) ran over this PR. All findings reproduced, fixed, and re-verified by re-running the repros:
trace_id=""cascade-deleted every entity withsource_task_id=="", regardless of real provenance. Fixed: falsy ids never form a cascade bucket (guarded on both package and plugin side); genuine matches (including int↔str, e.g.1↔"1") still cascade. Regression tests added.unused-delete → configurable, fail-safe. AnunusedDELETE rule previously deleted never-access-stamped entities by measuring disuse fromcreated_at— silent data loss when stamping is off. Now a per-ruleon_missing_access_signal: skip | flag | deletewith a fail-safeskipdefault; skipped entities are surfaced in the report and CLI. Wired through the engine, the plugin (all 4 variants), the CLI, and the docs.scan_limitparameter and emits a warning when a namespace may exceed it (previously silent).Retention tests 42 → 59; full suite green.