fix: CJK-aware remember dedup with negation guard (#1021)

[ziishanahmad]

Problem

mem::remember dedup used jaccardSimilarity with a whitespace-only tokenizer (split(/\s+/)). This works for English but makes Chinese/Japanese/Korean text — typically written without spaces — look nearly disjoint. Repeated CJK preferences were saved as separate isLatest=true memories instead of superseding older entries.

Fix

1. CJK-aware tokenizer in jaccardSimilarity (src/state/schema.ts)

• Reuses the existing segmentCjk from src/state/cjk-segmenter.ts (already used by BM25 search)
• Adds CJK character bigrams so unsegmented no-space Chinese text produces useful overlap
• Fast path preserved: when no CJK characters are present, falls back to the original whitespace split (zero regression for English-only users)
• Punctuation normalized via \p{L}\p{N} Unicode property strip

2. Negation guard (src/state/memory-dedup.ts)

Prevents do X from superseding do not X (and vice versa) even when token overlap exceeds the 0.7 threshold. Supports:

• English: not, never, don't, do not, cannot, should not, must not, no
• CJK: 不要, 别, 勿, 無, 无, 不, なし, 않, 안, 못

If both sides have negation markers, they are treated as compatible (both say "don't do X") and the guard does not fire.

3. Wired into remember.ts

The dedup loop now checks !isNegationConflict() before superseding.

Tests

• test/remember-dedup-similarity.test.ts (19 tests): pure-function tests for jaccardSimilarity (English regression, CJK identical/near-identical/reordered/distinct/mixed) and isNegationConflict (English do/do-not, Chinese 使用/不要使用, both-negated, both-affirmative)
• test/remember-cjk-dedup.test.ts (6 tests): end-to-end mem::remember tests covering pure Chinese duplicate supersede, mixed Chinese/English supersede, distinct non-supersede, English negation conflict, Chinese negation conflict, cross-project isolation
• All existing tests still pass (40/40 new + existing, 61 pre-existing Windows-only failures unchanged)

Design decisions

• No new dependencies: reuses existing segmentCjk which optionally uses @node-rs/jieba (already in the tree for BM25)
• Deterministic: no LLM/embedding calls, keeps mem::remember low-latency
• Conservative negation guard: only fires when exactly one side has a negation marker
• Project isolation preserved: unchanged from existing logic

Closes #1021

Summary by CodeRabbit

• New Features

  • Improved memory deduplication for CJK text, making similar Chinese and mixed-language entries match more reliably.
  • Added safer handling for negated statements so opposite meanings are less likely to overwrite each other.

• Bug Fixes

  • Reduced incorrect superseding of memories when the newer and existing text conflict in meaning.
  • Improved similarity checks across punctuation, reordered phrases, and empty-text edge cases.

[vercel]

@ziishanahmad is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds CJK-aware tokenization to the Jaccard similarity function in schema.ts, introduces a new memory-dedup module with an isNegationConflict guard, and wires the guard into mem::remember's supersession logic to prevent superseding conflicting negated statements. Adds corresponding unit and integration tests.

Changes

CJK dedup and negation guard

Layer / File(s)	Summary
Negation-conflict guard helper src/state/memory-dedup.ts	New module with English/CJK negation regexes, a hasNegation helper, and exported isNegationConflict returning true when exactly one input contains a negation marker.
CJK-aware Jaccard tokenization src/state/schema.ts	Imports hasCjk/segmentCjk and adds tokenizeForJaccard, which segments CJK text and adds character bigrams; jaccardSimilarity now builds token sets via this helper.
Wire negation guard into remember supersession src/functions/remember.ts	Imports isNegationConflict and updates the supersession condition to require similarity > 0.7 AND no negation conflict.
CJK dedup end-to-end tests test/remember-cjk-dedup.test.ts	Mocks logger, mutex, tracker, config, KV, and SDK dispatch; verifies supersedence for pure Chinese and mixed-language duplicates, non-supersedence for distinct preferences, negation conflicts (English and Chinese), and cross-project isolation.
Similarity and negation unit tests test/remember-dedup-similarity.test.ts	Adds unit tests for jaccardSimilarity across English/CJK/mixed scenarios and edge cases, and for isNegationConflict across English and Chinese negation patterns.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Remember as mem::remember
  participant Schema as jaccardSimilarity
  participant Dedup as isNegationConflict

  Caller->>Remember: submit new memory content
  Remember->>Schema: compute similarity(new, existing)
  Schema-->>Remember: similarity score
  alt similarity > 0.7
    Remember->>Dedup: isNegationConflict(new, existing)
    Dedup-->>Remember: conflict true/false
    alt conflict is false
      Remember->>Remember: mark existing isLatest=false, save new as latest
    else conflict is true
      Remember->>Remember: keep existing memory, save new separately
    end
  else similarity <= 0.7
    Remember->>Remember: save new memory as separate entry
  end

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately describes the main change: CJK-aware remember deduplication with a negation guard.
Linked Issues check	✅ Passed	The PR implements the core #1021 requirements: CJK-aware deduplication, a negation conflict guard, and tests covering mixed-language and cross-project cases.
Out of Scope Changes check	✅ Passed	The changes stay focused on remember deduplication logic and tests, with no unrelated feature work evident in the provided summary.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/state/schema.ts (1)

108-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider lowercasing tokens in the fast path for consistency with the CJK path.

The fast path doesn't lowercase tokens (text.split(/\s+/).filter(...)) while the CJK path does (cleaned...toLowerCase() at line 120). This means the same logical inputs can produce different similarity scores depending on whether CJK characters are present. The current production caller lowercases beforehand so there's no practical impact today, but making tokenizeForJaccard self-contained avoids a subtle trap for future callers.

♻️ Suggested fix

   if (!hasCjk(text)) {
-    return new Set(text.split(/\s+/).filter((t) => t.length > 2));
+    return new Set(
+      text.toLowerCase().split(/\s+/).filter((t) => t.length > 2),
+    );
   }

🤖 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 `@src/state/schema.ts` around lines 108 - 111, The fast path in
tokenizeForJaccard currently returns whitespace-split tokens without normalizing
case, unlike the CJK branch, so update that branch to lowercase tokens before
filtering/adding them to the Set. Keep the behavior consistent with the
cleaned...toLowerCase() logic used in the CJK path, so the function is
self-contained and produces stable Jaccard tokens regardless of input casing.

🤖 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.

Nitpick comments:
In `@src/state/schema.ts`:
- Around line 108-111: The fast path in tokenizeForJaccard currently returns
whitespace-split tokens without normalizing case, unlike the CJK branch, so
update that branch to lowercase tokens before filtering/adding them to the Set.
Keep the behavior consistent with the cleaned...toLowerCase() logic used in the
CJK path, so the function is self-contained and produces stable Jaccard tokens
regardless of input casing.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7a4ca43c-c311-405a-92ff-6ac04d861856

📥 Commits

Reviewing files that changed from the base of the PR and between 93ae9bc and 1036034.

📒 Files selected for processing (5)

• src/functions/remember.ts
• src/state/memory-dedup.ts
• src/state/schema.ts
• test/remember-cjk-dedup.test.ts
• test/remember-dedup-similarity.test.ts