Skip to content

feat(extraction): drop candidates that echo a recent manual store - #963

Open
gorkem2020 wants to merge 4 commits into
CortexReach:masterfrom
gorkem2020:feat/manual-echo-guard
Open

feat(extraction): drop candidates that echo a recent manual store#963
gorkem2020 wants to merge 4 commits into
CortexReach:masterfrom
gorkem2020:feat/manual-echo-guard

Conversation

@gorkem2020

Copy link
Copy Markdown
Contributor

Problem

When a user stores a fact manually with memory_store (or memory_update), the same conversation turn usually still flows through auto-capture extraction. The extractor then re-emits the just-stored fact as a candidate, and the pipeline spends judge, dedup, and merge LLM calls deciding what to do with a row that already exists verbatim. In the noisiest case the near-duplicate survives as a second row.

Fix

A small deterministic echo guard:

  • Manual memory_store / memory_update texts are recorded in an in-memory per-agent ring (8 entries, no persistence, no config surface).
  • Extraction candidates near-identical to a recorded text are dropped before the admission judge, with an INFO log line. Matching is deterministic and cheap: normalized containment, token-subset ratio >= 0.9 (both directions), or Jaccard >= 0.75. No LLM calls, no vector reads.
  • The manual row itself is untouched; it was already stored verbatim by the manual-priority lane.

Paraphrase-level echoes (same meaning, different words) are deliberately out of scope: covering them would need embedding lookups and would risk dropping genuinely new memories. The word-level tiers plus the existing dedup layer cover the observed failure shape.

Notes

  • Stacked on #960 (manual-priority supersede) because both touch the same tools.ts region; this diff is the last commit on the branch. Best reviewed after feat(tools): manual-priority supersede for memory_store #960 merges.
  • New unit suite test/manual-echo-guard.test.mjs (17 cases: tier matches, short-text guard, per-agent isolation, ring eviction); registered in the test chain and CI manifest.

🤖 Generated with Claude Code

@gorkem2020
gorkem2020 marked this pull request as ready for review August 29, 2026 12:24
@gorkem2020

Copy link
Copy Markdown
Contributor Author

Recomposed against current master (post-#959/#947 merges) as a single commit carrying only this family's delta. Full suite green locally. Ready for review.

@gorkem2020
gorkem2020 force-pushed the feat/manual-echo-guard branch from 2b40a6a to dfe94e9 Compare August 29, 2026 12:24

@rwmjhb rwmjhb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed head dfe94e9. The focused tests, full suite, and CI are green, but the guard currently trades duplicate rows for silent loss of legitimate memory updates.

  1. Cross-turn fuzzy matching can discard corrections and qualified supersets. Matching is bidirectional and uses unordered token subsets without negation or qualifier awareness, while ledger entries have no TTL or consume-on-match rule. Both Alice no longer works at Acme and Alice works at Acme until Friday match a recorded Alice works at Acme and are dropped before admission. Ordinary expansions are also vulnerable because stopwords remain in the token set and a candidate containing 90% of the manual tokens is discarded wholesale. Please scope entries to a short, explicit lifetime/origin and make matching conservative for negation, changed values, temporal qualifiers, and candidates carrying additional facts; add regressions for each.

  2. The successful temporal memory_update path is not recorded. Preference/entity supersedes return at src/tools.ts:2179-2192, before the only update-side manualEchoLedger.record() call. Record the new text after the superseding write succeeds and before that return, with handler-level coverage.

  3. Echo-only batches are treated as barren and retried. After all candidates are filtered, the result reports zero created/merged/skipped and no settled outcome. The auto-capture caller therefore defers the ingress text after already charging the extraction limiter, so later flushes can repeat the same model call. Count echo drops or otherwise mark an echo-only successful run as settled, and test the full auto-capture path.

  4. Wrapped CJK echoes bypass matching. tokenSet() splits only on whitespace, and the fewer-than-three-token guard runs before containment. A wrapped Chinese or Japanese sentence therefore fails unless it is byte-for-byte exact. Use a Unicode character/grapheme fallback or a conservative CJK containment path and add focused cases.

  5. memory_forget leaves stale suppression state. ManualEchoLedger.clear() has no production caller, so a deleted manual fact can still suppress a later re-statement until write-count eviction or restart. Invalidate the relevant ledger entry on deletion, or make lifetime/consumption semantics remove this stale state safely.

Requesting changes on this head.

… settled echo batches, CJK fallback, forget invalidation

Round-1 review: the guard must never trade duplicate rows for silent loss
of legitimate updates.

1. Matching is now one-sided: a candidate is an echo only when it adds
   NOTHING beyond the manual text (exact, manual-contains-candidate, or
   full content-token containment after glue-word stripping). Negation and
   temporal markers on either side refuse the match, so corrections,
   changed values, qualified statements, and added facts always survive.
   The bidirectional Jaccard/subset fuzz is gone.
2. Ledger entries carry a 10-minute TTL and are consumed on match: one
   manual store suppresses at most one echo, so later identical statements
   are deliberate re-assertions and never dropped.
3. The successful temporal memory_update supersede path records the new
   text before its early return (handler-level regression included).
4. An echo-only batch counts its drops as skipped and reports
   settledOutcomes, so the auto-capture caller consumes the input instead
   of deferring a retry that re-runs the same extraction.
5. CJK fallback: whitespace-stripped containment with a marker-guarded
   wrapper budget, so wrapped CJK echoes drop while qualified or negated
   CJK statements survive.
6. memory_forget invalidates the deleted row's ledger entry on both the
   id and query paths.

test/manual-echo-guard.test.mjs rewritten around the new contract (31
tests incl. full auto-capture path + handler-level supersede coverage).
@gorkem2020

Copy link
Copy Markdown
Contributor Author

Round 1 addressed on head 9bc03dd, all five accepted. The premise of your review was right: the old matcher optimized for catching echoes at the cost of eating legitimate updates, and the redesign inverts that priority (the worst case of the guard staying quiet is one duplicate row for dedup, which was the pre-guard status quo).

  1. Matching is now ONE-SIDED and conservative: a candidate is an echo only when it adds nothing beyond the manual text (exact match, manual-contains-candidate, or full content-token containment after glue-word stripping). Negation/temporal markers on either side refuse the match. The bidirectional subset and Jaccard paths are gone. Regressions cover: negated candidate, negated manual text, temporal qualifier (until friday), changed value, and a candidate carrying additional facts, each asserted to survive.

  2. Ledger entries now have a 10-minute TTL and are consumed on match: one manual store suppresses at most one echo, so a later identical statement is a deliberate re-assertion and always survives. Both behaviors have direct regressions (injectable clock).

  3. The successful temporal memory_update supersede path records the new text before its early return, with handler-level coverage (context double driving the real memory_update handler, asserting the action and the ledger hit).

  4. Echo drops now count as skipped and an echo-only batch reports settledOutcomes, so the auto-capture caller consumes the input instead of deferring a retry. Covered end-to-end through the full plugin path (manual store via the real memory_store tool, then agent_end extraction whose only candidate echoes; asserts the settled log and single extraction).

  5. Wrapped CJK echoes now drop via a whitespace-stripped containment fallback with a marker-guarded wrapper budget, while qualified (直到...) or negated (不再...) CJK statements survive; four focused cases added.

  6. memory_forget invalidates the deleted row's ledger entry on both the id path (row text fetched before delete) and the query path.

Full suite and manifest verifier green; the echo-guard file runs 31 tests.

@rwmjhb rwmjhb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed head 9bc03dd. The previous temporal-update, echo-only settlement, TTL, and tool-level forget fixes are present, and the focused/full suites plus CI pass. The revised matcher still has correctness blockers:

  1. Consume-on-hit fails once an agent has multiple live entries. match() creates live with filter(), but writes that array back to byAgent only when its length differs from ring. In the normal no-expiry case, live.splice(i, 1) mutates a detached copy while the Map still retains the matched entry. The single-entry test passes only because the empty-copy branch deletes the whole key. With two recent manual writes, a hit can therefore suppress repeated re-statements for the entire TTL. Always persist the post-splice array, and add a two-live-entry regression that matches one entry twice.

  2. The CJK branches still silently drop changed or additional facts. The shortened branch checks only man.includes(cand) and never inspects the removed residual, so 喜欢喝茶和咖啡 is treated as an echo of the opposite manual statement 用户不喜欢喝茶和咖啡. The wrapped branch treats any marker-free residual of at most eight characters as glue: on the current runtime, isNearIdenticalEcho("我住在北京市海淀区并养猫", "我住在北京市海淀区") returns true, dropping the new fact 并养猫. Match only known wrapper forms or otherwise prove the residual is non-substantive; cover both exact cases.

  3. English token-set containment loses direction. The final rule ignores order and grammatical roles after stopword removal, so Alice reports to Bob and Bob reports to Alice reduce to the same set and the reversed relationship is discarded. Require order-preserving/structural containment rather than bag-of-words subset membership, with directional relationship regressions.

Also route every deletion lane through targeted ledger invalidation (CLI delete/delete-bulk currently only invalidates reflection caches), and avoid the extra getById on memory_forget when smart extraction is disabled.

Requesting changes on this head.

…ive-residual CJK checks, order-preserving containment, deletion-lane invalidation

Round-2 review:

1. match() now persists the post-splice array in every outcome: with two
   or more live entries the consume mutated a detached filter() copy while
   the Map kept the matched entry, so one manual store could suppress
   repeated re-statements for its whole TTL. Regression: two live entries,
   the same candidate matched twice consumes exactly one entry.
2. CJK residuals must be proven non-substantive: the shortened branch
   rejects a match when the REMOVED manual residual carries a marker
   (stripping the negated wrapper yields the opposite claim), and the
   wrapped branch accepts only residuals composed entirely of known
   reporting-glue fragments (short and marker-free was not enough: a
   three-character residual can be a new fact). Both review cases covered
   exactly.
3. English containment is order-preserving: candidate content tokens must
   appear in the manual text as an ordered subsequence, so reversed
   relationships (alice reports to bob vs bob reports to alice) survive.
4. Every deletion lane invalidates the ledger: CLI delete fetches the row
   pre-delete and invalidates its text across all agent buckets, CLI
   delete-bulk clears the ledger wholesale (fail-open), and the ledger is
   wired into contexts only while smart extraction is enabled, which also
   spares memory_forget its pre-delete getById fetch when disabled.
@gorkem2020

Copy link
Copy Markdown
Contributor Author

Round 2 addressed on head 72290ed, all three blockers plus both tail items:

  1. Consume persistence: confirmed, my bug. match() now persists the post-splice array in every outcome (hit with survivors, hit emptying the ring, and expiry-only shrink), so the consume mutates the Map's state rather than a detached filter() copy. Regression added with two live entries: the same candidate matched twice consumes exactly the matched entry, and the sibling entry survives.

  2. CJK residual substance: both branches now prove the residual non-substantive instead of assuming it. The shortened branch rejects the match when the REMOVED manual residual carries a marker (your exact case: stripping 用户不 off the negative statement yields the opposite claim, kept). The wrapped branch accepts only residuals composed entirely of known reporting-glue fragments (用户说/提到/です and friends), so 并养猫 survives as the new fact it is; short-and-marker-free is no longer sufficient. Both review cases are covered verbatim.

  3. Direction: English containment now requires the candidate's content tokens to appear in the manual text as an ORDERED subsequence, so "Alice reports to Bob" no longer collapses onto "Bob reports to Alice" (regression covers both the reversed pair surviving and the same-order wrap echo still dropping).

Tail items: every deletion lane now invalidates the ledger — CLI delete fetches the row before deleting and invalidates its text across all agent buckets (the CLI cannot name the writing agent), CLI delete-bulk clears the ledger wholesale (fail-open: worst case is one uncaught echo for dedup to handle), and memory_forget keeps its targeted invalidation. The ledger is wired into the tool and CLI contexts only while smart extraction is enabled, which also removes memory_forget's pre-delete getById fetch in the disabled configuration.

Focused file 37/37, full suite, typecheck, and manifest verifier green.

@rwmjhb rwmjhb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed head 72290ed. The consume persistence, CJK residual, ordered-direction, deletion wiring, and disabled-extraction fixes from the previous round are present, and the focused/full suites plus CI pass. The matcher still has a silent-loss blocker:

  1. Semantic verbs are stripped, allowing different assertions to match as echoes. ECHO_STOPWORDS removes has, wants, likes, and prefers, and the ordered-subsequence path applies no minimum to the candidate content tokens. On this head, all of these are classified as echoes and dropped:
    • candidate User wants a golden retriever named Max vs manual User has a golden retriever named Max;
    • candidate User prefers Python vs manual User prefers Go over Python for backend services;
    • candidate User likes tea vs manual User prefers coffee over tea in the morning.

These candidates assert a different relation or preference, yet the guard increments skipped and settles the input, so the memory is silently lost. Keep only true reporting glue in the stopword set, preserve semantic predicates, and require a meaningful candidate-side content floor before subsequence matching. Add the exact three regressions above while retaining the legitimate same-order wrapper cases.

  1. Supersede/update leaves the replaced text in the ledger. The new text is recorded, but the prior row text is not invalidated. A quick reversal back to the old statement can therefore consume that stale entry and be dropped even though the store no longer contains the old fact. Invalidate the replaced text on every successful update/supersede path.

The CLI invalidation additions are process-local and cannot clear the running gateway's in-memory ledger; the extra pre-delete read and clearAll() only affect the short-lived CLI process. Please remove that ineffective work or introduce an actual cross-process invalidation mechanism, and update the stale _initPluginState comment to match the conditional wiring.

Requesting changes on this head.

…ontent equality for wrap echoes, invalidate replaced text on update and supersede

Review round 3 follow-ups:
- ECHO_STOPWORDS is reporting glue only; has/have/had, wants/want,
  likes/like, prefers/prefer are content tokens again, so a candidate that
  swaps a predicate is never an echo
- the wrap-echo path requires the candidate's content tokens to EQUAL the
  manual text's, in order, instead of forming an ordered subsequence: a
  candidate that drops distinguishing content ("User prefers Python" against
  "User prefers Go over Python for backend services") is a different
  assertion and survives; the manual-side minimum doubles as the candidate
  floor since equal sequences have equal length
- memory_store supersede, memory_update temporal supersede, and the plain
  memory_update path invalidate the replaced row text in the ledger before
  recording the new text, so a reversal back to a replaced statement is
  never treated as an echo of a fact the store no longer holds
- the CLI ledger wiring (pre-delete getById, invalidateEverywhere, clearAll)
  is removed: the CLI runs in its own process and could never reach the
  gateway's in-memory ledger; the class documents the per-process scope and
  the TTL-bounded window it implies
- the ledger construction comment in index.ts matches the conditional
  wiring (recording happens only when a smart extractor exists)
- regressions: the three predicate/subsequence cases, a predicate-carrying
  wrap echo that must still collapse, and replaced-text invalidation on all
  three write paths
@gorkem2020

Copy link
Copy Markdown
Contributor Author

Round 3 addressed on head 1855fdc.

  1. ECHO_STOPWORDS is reporting glue only now (articles, copulas, pronouns, prepositions, reporting verbs); has/have/had, wants/want, likes/like, prefers/prefer are content tokens again. The wrap-echo path also changed shape: instead of an ordered subsequence it requires the candidate's content tokens to equal the manual text's content tokens, in order, so a candidate that adds a token OR drops one is never an echo. That is what makes "User prefers Python" survive against "User prefers Go over Python for backend services" (a subsequence match with any floor short of full coverage would still have dropped a three-token variant of it), and the manual-side minimum doubles as the candidate floor since equal sequences have equal length. The three regressions are in verbatim, plus a predicate-carrying wrap echo ("User mentioned that Alice prefers Go for backend services" against "alice prefers go for backend services") that must still collapse; the existing same-order wrapper cases are unchanged. The verbatim-substring shortening path is untouched: it only fires when the candidate is a contiguous piece of the manual text, so it cannot produce the swapped-predicate or dropped-qualifier class.

  2. Every successful update or supersede path now invalidates the replaced row text before recording the new one: memory_store supersede (each confirmed superseded target), the memory_update temporal supersede, and the plain memory_update path. Handler-level regressions cover all three (record old text, perform the write, the old text no longer matches, the new text does).

  3. The CLI ledger wiring is removed (the pre-delete getById, invalidateEverywhere, clearAll, and the CLIContext field), since a memory-pro process can never reach the gateway's in-memory ledger. The class doc now states the per-process scope and the window it implies (a CLI delete leaves its text suppressible for at most the 10-minute TTL). The _initPluginState comment matches the conditional wiring.

Gates: tsc, full suite, cli-smoke group, and build (dist committed) are green.

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