fix(#287): memory correctness cleanups (contradiction fallback, note merge, batch dedup)

[sshekhar563]

Resolves #287

Description

Three related correctness gaps in the memory pipeline (found during 2026-07-03 dev audit, commit 2a08e9):

Bugs Fixed

1. Raw-text contradiction fallback stored a sentinel, not a replacement fact

   • manager.py:1163 set superseded_by = "raw-text-contradiction" (a string literal in an ID field) and never stored the new fact
   • Semantic recall filters on superseded_by is None, so the old fact vanished — but no replacement was ever written → knowledge loss
   • Fix: Store the raw text as a real MemoryEntry, use its actual ID for superseded_by, set supersedes back-edge, and append a supersede-audit record

2. note() MERGE had no provenance trail

   • The MERGE branch at soul.py:1604 set old.superseded_by = new_id but never set new.supersedes = old_id and never appended a supersede-audit record
   • _walk_supersedes_chain() relies on the supersedes field, so note-driven merges were completely invisible to provenance walks
   • Fix: Look up the new entry via _memory_lookup_sync(), set supersedes, and append audit record with reason: "note-merge"

3. observe() batch dedup missed within-batch duplicates

   • existing_facts = self._semantic.facts() was snapshotted once at manager.py:1053 before the dedup loop
   • Two near-identical facts extracted from the same interaction (e.g. "User likes Python" and "User enjoys Python") were never compared — both passed dedup and persisted
   • The dream sweeper only catches overlap ≥ 0.85, so 0.6–0.84 band pairs survived indefinitely
   • Fix: Append each stored fact to existing_facts after self.add() in both MERGE and CREATE branches

Changes

File	Change
src/soul_protocol/runtime/memory/manager.py	Bug 1 + Bug 3 fixes, updated file header
src/soul_protocol/runtime/soul.py	Bug 2 fix (note MERGE provenance), updated file header
tests/test_memory/test_correctness_287.py	[NEW] 4 regression tests

Tests

• test_raw_text_contradiction_uses_real_id — verifies no fact has sentinel superseded_by
• test_note_merge_sets_supersedes_backedge — verifies new MERGE entry has supersedes set
• test_note_merge_records_audit_trail — verifies supersede_audit contains the merge record
• test_observe_batch_dedup_appends_to_existing — verifies near-duplicate notes get MERGE/SKIP, not double CREATE

422 existing tests passing, 4 new regression tests passing.

[prakashUXtech]

Good direction, and two of the three items are genuinely fixed in the code. I verified by running your tests and my own reproductions against both dev and this branch, so the following is measured rather than read.

What's solidly right:

• Item 3 (batch dedup) is fully fixed. Appending each stored fact back into the working set is exactly the right shape, and you got the two subtle parts right: facts() returns a fresh sorted list so the append can't corrupt the store, and the elements stay live references so the superseded_by mutation still persists. Measured: "I like Python. I prefer Python for backend work." in one interaction gives 2 live facts on dev and 1 here.
• Item 2's back-edge and audit trail work. supersedes is set and _walk_supersedes_chain now returns a walkable chain where dev returned [].
• Item 1's sentinel is gone and the knowledge no longer vanishes. new_id now carries a resolvable id, so provenance output in diff.py and the CLI resolves properly.

Two blockers:

1. Internal supersession is now written into the public supersede_audit. That list is documented as a user-intent trail in three places: the supersede_audit docstring (manager.py:1971-1982), docs/api-reference.md:508, and docs/cli-reference.md:964, all saying internal supersession (dream-cycle dedup, contradiction resolution) does not append here. After a single observe() this branch appends a verb-fact-conflict record, so the property no longer means what it advertises. The existing guard test passed only because it simulates internal supersession by mutating the entry directly instead of driving observe(). Could you route these to a separate internal list (or tag them and filter the public property), and while you're there add the documented tier and prediction_error fields and use datetime.now(UTC)? Right now the list mixes naive and aware timestamps, so sorting it raises TypeError.

2. One message contradicting N facts stores N identical replacements. The MemoryEntry is constructed inside the per-contradiction loop (manager.py:1174, loop opens at :1165), so I get two byte-identical live facts with different supersedes values from a single interaction. It also bypasses reconcile_fact, so nothing downstream collapses them. That is the exact duplicate-accumulation class item 3 exists to remove.

On the replacement itself, I'd push back on the approach a little. Storing interaction.user_input verbatim promotes raw first-person chat text into the semantic tier, which by convention holds normalized third-person facts. It is unbounded and unsanitized, arrives with no category or abstract, and is hard-coded to importance=5 while superseding an importance-7 fact, so the replacement is more likely to be evicted than what it replaced. The cheaper and safer fix is probably to stop suppressing the old fact when there is no genuine replacement: report the contradiction in the returned dict and let the next extraction settle it. Nothing is lost, and the duplicate problem disappears. If you'd rather keep a replacement, hoist the construction out of the loop, run it through reconcile_fact, normalize it, and bound the length.

Two more before merge:

3. detect_contradictions is still a no-op. soul.py:1515-1516 is byte-identical to dev, TODO comment and all. So item 2 is only half-addressed and #239 is not closed. Either wire ContradictionDetector in here (the detector is already available on the manager, so it's a small addition) or say so plainly in the PR body so the checklist isn't ticked early.

4. Two of the four tests pass on unpatched dev, so they don't guard anything. test_raw_text_contradiction_uses_real_id never calls observe(), so the 4d contradiction path never runs and it asserts a code path that didn't execute didn't misbehave. test_observe_batch_dedup_appends_to_existing also never calls observe(); its first half asserts len(facts) >= 2 after two remember() calls, which is #251's bug asserted as expected behavior, and its second half exercises note()'s single-fact path, which already worked on dev. The two behaviors you actually fixed are the ones left unprotected. Useful triggers: for the 4d path, a user_input that the verb-fact patterns match but FACT_PATTERNS misses ("I reside in Amsterdam now" works); for intra-batch dedup, the Python example above.

Smaller notes: a SOCIAL note-merge writes the audit record but silently skips the back-edge, because _memory_lookup_sync only covers episodic, semantic, and procedural, so the audit claims a supersession the chain can't reconstruct. The note-merge also doesn't set superseded = True on the loser, which the contradiction detector and the procedural curator both branch on. And soul.py:1616 reaches through the façade into self._memory._supersede_audit, which is the coupling #284 is trying to remove.

Finally: the title should follow the repo pattern fix(memory): ... (#287) rather than putting the issue number in the scope slot, and the CHANGELOG plus the two doc pages above need updating since this changes documented behavior.

No regressions anywhere: I ran the full suite both sides and the only delta is your four new tests. The note() provenance work and the batch-dedup fix are both genuinely good, so this is mostly about the raw-text replacement approach and making the tests prove what they claim.

[prakashUXtech]

The three correctness blockers are genuinely resolved, and this time the tests prove it. I re-ran the new file against a reconstructed dev runtime and 6 of 7 tests fail without your changes, so they're real. Specifically: the internal supersession now goes to a separate _internal_supersede_log with the documented tier field and a UTC timestamp, so the public supersede_audit is no longer polluted; the per-contradiction loop no longer constructs a replacement at all, so the duplicate bug is gone by construction; and no raw user_input is written into the semantic tier anymore. That's the hard part done.

What's left:

1. The detect_contradictions block you added in note() (soul.py:1519-1535) is currently dead code. It calls the detector but the result is never returned or read, sets no contradicted_id, and its one test (test_note_detect_contradictions_wired) only asserts action in (CREATE, MERGE, SKIP), which is a tautology that passes on dev too. So it doesn't meet #239's contract and it's the one vacuous test in the file. It also has a live side effect: any semantic note() that trips the verb heuristic silently flips superseded=True on the matched fact. I'd either finish it per #239 (return the contradicted id, set the supersede pointers and prediction_error, gate it on CREATE, reference #239) or drop it from this PR, since it's outside #287's scope. Right now it's the worst of both.

2. A design point worth a decision: both search() and facts() filter recall on superseded_by is None, not the superseded boolean. The report-and-defer path sets superseded=True but leaves superseded_by=None, so a contradicted fact stays fully recallable. That's correct if the goal was only "stop losing knowledge", which it achieves, but it means contradiction resolution is a no-op for retrieval, so after note("User lives in Tokyo") both Berlin and Tokyo stay live. If that's intended, fine; if not, it's a good tracked follow-up rather than expanding this PR.

3. The SOCIAL note-merge still skips the back-edge: _memory_lookup_sync covers episodic, semantic, and procedural but not social, so a social note-merge writes the old entry's superseded_by and the audit record but not the new entry's supersedes. Either extend the lookup to social or skip the forward-pointer and audit for social too, and add a social test.

4. The PR body no longer matches the code after the force-push: it still describes the rejected "store the raw text as a real MemoryEntry" approach and lists two test methods that don't exist. Please rewrite it to describe report-and-defer with the actual test names, add a CHANGELOG line plus a one-line doc note for the new note-merge audit behavior, and retitle to fix(memory): ... (#287).

The correctness work is basically there, so this is mostly about the #239 block (finish or drop) and the paperwork.