fix: filter internal prompts from chat capture observations#21
fix: filter internal prompts from chat capture observations#21wjiuxing wants to merge 2 commits into
Conversation
The chat-capture hook unconditionally stored all user messages as observations, including open-mem internal prompts (observation extraction, summarization, conflict evaluation, etc.) that leak into the chat stream. These prompts accumulated as garbage observations (49% of total in one project database) and were injected back into context via true_memory_context, wasting tokens and re-injecting stale directives. Fix: Add INTERNAL_PROMPT_PATTERNS filter to chat-capture.ts that rejects messages starting with known internal prompt templates before observation creation. Closes clopca#20
|
Warning Review limit reached
More reviews will be available in 55 minutes and 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds filtering to the chat capture hook to prevent internal system prompts from being persisted as observations. A set of regex patterns identifies internal prompts for observation extraction, session summarization, conflict evaluation, and related tasks. The ChangesInternal Prompt Filtering for Chat Capture
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR fixes a data-quality problem in the
Confidence Score: 4/5Safe to merge; the filter correctly targets all 5 internal prompt templates and cannot false-positive on normal user text due to start-anchored patterns. The core logic in chat-capture.ts is correct and well-placed. The only gap is that two of the five new regex patterns (entity_extraction and rerank_request) have no corresponding filter tests, so a future regression there would go undetected. tests/hooks/chat-capture.test.ts — needs filter tests for the entity_extraction and rerank_request patterns. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[chat.message hook fires] --> B[extractTextFromParts]
B --> C[stripPrivateBlocks + redactSensitive]
C --> D{length >= MIN_MESSAGE_LENGTH?}
D -- No --> E[return false]
D -- Yes --> F{matches INTERNAL_PROMPT_PATTERNS?}
F -- Yes --> G[return false filtered internal prompt]
F -- No --> H[sessions.getOrCreate]
H --> I[observations.create]
I --> J[return true]
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
tests/hooks/chat-capture.test.ts:311-327
**Missing test coverage for `entity_extraction` and `rerank_request` patterns**
The PR adds 5 patterns to `INTERNAL_PROMPT_PATTERNS` but only 3 of the 5 are covered by the new "filters out…" tests. The `<entity_extraction>` and `<rerank_request>` patterns have no corresponding filter tests. If either pattern were accidentally removed or broken in a future refactor, nothing would catch the regression.
Reviews (1): Last reviewed commit: "fix: filter internal prompts from chat c..." | Re-trigger Greptile |
| test("does not filter normal user messages that happen to contain XML-like tags", async () => { | ||
| const observations = makeMockObservations(); | ||
| const sessions = makeMockSessions(); | ||
| const hook = createChatCaptureHook(observations as never, sessions as never, "/tmp/proj"); | ||
|
|
||
| await hook( | ||
| { sessionID: "s1" }, | ||
| { | ||
| message: {}, | ||
| parts: [ | ||
| "Please help me debug this HTML issue with <div> tags and also fix the layout", | ||
| ], | ||
| }, | ||
| ); | ||
|
|
||
| expect(observations.calls.find((c) => c.method === "create")).toBeDefined(); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for
entity_extraction and rerank_request patterns
The PR adds 5 patterns to INTERNAL_PROMPT_PATTERNS but only 3 of the 5 are covered by the new "filters out…" tests. The <entity_extraction> and <rerank_request> patterns have no corresponding filter tests. If either pattern were accidentally removed or broken in a future refactor, nothing would catch the regression.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/hooks/chat-capture.test.ts
Line: 311-327
Comment:
**Missing test coverage for `entity_extraction` and `rerank_request` patterns**
The PR adds 5 patterns to `INTERNAL_PROMPT_PATTERNS` but only 3 of the 5 are covered by the new "filters out…" tests. The `<entity_extraction>` and `<rerank_request>` patterns have no corresponding filter tests. If either pattern were accidentally removed or broken in a future refactor, nothing would catch the regression.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/hooks/chat-capture.test.ts (1)
235-327: ⚡ Quick winAdd coverage for the two remaining internal prompt patterns.
Current new tests cover extraction/summarization/conflict, but
INTERNAL_PROMPT_PATTERNSalso filters<entity_extraction>and<rerank_request>. Please add one negative test per pattern (assertobservations.createis not called) to lock down all branches introduced by this fix.🤖 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 `@tests/hooks/chat-capture.test.ts` around lines 235 - 327, Add two negative tests in tests/hooks/chat-capture.test.ts mirroring the existing pattern tests: one that passes a part containing an <entity_extraction>... payload and one that passes a <rerank_request>... payload, both using makeMockObservations(), makeMockSessions(), and createChatCaptureHook(...) and invoking hook({ sessionID: "s1" }, { message: {}, parts: [ "<entity_extraction>...</entity_extraction>" ] }) (and similarly for <rerank_request>); assert that observations.calls has length 0 or that observations.create is not called to ensure INTERNAL_PROMPT_PATTERNS filtering covers these two cases. Ensure test names clearly state they "filter out internal entity extraction prompts" and "filter out internal rerank request prompts" so they follow the existing tests' style.
🤖 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 `@tests/hooks/chat-capture.test.ts`:
- Around line 235-327: Add two negative tests in
tests/hooks/chat-capture.test.ts mirroring the existing pattern tests: one that
passes a part containing an <entity_extraction>... payload and one that passes a
<rerank_request>... payload, both using makeMockObservations(),
makeMockSessions(), and createChatCaptureHook(...) and invoking hook({
sessionID: "s1" }, { message: {}, parts: [
"<entity_extraction>...</entity_extraction>" ] }) (and similarly for
<rerank_request>); assert that observations.calls has length 0 or that
observations.create is not called to ensure INTERNAL_PROMPT_PATTERNS filtering
covers these two cases. Ensure test names clearly state they "filter out
internal entity extraction prompts" and "filter out internal rerank request
prompts" so they follow the existing tests' style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 598ed226-0056-4e49-b0c8-2ba1f8425e7f
📒 Files selected for processing (2)
src/hooks/chat-capture.tstests/hooks/chat-capture.test.ts
Addresses review feedback: add test cases for the two INTERNAL_PROMPT_PATTERNS that were missing dedicated filter tests (entity_extraction and rerank_request).
|
Re: #21 (comment) Good catch! Added in commit 3b2c790 — both |
Problem
Closes #20
The
chat-capturehook (chat.message) unconditionally stored all user messages as observations, including open-mem internal prompts that leak into the chat stream.In one project database, 2330 out of 4710 observations (49%) were garbage
"User request: <task>\nAnalyze the following tool output..."entries — internal extraction prompts, not user intent.These garbage observations were injected back into context via
true_memory_context, causing:[search-mode],[analyze-mode]directives re-injected into new sessionsRoot Cause
open-mem injects internal prompts (observation extraction, summarization, conflict evaluation, entity extraction, reranking) into the chat stream. The
chat-capturehook sees these as user messages and stores them as observations withtitle="User request: <task>...".Fix
Add
INTERNAL_PROMPT_PATTERNS— a list of regex patterns matching all 5 internal prompt templates — tochat-capture.ts. Before creating an observation, the text is tested against these patterns and rejected if it matches.Changes:
src/hooks/chat-capture.ts: +18 lines (pattern definitions + filter check)tests/hooks/chat-capture.test.ts: +97 lines (4 new test cases)Testing
Workaround for existing databases
For users with existing garbage observations, run:
Summary by CodeRabbit
Bug Fixes
Tests