Skip to content

feat: self-pruning memory consolidation and on-demand rule loading#74

Merged
gupsammy merged 5 commits into
mainfrom
feat/memory-self-management
Jun 1, 2026
Merged

feat: self-pruning memory consolidation and on-demand rule loading#74
gupsammy merged 5 commits into
mainfrom
feat/memory-self-management

Conversation

@gupsammy
Copy link
Copy Markdown
Owner

Summary

Two related changes to how this marketplace's tooling manages always-loaded context, unified by one principle: replace unconditional loading with trigger-activated loading.

Memory consolidation was append-only

extract-learnings is meant to manage memory, but nothing applied downward pressure. signal-discoverer only adds; memory-auditor removed entries only on factual contradiction — explicitly exempting decisions, preferences, and principles, which are the large majority of entries; and the single size gate fired at 170 lines while real indexes sit well below it. The index grew without bound.

  • memory-auditor: adds value-based retirement — SUPERSEDED / REDUNDANT / LOW-VALUE categories that apply to principles, not just factually-checkable entries. Factual contradiction is no longer the only removal trigger.
  • signal-discoverer: adds a raise-the-bar rule — zero new candidates is a valid, valuable result, not a failure.
  • extract-learnings: replaces the dead size gate with per-run downward pressure, plus adaptive clustering — a MEMORY.md section past 25 entries splits into memory/clusters/<section>.md, capping the always-loaded index while detail stays on demand.

update-claudemd defeated its own progressive disclosure

The skill writes topic files to .claude/rules/, but .claude/rules/*.md auto-loads at SessionStart with CLAUDE.md priority unless scoped with paths: frontmatter (per the docs). So every "load on demand" topic file actually loaded every session — the opposite of the skill's stated purpose.

  • update-claudemd: mandates paths: frontmatter on every topic file it writes, scoped to the globs the subject covers, and threads the backfill of existing files through the research → classify → approve pipeline so it surfaces at the approval gate instead of being a silent rewrite.
  • .claude/rules/claude-memory-architecture.md: applies the fix to this repo's own leaking file (it was loading ~1.5k every session); now scoped to plugins/claude-memory/**, so it loads only when that subsystem is touched.

Commits

  • feat(memory) — self-pruning consolidation with adaptive clustering
  • fix(coding) — scope .claude/rules topic files with paths frontmatter

Notes

Plugin version bumps and README badge syncs in the diff are from the auto-version pre-commit hook, one per affected plugin.

gupsammy added 2 commits May 24, 2026 11:27
Consolidation was append-only: signal-discoverer added, memory-auditor removed only on factual contradiction, and the 170-line size gate never fired — so the index grew unbounded.

- memory-auditor: audit principles for SUPERSEDED/REDUNDANT/LOW-VALUE retirement, not just factual staleness
- signal-discoverer: raise the bar — zero new candidates is a valid result
- extract-learnings: replace the dead size gate with per-run downward pressure plus adaptive clustering (split sections past 25 entries into memory/clusters/*.md)
update-claudemd promised progressive disclosure but wrote topic files to .claude/rules/, which auto-loads every session — defeating its own purpose. Mandate paths: frontmatter (path-specific rules) on every topic file, threaded through the research/classify/approve pipeline so a backfill is visible at the approval gate rather than a silent rewrite. Apply the fix to this repo's claude-memory-architecture.md, which was loading 1.5k every session.
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 24, 2026

Claude finished @gupsammy's task in 5m 46s —— View job


PR #74 Review

  • Gather context and fetch PR diff
  • Review skill files (SKILL.md)
  • Review agent files
  • Review plugin manifests
  • Review .claude/rules changes
  • Post findings

What looks good

No Python changes. All modified files are Markdown skill/agent definitions or JSON manifests. The stdlib-only, from __future__ import annotations, and SQLite rules don't apply here.

Version sync is correct across all four locations.

  • claude-memory bumped 0.8.101 → 0.8.102 in plugin.json, marketplace.json, root README.md, and plugins/claude-memory/README.md. ✓
  • claude-coding bumped 0.2.53 → 0.2.54 in all four matching locations. ✓

memory-auditor.md — value-based retirement is well-specified.
Step 6 adds three new retirement categories (SUPERSEDED / REDUNDANT / LOW-VALUE) and cleanly distinguishes the evidence standard: factual findings (STALE / CONTRADICT / DATE_FIX) require codebase evidence; corpus-internal findings (SUPERSEDED / REDUNDANT / LOW-VALUE) require a quote from a competing entry. The updated Quality Rules section (line 84-88) threads this distinction correctly. The description stays within ~65-70 tokens (borderline but acceptable given the semantic addition).

signal-discoverer.md — "raise the bar" rule is tight.
Adding "returning 0 is a valid, valuable result, not a failure" directly addresses the padding failure mode. Placement in Quality Rules is correct.

extract-learnings/SKILL.md — adaptive clustering is coherent end-to-end.

  • Memory hierarchy table adds layer 2c cleanly.
  • Uncapping retirements/merges (only ADD candidates are capped at 3-7) is the right policy — auditor findings shouldn't compete for budget with new captures.
  • Phase 3 step 4 now explicitly includes MERGE as an action (ADD / EDIT / MERGE / REMOVE), consistent with the auditor producing MERGE findings.
  • Step 7 refactored from a dead size gate (170 lines total) to per-section overflow at 25 entries per section. The adaptive clustering trigger and migration path are fully specified.

update-claudemd/SKILL.md — backfill pipeline is correctly threaded.
The has_paths_frontmatter field on Agent A's return, the new add-paths-frontmatter → <glob> action in Phase 3, the "Paths frontmatter: added" line in Phase 5 presentation, and the backfill step in Phase 6 form a complete pipeline: detect → classify → surface at approval gate → execute. The PR description's claim that backfill "surfaces at the approval gate instead of being a silent rewrite" is accurate.

.claude/rules/claude-memory-architecture.md — fix is correct.
The committed version on this branch adds:

---
paths:
  - "plugins/claude-memory/**"
---

Scoping to plugins/claude-memory/** is the right glob — it covers hooks, skills, agents, and the plugin manifest without over-matching.


Must fix before merge

Nothing blocking. The committed state on this branch is correct.

One working-tree note (not a merge blocker): git status shows .claude/rules/claude-memory-architecture.md with an unstaged modification that reverts the frontmatter. The committed version is correct; CI builds from committed state. But if someone runs git commit -a from this working tree, the fix would be silently reverted. Worth a git checkout -- .claude/rules/claude-memory-architecture.md to clean the working tree before merging.


Optional suggestions

  1. YAML style consistency (inline comment on update-claudemd/SKILL.md:195): Phase 6 step 3 uses paths: ["plugins/*/hooks/**"] (inline JSON-array), while the canonical example 15 lines later uses the YAML block-sequence form. Both are valid; the block form is more readable and matches the example.

  2. Glob memory/**/*.md qualifier (inline comment on extract-learnings/SKILL.md:64): the "from resolved path" qualifier is load-bearing since Glob runs relative to the session cwd, not the memory directory. Keep that phrasing if the section is ever reworded.

  3. memory-auditor.md:63: "a consolidation that retires nothing is suspect" — good phrasing; worth preserving as-is if this line is ever revised.

3. Apply topic file actions in this sub-order: delete orphaned topic files the user approved for removal, then write new topic files, then edit existing topic files (applying keep/update/delete/promoted-out/moved-in actions). Promoted-out sections are removed from the topic file in this step; they reappear in CLAUDE.md in step 4
3. Apply topic file actions in this sub-order:
- Delete orphaned topic files the user approved for removal
- Write new topic files; each must carry `paths:` YAML frontmatter scoped to the globs its subject covers (e.g. `paths: ["plugins/*/hooks/**"]`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: inline JSON-array form (paths: ["plugins/*/hooks/**"]) differs from the YAML block-sequence form used in the canonical example ~15 lines below. Both are valid YAML, but the block form reads more clearly and matches what the example shows. Consider:

Suggested change
- Write new topic files; each must carry `paths:` YAML frontmatter scoped to the globs its subject covers (e.g. `paths: ["plugins/*/hooks/**"]`)
- Write new topic files; each must carry `paths:` YAML frontmatter scoped to the globs its subject covers (e.g. `paths:\n - "plugins/*/hooks/**"`)

Or simpler — just drop the inline form since the full example already shows the correct shape.

- REDUNDANT: overlaps a sibling entry and adds no distinct value → REMOVE or MERGE.
- LOW-VALUE: too generic, obvious, or narrow to change future behavior → REMOVE.
Evidence here is corpus-internal (quote the competing/overlapping entry), not codebase.
Downward pressure is the goal: a consolidation that retires nothing is suspect.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"a consolidation that retires nothing is suspect" — deliberate, well-stated policy. Worth keeping exactly as phrased; softer wording like "should be reviewed" would dilute it.

Steps 2-4 are required and run as parallel tool calls.

2. Read MEMORY.md + list topic files (`Glob memory/*.md` from resolved path)
2. Read MEMORY.md + list topic and cluster files (`Glob memory/**/*.md` from resolved path)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pattern upgrade from memory/*.mdmemory/**/*.md is correct for the new clusters/ sub-directory. Just a reminder that Glob runs relative to the cwd of the main session, not to the resolved memory directory — the "from resolved path" qualifier in the instruction is load-bearing here and should survive if this section is ever reworded.

Follow-up to 5261ef5. An audit of a live consolidation run showed the new pruning generated retirements it could not execute (no deletion tool) and reported them done anyway, and that plain-name index pointers never drove lookups.

- extract-learnings: grant Bash(trash:*); Phase 4 deletes via trash then verifies absence with Glob before reporting success; enforce retire->merge->cluster order so clustering cannot mask removable entries; add condition-first Pointer Format (triggered vs always-on)
- memory-auditor: merge criterion now fires on shared slug-prefix + same subject, not only >50% textual overlap
- signal-discoverer: propose entries with condition-first pointers
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 25, 2026

Claude finished @gupsammy's task in 2m 35s —— View job


PR #74 Review — updated pass (commit b5a572a)

  • Gather context
  • Fetch diff and identify new commit
  • Review b5a572a feat(memory): make pruning executable and index pointers trigger-based
  • Post findings

This pass covers the single new commit added since the previous review (b5a572a, 2026-05-25). The two earlier commits (5261ef5, ccde525) were reviewed on 2026-05-24 and had no blockers; those findings stand.


What the new commit addresses

The commit message explains an operational failure: a live consolidation run produced REMOVE/MERGE proposals it couldn't execute (no deletion tool) and then reported them done anyway, and MEMORY.md index pointers written as bare filenames were never opening files. The changes fix both:

  1. extract-learnings/SKILL.md: adds Bash(trash:*) to allowed-tools; rewrites Phase 4 execution order (REMOVE → MERGE → Cluster → ADD/EDIT); mandates Glob verification of each deleted file before marking done; adds a "Pointer Format" section defining condition-first pointer shapes.
  2. memory-auditor.md: broadens merge criterion from single rule (>50% overlap) to any-one-of-three (overlap OR shared slug-prefix+subject OR same entity/decision); adds pointer-format rule to Quality Rules.
  3. signal-discoverer.md: adds matching pointer-format guidance to Quality Rules.
  4. Version bump: claude-memory 0.8.102 → 0.8.103 in all four required locations. ✓

What looks good

Execution ordering is now correct and explicit. The retire → merge → cluster → add sequence prevents clustering from masking removable entries — exactly the failure mode the commit describes. The ordering guard ("Clustering never runs before retirement") is stated twice (steps 7 and Phase 4), which is the right level of emphasis for something that silently failed before.

Bash(trash:*) instead of rm is the right call. Reversibility matters when the skill is autonomously deleting topic files. The permission scope is correctly minimal (trash:* rather than Bash(*)).

Glob verification before marking done is a meaningful guard. Tying Phase 4 completion to Glob memory/**/*.md confirmation closes the exact gap the commit describes ("claiming a deletion that did not happen is the exact failure this guards against").

Pointer Format section is well-positioned. It sits between Memory Hierarchy and Early Exit Guard, so it's loaded before any phase that writes pointers. The two-shape definition (triggered vs. always-on) with a worked example is clear enough for an agent to follow without ambiguity.

Slug-prefix merge trigger is a good heuristic. A family of feedback_composite_* files covering the same topic is a stronger merge signal than content overlap alone, and naming it explicitly ("check these first") gives the auditor a priority order.

Version sync correct. All four locations (plugin.json, marketplace.json, root README.md, plugins/claude-memory/README.md) show 0.8.103. ✓


Must fix before merge

Nothing blocking from the new commit.

Working-tree note (carried from previous review, still present): git status and git diff HEAD confirm that .claude/rules/claude-memory-architecture.md has an unstaged local modification that removes the paths: frontmatter the PR is specifically fixing. The committed version at HEAD is correct — CI sees the right file. But a git commit -a from this working tree would silently revert the fix. Run git checkout -- .claude/rules/claude-memory-architecture.md before merging.


Optional suggestions

  1. trash availability on Linux. trash is available on macOS by default and via brew install trash or npm install -g trash-cli, but on headless Linux (CI, remote servers) it usually requires sudo apt install trash-cli and may not be present. If a user runs consolidation on a system without trash, Phase 4 silently fails or errors. Consider a one-line fallback check:

    Bash: trash <path> 2>/dev/null || mv <path> <path>.deleted
    

    Or document the dependency in the plugin README. Low priority given the interactive use case.

  2. Glob memory/**/*.md in Phase 4 is missing the "from resolved path" qualifier. Phase 1 step 2 and the inline review comment on step 2 (from the prior review) both note that Glob runs relative to session cwd, not the memory directory. The Phase 1 wording uses "from resolved path" as the load-bearing qualifier; Phase 4 drops it. An agent resolving memory/**/*.md against a repo root will look in the wrong place and report all files as absent. Worth threading the resolved path explicitly: Glob memory/**/*.md from <memory-dir> as resolved in Phase 1.

  3. Phase 3 step 7 trailing sentence is dense. "Early-exit captures (no auditor findings) skip (a) and run (b) only if the new entry pushes a section past 25" is appended to an already-long paragraph. Moving it to its own numbered point or the Early Exit Guard section would improve readability without changing semantics.

  4. Consolidation marker comment regression (minor). The previous text "write consolidation marker using Bash (Write tool requires prior Read and cannot create new files)" carried the technical rationale for using Bash instead of Write. The new text loses this hint. An agent unfamiliar with the Write-requires-Read constraint may reach for Write and fail silently. Worth keeping a parenthetical: (use Bash — Write requires a prior Read of the file to exist).


gupsammy added 2 commits June 1, 2026 11:38
Phase 4 Glob verification dropped the from-resolved-path qualifier present
in Phase 1, risking a glob against session cwd that reports all files
absent. Marker step lost the rationale for using Bash over Write. Restore
both per PR #74 review.
Drop the inline JSON-array paths example that diverged from the canonical
block-sequence form shown in the full example below. Matches the YAML
block-sequence convention. Per PR #74 review.
@gupsammy gupsammy merged commit 9415bf2 into main Jun 1, 2026
@gupsammy gupsammy deleted the feat/memory-self-management branch June 1, 2026 06:08
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 1, 2026

Claude finished @gupsammy's task in 2m 15s —— View job


PR #74 Review — final pass (post-merge, all 5 commits)

  • Gather context
  • Fetch PR diff and read changed files
  • Review skill files (SKILL.md)
  • Review agent files
  • Review plugin manifests and version sync
  • Review .claude/rules changes
  • Post findings

What looks good

No Python changes. All modified files are Markdown skill/agent definitions or JSON manifests. The stdlib-only, from __future__ import annotations, and SQLite rules don't apply.

Version sync is correct — final state.

  • claude-memory: 0.8.101 → 0.8.104 across all four locations (marketplace.json, plugin.json, root README.md, plugins/claude-memory/README.md). ✓
  • claude-coding: 0.2.53 → 0.2.55 across all four locations. ✓

The three-bump increment on claude-memory reflects the five commits in the PR — the auto-version hook ran once per commit that touched plugin code, which is correct behaviour.

.claude/rules/claude-memory-architecture.md — fix confirmed in merged state.
The committed and now-merged file opens with:

---
paths:
  - "plugins/claude-memory/**"
---

The previously-flagged working-tree divergence (unstaged deletion of this frontmatter) is not present on the merged main branch. The file is clean.

update-claudemd/SKILL.md — YAML style suggestion addressed.
Phase 6 step 3 no longer uses the inline JSON-array form (paths: ["..."]). It reads "as a block sequence (the full example below shows the shape)" and the canonical example fifteen lines later shows the block form — consistent end-to-end. The optional suggestion from the first review pass is resolved.

extract-learnings/SKILL.md — "from resolved path" qualifier present in both Phase 1 and Phase 4.

  • Phase 1 step 2: Glob memory/**/*.md from resolved path ✓
  • Phase 4 verify clause: run Glob memory/**/*.md from resolved path

The second-review suggestion about the missing qualifier in Phase 4 is resolved.

Consolidation marker comment restored.
Line 130: "Use Bash, not Write — Write requires a prior Read and cannot create a new file." The rationale lost in an earlier commit is back. ✓

Phase 3 step 7 remains dense but now self-contained.
The step covers a lot of ground in one paragraph (line 109). It's long but the logic is sequential and correct: retirement must precede clustering, clustering must not mask removable entries, early-exit path skips retirement. No behavioural issue; a future refactor could split it into sub-bullets.


Must fix before merge

Nothing — PR is already merged.


Optional suggestions (not carried forward)

  1. trash on headless Linuxtrash requires trash-cli (apt install trash-cli) on Linux servers/CI. If a user runs consolidation on a system where it isn't installed, Phase 4 will error. Low priority given the interactive use case, but worth a one-liner note in plugins/claude-memory/README.md under dependencies, or a fallback in Phase 4: trash <path> 2>/dev/null || mv <path> <path>.bak.

  2. Phase 3 step 7 density — the early-exit sub-clause ("Early-exit captures… skip (a) and run (b) only if the new entry pushes a section past 25") is appended to an already long paragraph. Promoting it to its own numbered sub-point inside the Early Exit Guard section would aid scannability without changing semantics.

Both were noted in prior reviews and left as-is by choice. Neither blocks correctness.


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.

1 participant