Skip to content

fix: entity-orphan guard + log message correction for memory system - #199

Merged
lishuceo merged 4 commits into
mainfrom
fix/memory-entity-binding
Apr 8, 2026
Merged

fix: entity-orphan guard + log message correction for memory system#199
lishuceo merged 4 commits into
mainfrom
fix/memory-entity-binding

Conversation

@lishuceo

@lishuceo lishuceo commented Apr 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • 实体溯源防护:提取记忆时,如果 fact/relation/decision 引用的实体(人名/项目名)在对话原文中找不到,直接丢弃该记忆
  • 修复日志文案"Memories injected into system prompt""into user prompt prefix",消除调试时的误导

Root Cause

记忆提取 LLM 会将聊天上下文中的碎片信息(如父群讨论的其他候选人的面试结果)错误地归到当前分析对象身上,并作为 fact 存入记忆库。后续分析新候选人时,这些虚假记忆被注入 system prompt,导致错误信息在候选人之间传播。

传播链:父群上下文混合 → LLM幻觉 → 存为fact → 注入新候选人分析 → 再存一条错误fact

Changes

  1. src/memory/extractor.ts

    • 提取 prompt 新增 entities 输出字段和实体溯源规则
    • 新增 filterUngroundedMemories() 硬校验:每个声明的 entity 必须在对话文本中有子串匹配
    • 仅对 fact/relation/decision 类型做校验(preference/state 关乎用户自身,不需要)
    • 向后兼容:如果 LLM 未返回 entities 字段,不拦截
  2. src/memory/injector.ts

    • 修正误导性日志文案

Test plan

  • 9 个新增测试覆盖 entity-orphan guard 各场景
  • 2 个新增测试覆盖 entities 字段解析
  • 全量回归 1218/1219 通过(1 个预存的 memory quality 失败无关)
  • 线上验证:分析候选人简历后检查记忆提取日志是否有 entity-orphan guard 拦截记录

🤖 Generated with Claude Code

lishuceo and others added 2 commits April 2, 2026 11:21
Memories are injected into user prompt prefix (executor.ts:481),
not system prompt. The stale log message caused confusion during
debugging.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the extraction LLM produces a fact/relation/decision that
references entities (person names, project names) not present in the
source conversation, the memory is now rejected instead of stored.

Two-layer defense:
1. Prompt rule: instructs extraction LLM to skip facts whose entities
   lack explicit textual evidence in the conversation
2. Code validation: filterUngroundedMemories() verifies every declared
   entity appears in the conversation text via substring match

This prevents the propagation chain where hallucinated facts about
one candidate get stored and later injected into analyses of
different candidates.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Apr 2, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 1m 9s —— View job


PR Review: Entity-Orphan Guard + Log Message Correction

  • Check for previous review comments — none found
  • Read changed files in full context
  • Analyze logic correctness and security
  • Post review summary

Approved — Clean, well-scoped fix with good test coverage.

Assessment

This PR adds a solid defense-in-depth mechanism against hallucinated entity extraction. The approach is sound: a hard substring check on declared entities, applied only to entity-bearing memory types (fact/relation/decision), with backward compatibility for LLMs that omit the entities field.

What looks good:

  • Clear separation of prompt-level guidance (entity tracing rules) + code-level enforcement (filterUngroundedMemories)
  • Correct backward compatibility — memories without entities field pass through
  • Type-selective validation — preference/state correctly skip the guard
  • Thorough test coverage (9 guard tests + 2 parsing tests covering all edge cases)
  • Log message fix in injector is accurate

Minor observations (not blocking):

  1. Entity matching is case-sensitive (conversation.includes(entity)) — this is fine for Chinese names (the primary use case), but if English entity names ever appear with different casing (e.g., "OpenAI" vs "openai"), they'd be rejected. Low risk given the use case but worth noting for the future.

  2. The guard relies on LLM cooperation — if the extraction model omits entities, the memory passes through unchecked. This is the correct trade-off for backward compatibility, but it means the guard's effectiveness depends on prompt adherence. The prompt changes are clear enough that this should work well in practice.

  3. No false-positive risk from substring matchingincludes() could technically match partial strings (e.g., entity "张三" found inside "张三丰"), but this would result in allowing a memory rather than rejecting it, so the error mode is safe (false negatives rather than false positives).

Overall this is a well-designed fix that addresses a real cross-contamination bug in the memory pipeline. The "prefer false negatives over false positives" philosophy is exactly right for a memory system.

9 new test cases:
- parseExtractionResponse: entities field parsing and missing field handling
- filterUngroundedMemories: grounded entities kept, orphaned entities
  rejected, partial missing rejected, preference/state bypass,
  backward compatibility, mixed batch filtering

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lishuceo
lishuceo force-pushed the fix/memory-entity-binding branch from a7fb33b to 420be4e Compare April 2, 2026 03:29
@claude

claude Bot commented Apr 2, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 1m 4s —— View job


PR Review: Entity-Orphan Guard + Log Message Correction

  • Check for previous review comments — none found
  • Read changed files in full context
  • Analyze logic correctness and security
  • Post review summary

Approved — Clean, well-scoped fix with good test coverage.

Assessment

This PR adds a solid defense-in-depth mechanism against hallucinated entity extraction. The approach is sound: a hard substring check on declared entities, applied only to entity-bearing memory types (fact/relation/decision), with backward compatibility for LLMs that omit the entities field.

What looks good:

  • Clear separation of prompt-level guidance (entity tracing rules) + code-level enforcement (filterUngroundedMemories)
  • Correct backward compatibility — memories without entities field pass through
  • Type-selective validation — preference/state correctly skip the guard
  • Thorough test coverage (9 guard tests + 2 parsing tests covering all edge cases)
  • Log message fix in injector is accurate

Minor observations (not blocking):

  1. Entity matching is case-sensitive (conversation.includes(entity)) — this is fine for Chinese names (the primary use case), but if English entity names ever appear with different casing (e.g., "OpenAI" vs "openai"), they'd be rejected. Low risk given the use case but worth noting for the future.

  2. The guard relies on LLM cooperation — if the extraction model omits entities, the memory passes through unchecked. This is the correct trade-off for backward compatibility, but it means the guard's effectiveness depends on prompt adherence. The prompt changes are clear enough that this should work well in practice.

  3. No false-positive risk from substring matchingincludes() could technically match partial strings (e.g., entity "张三" found inside "张三丰"), but this would result in allowing a memory rather than rejecting it, so the error mode is safe (false negatives rather than false positives).

Overall this is a well-designed fix that addresses a real cross-contamination bug in the memory pipeline. The "prefer false negatives over false positives" philosophy is exactly right for a memory system.


@claude

claude Bot commented Apr 8, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 1m 1s —— View job


PR Review: Entity-Orphan Guard + Log Message Correction

  • Check for previous inline review comments — none found
  • Read changed files in full context
  • Analyze logic correctness and security
  • Post review summary

Approved — Clean, well-scoped fix with good test coverage.

Assessment

This PR adds a solid defense-in-depth mechanism against hallucinated entity extraction. The approach is sound: a hard substring check on declared entities, applied only to entity-bearing memory types (fact/relation/decision), with backward compatibility for LLMs that omit the entities field.

What looks good:

  • Clear separation of prompt-level guidance (entity tracing rules) + code-level enforcement (filterUngroundedMemories)
  • Correct backward compatibility — memories without entities field pass through
  • Type-selective validation — preference/state correctly skip the guard
  • Thorough test coverage (9 guard tests + 2 parsing tests covering all edge cases)
  • Log message fix in injector is accurate

Minor observations (not blocking):

  1. Entity matching is case-sensitive (conversation.includes(entity)) — this is fine for Chinese names (the primary use case), but if English entity names ever appear with different casing (e.g., "OpenAI" vs "openai"), they'd be rejected. Low risk given the use case but worth noting for the future.

  2. The guard relies on LLM cooperation — if the extraction model omits entities, the memory passes through unchecked. This is the correct trade-off for backward compatibility, but it means the guard's effectiveness depends on prompt adherence. The prompt changes are clear enough that this should work well in practice.

  3. No false-positive risk from substring matchingincludes() could technically match partial strings (e.g., entity "张三" found inside "张三丰"), but this would result in allowing a memory rather than rejecting it, so the error mode is safe (false negatives rather than false positives).

Overall this is a well-designed fix that addresses a real cross-contamination bug in the memory pipeline. The "prefer false negatives over false positives" philosophy is exactly right for a memory system.


@lishuceo
lishuceo merged commit 4cec466 into main Apr 8, 2026
4 checks passed
@lishuceo
lishuceo deleted the fix/memory-entity-binding branch April 8, 2026 04:00
lishuceo added a commit that referenced this pull request Apr 8, 2026
…199)

* fix: correct misleading log message for memory injection location

Memories are injected into user prompt prefix (executor.ts:481),
not system prompt. The stale log message caused confusion during
debugging.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: entity-orphan guard to prevent ungrounded memory extraction

When the extraction LLM produces a fact/relation/decision that
references entities (person names, project names) not present in the
source conversation, the memory is now rejected instead of stored.

Two-layer defense:
1. Prompt rule: instructs extraction LLM to skip facts whose entities
   lack explicit textual evidence in the conversation
2. Code validation: filterUngroundedMemories() verifies every declared
   entity appears in the conversation text via substring match

This prevents the propagation chain where hallucinated facts about
one candidate get stored and later injected into analyses of
different candidates.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test: add entity-orphan guard and entities field parsing tests

9 new test cases:
- parseExtractionResponse: entities field parsing and missing field handling
- filterUngroundedMemories: grounded entities kept, orphaned entities
  rejected, partial missing rejected, preference/state bypass,
  backward compatibility, mixed batch filtering

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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