fix: wrap extraction transcript turns in speaker tags so assistant context cannot be misattributed#961
Closed
gorkem2020 wants to merge 13 commits into
Closed
Conversation
captureAssistant: false filters assistant turns out of extraction input entirely, protecting against misattribution but starving the extractor of disambiguating context (a user reply like "yes exactly, that one" extracts poorly with no idea what "that" refers to). Add a middle mode, captureAssistant: "context": assistant turns are routed into the extraction prompt as clearly marked, non-extractable context, while remaining fully capture-ineligible. Chosen as a union literal on the existing boolean field (captureAssistant?: boolean | "context") rather than a sibling flag, since it composes naturally with the current true/false normalization (cfg.captureAssistant === "context" ? "context" : cfg.captureAssistant === true) and needed no new config surface. true and false keep byte-identical semantics, verified directly against parsePluginConfig. Assistant-context texts are collected into their own array during the auto-capture message loop (never merged into eligibleTexts), rolled across turns in a new autoCaptureRecentAssistantTexts map (mirroring the existing autoCaptureRecentTexts pattern), and cleared on successful extraction. They never touch autoCaptureSeenTextCount, so eligibility counting and the auto-capture watermark are provably unaffected — a hook-level regression test proves cumulative counting stays 1-then-2 across two user+assistant turns, not 2-then-4. SmartExtractor.extractAndPersist() gains an assistantContextTexts option threaded down to the prompt assembly, which appends the marked lines under an "Assistant Context" heading. The extraction prompt's criteria gained a matching rule: candidates may only assert facts sourced from user-authored lines, never from an assistant-marked line alone. This is prompt-level guidance only (no deterministic storage-side gate exists for this, unlike the grounding work) — coverage here is structural (the instruction text is present) plus the prompt-assembly behavior (marked lines appear only when assistantContextTexts is non-empty). Note for integration: fix/autocapture-watermark-reset (67ef600, not yet merged) touches the same post-success reset block this change touches (the line right after autoCaptureSeenTextCount.set(sessionKey, ...)). Resolving that merge needs to keep both: the flow-aware reset value and autoCaptureRecentAssistantTexts.delete(sessionKey).
…ario runAssistantContextModeScenario's turn 2 intentionally sends only that turn's delta messages, not full accumulated history. On this branch's current counter model (issue CortexReach#417 Fix CortexReach#4/CortexReach#5), that is correct: the watermark only resets on a successful extraction and never rolls back on a skip, so per-turn deltas already accumulate correctly. This is documentation only, no fixture behavior changed. A prior assembly integration test surfaced that a *rolled-back* counter model (issue CortexReach#417 Fix CortexReach#9) requires this fixture to send full accumulated history instead, so the note pins down which fixture style belongs here and why, to prevent that failure mode from resurfacing silently if this branch is later combined with Fix CortexReach#9's rollback logic.
configSchema.properties.captureAssistant was still the pre-feature plain boolean, even though this branch's own captureAssistant: "context" mode (and its runtime normalization in index.ts) has existed since this feature landed. The oneOf fix had only ever been applied as a post-assembly commit on the old deploy branch, never ported back here. Pinned exact shape from the live fleet manifest. Red-first regression test added to plugin-manifest-regression.mjs (none existed for this key). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolves the index.ts conflict from issue CortexReach#417 Fix CortexReach#9's flow-aware watermark reset (already on master) landing alongside this branch's own autoCaptureRecentAssistantTexts.delete(sessionKey) cleanup -- keeps both, per the integration note left in the original captureAssistant commit message.
…onversation-turns transcript The extraction prompt's chat history was two disjoint blocks: user texts flattened under "## Recent Conversation", then (in captureAssistant: "context" mode) a separate "## Assistant Context (for disambiguation only)" block appended after, each assistant line individually labeled "Assistant (context only -- do not extract from these lines)". Replace both with one "## Recent conversation turns" section: a header, a single description sentence carrying the "extract from user turns only" rule, then a continuous oldest-first transcript of "User: text" / "Assistant: text" line-groups with no other headers or per-turn metadata. The user label is the configured extractor user name when set, falling back to "User". New pure functions in src/auto-capture-cleanup.ts: - formatConversationTranscript(turns, userLabel?) -- pure rendering. - buildConversationTurnsForExtraction(...) -- assembles the ordered turn sequence from this call's true message-loop order, consuming (never recomputing) the watermark/rolling-window decisions already made by the auto-capture hook: drops already-extracted leading user turns when newUserTexts is a narrower tail-slice of eligibleTexts, keeps every assistant-context turn from this call, prepends assistant context rolled over from a prior call, and falls back to flat user turns (still preceded by rolled-over context) when newUserTexts didn't come from a tail-slice of eligibleTexts at all (pending-ingress replay has no per-message role correlation available). Orthogonal to eligibility: it never touches autoCaptureSeenTextCount, minMessages, or any counting decision, only consumes their results for rendering. index.ts's auto-capture message loop now also collects role-tagged turns (both eligible and context-only messages, true chronological order) alongside the existing eligibleTexts/assistantContextTexts arrays, then narrows against cleanTexts (the POST noise-filter list, not the pre-filter texts) so the rendered transcript never re-includes content the embedding-noise filter already dropped from the actual extraction. SmartExtractor.extractCandidates prefers the caller's conversationTurns when given, falling back to one user turn from conversationText plus assistantContextTexts as trailing assistant turns for callers that don't have per-message role data. Fixed three tests asserting on the old two-block format (the label text, the old "## Recent Conversation" heading, and a hardcoded-heading prompt-capture filter in a third test's own LLM mock) to match; the eligibility-counting assertions in all three were untouched and still pass, confirming the watermark/minMessages behavior this task was scoped to leave alone is unaffected. New test/auto-capture-cleanup.test.mjs coverage (was previously unregistered) and this file are now both registered in package.json's test chain and scripts/ci-test-manifest.mjs. index.ts and scripts/ci-test-manifest.mjs carry this repo's known mixed CRLF/LF encoding; both were edited with a latin1 Buffer splice (never Edit) to avoid normalizing the files, per this fork's CLAUDE.md -- the first ci-test-manifest.mjs attempt via Edit did trigger the documented trap (55 lines flagged instead of 1) and was discarded and redone via the splice script.
A live-incident investigation initially suspected captureAssistant:
"context" of starving the auto-capture watermark gate by halving the
eligible-message count after a restart. That mechanism was falsified:
captureAssistantEligible = (value === true) is false for both false and
"context", so isEligibleRole reduces to role === "user" either way --
"context" mode only adds assistant-role text to a separate
assistantContextTexts array for prompt context, which never touches
eligibleTexts or the cumulative watermark count. The actual restart
-survivability defect was unrelated to captureAssistant (see the
fix/watermark-restart-survival branch).
Encode the four-way harness that proved this (captureAssistant {false,
"context"} x payload shape {full-history, delta-only}, each run across a
simulated restart) as a permanent test, so a future change can't silently
reintroduce a counting/watermark difference between the two modes without
a test noticing.
Test-only: no production code change, since the invariant already holds.
Validated the test is meaningful by temporarily reintroducing the
originally-suspected bug (captureAssistantEligible also true for
"context") locally, confirming both trace-symmetry assertions fail with a
clear diff, then reverting -- this file's diff contains only the new test
plus its two registrations.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Messages extractMinMessages now bounds the extraction transcript in user<->assistant PAIRS: the window keeps the newest N user turns with their assistant replies interleaved in true order (or all of this call's unextracted user turns when there are more), and captureAssistant context rides the same window instead of an independent 6-slot assistant carry. Consumed pairs drop together, removing the asymmetry where assistant history was effectively unbounded while user history was cut to the watermark tail. Also hardens the extraction prompt's assistant-lines rule with a live-caught counterexample (the assistant greeting the user by name is not the user introducing themselves). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tant captureAssistant true makes assistant-authored lines eligible grounding sources (concrete durable uncorrected facts only, user line wins on a tie); the default and 'context' modes keep assistant lines as disambiguation context that can never ground a candidate on its own. Previously the config reached message eligibility but the extraction prompt never branched.
…ves turns A below-threshold deferral keeps content alive on two independent paths: the rolling pair buffer, and the watermark rollback (or pending-ingress re-queue) whose next slice re-includes the same turns. When the deferred turn finally fires, window assembly concatenated both copies and the extraction transcript carried the same exchange twice, back to back (observed live: window inflation plus mild extraction bias toward the duplicated line). Collapse duplicates at assembly time, by user text at pair granularity: a pair-shaped copy beats a flat re-queued copy, copies of an identical exchange collapse to the latest, and a repeated user text whose replies differ is a real conversation and is kept whole. The deferral itself lives in the regex-fallback gating change (CortexReach#934); with both applied the window stays duplicate-free on every path.
…ant misattribution With User:/Assistant: line prefixes only the first line of a message carries a speaker marker, so a multi-paragraph assistant reply sheds its marker after the first paragraph and the extractor attributes assistant-authored plans and preferences to the user. Wrap every message wholly in <user_message>/ <assistant_message> blocks, teach the format up front, rewrite the assistant-context rule in tag vocabulary (context only, disambiguation use, never attributable to the user; the captureAssistant=true eligible variant keeps its semantics), and snap extractMaxChars truncation to a tag boundary so a sliced transcript never leads with headless text.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
With the "User:"/"Assistant:" line-prefix transcript, only the first line of a message carries a speaker marker. A multi-paragraph assistant reply sheds its marker after the first paragraph, so every later paragraph reads as unowned text sitting between two user turns. In live use the extraction model attributed assistant-authored plans and preferences to the user and stored them as user memories (for example, an assistant's own "what clicks for me now" mental-model summary stored as a user preference), even with the context-only rule present in the prompt.
Change
formatConversationTranscriptnow wraps every message wholly in<user_message>...</user_message>/<assistant_message>...</assistant_message>blocks, so every line has an unambiguous owner regardless of paragraph count.buildExtractionPromptteaches the tag format up front (new "Transcript format" section) and keeps a one-line reminder on the conversation header. The assistant-context rule is rewritten in tag vocabulary: context ONLY, usable solely to resolve an ambiguous or obscure user message, never a grounding source, and assistant statements are never attributable to the user. ThecaptureAssistant: trueeligible variant keeps its existing semantics in the same vocabulary.trimTranscriptToTagBoundarysnaps theextractMaxCharstruncation to the next opening tag, so the prompt never leads with a headless half message (the old tail slice could cut mid-message and strand text with no speaker).Tests
test/extraction-transcript-speaker-tags.test.mjs(9 cases): whole-message wrapping, multi-paragraph containment, chronological ordering, tag-boundary truncation with raw-tail fallback, format-teaching placement, context-only pinning, and eligible-variant preservation.test/assistant-context-capture.test.mjsassertions updated to the tagged format (13/13 green).Stacked on #939 (captureAssistant context mode); the diff includes that branch until it merges.