docs(claude.md): steer Claude toward soul_observe for facts (post-#231)#252
Merged
Conversation
Update the agent-facing memory guide so the MCP recommendation matches the post-#231 reality: - soul_observe is the dedup-aware path: it runs the full pipeline (extraction + reconcile_fact + self-model update), so facts pulled out of real user/agent turns collapse onto existing entries rather than appending duplicates. - soul_remember is now scoped to blunt forced writes — short episodic events, or cases where dedup is explicitly wrong. The bullet calls out that the CLI sibling 'soul remember' is on a deprecation path toward 'soul note' (#231 phase 2 / PR #236) and that the dedup-aware MCP variant will follow once it ships. Pure docs change. No runtime behavior or test impact. Refs #231, #251.
|
All quality checks passed. Thanks for the clean PR! |
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.
What this PR changes
One line in
.claude/CLAUDE.md, the agent-facing memory guide for this repo.The "During work" section was telling Claude to use
soul_rememberfor "facts that should persist across sessions." Post-#231 that's wrong.soul_rememberwrites throughMemoryManager.add()with noreconcile_fact, no contradiction check, no significance gate, so repeated calls with similar text just pile up. Issue #251 caught what that looks like in practice: one soul ended up roughly 51% duplicates before someone noticed and ransoul cleanupby hand.The updated guidance points Claude at
soul_observefor facts that come out of real user/agent turns, sinceobserveruns the full pipeline includingreconcile_fact.soul_rememberis now scoped to blunt writes where the agent has already decided dedup is wrong (short episodic events, unique timestamped log lines). The bullet also flags that the CLI siblingsoul rememberis on a deprecation path towardsoul note(#231 phase 2, currently PR #236) and that the MCP layer will pick up the dedup-aware variant once it ships.Task 1 finding: the CLI deprecation warning is already in flight
The brief asked me to verify whether the post-#231 deprecation of
soul rememberhad actually landed (no warning fires today) and to add aDeprecationWarningif the project clearly meant to ship one. Before touching code I read the history:DeprecationWarningwhensoul rememberis invoked, pointing atsoul observeand the--no-dedupflag." Intent is unambiguous.Soul.note()runtime +soul noteCLI. Phase 2 was deferred.warnings.warn(..., DeprecationWarning, stacklevel=2)toremember_cmd, prints aconsole.printdeprecation banner, updates the help text, README, and CLI reference, and has a passing test. The review on chore(cli): deprecate soul remember in favor of soul note (phase 2 #231) #236 left four small change requests (CHANGELOG entry, removal-horizon version, docs header marker, test sharpening); the contributor says they've addressed them and is waiting on the CI PR (ci: run pytest matrix on PRs targeting dev (#241) #242) to land first.Adding a second warning here would clobber that active community PR mid-review, so I skipped it. No new tracking issue needed either, since #231 (Phase 2) and #236 already cover the question.
One nuance at the runtime layer that's easy to misread:
Soul.note()'s docstring describesremember()as "a blunt append," andnote()itself falls through toremember()for episodic anddedup=False. The two coexist by design inside the runtime (noteis the smart wrapper that callsrememberunderneath for the no-dedup paths). The deprecation lives at the CLI layer, which is where #236 is doing the work.Why ship the docs update separately
The MCP recommendation is wrong today regardless of when #236 merges. The agent guide pointing at
soul_rememberfor facts is what produced the #251 bloat in the first place. Once #236 merges and asoul_noteMCP tool follow-up lands, this guide can be tightened again.Verification
git diff --statconfirms one file changed (.claude/CLAUDE.md, +2/-2).Soul.remember()(src/soul_protocol/runtime/soul.py:1391) andSoul.note()(src/soul_protocol/runtime/soul.py:1432) to confirm the runtime semantics described above. Read thesoul_rememberandsoul_observeMCP definitions (src/soul_protocol/mcp/server.py:621,:574) to confirm nosoul_noteMCP tool exists yet.Refs
DeprecationWarning(do not duplicate)Local testing