diff --git a/src/lib/references.test.ts b/src/lib/references.test.ts index 851758d..70900c6 100644 --- a/src/lib/references.test.ts +++ b/src/lib/references.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from "vitest"; -import { formatReferences, rankReferences, MAX_REFERENCES } from "./references"; +import { + formatReferences, + rankReferences, + MAX_REFERENCES, + REFERENCES_FOOTER_MARKER, + stripReferencesFooter, +} from "./references"; describe("formatReferences", () => { it("returns empty string when no refs", () => { @@ -182,3 +188,31 @@ describe("rankReferences", () => { expect(ranked).toHaveLength(1); }); }); + +// ── #146: footer echo — format/strip round-trip ────────────────────── + +describe("stripReferencesFooter", () => { + const ref = { type: "doc" as const, label: "setup.md", url: "https://github.com/o/r/blob/main/docs/setup.md" }; + + it("round-trips: stripping a formatted footer restores the bare answer (drift guard)", () => { + const withFooter = "the answer body" + formatReferences([ref]); + expect(stripReferencesFooter(withFooter)).toBe("the answer body"); + }); + + it("formatReferences output contains the exported marker (the pair can't drift apart)", () => { + expect(formatReferences([ref])).toContain(REFERENCES_FOOTER_MARKER); + }); + + it("returns text unchanged when no footer is present", () => { + expect(stripReferencesFooter("plain answer\nwith lines")).toBe("plain answer\nwith lines"); + }); + + it("cuts from the marker to the END — trailing hint and any stragglers go too", () => { + const text = "answer\n\n" + REFERENCES_FOOTER_MARKER + "\n • 📄 \n_React with 👍 or 👎 to help me give better answers in the future._"; + expect(stripReferencesFooter(text)).toBe("answer"); + }); + + it("a footer-only string strips to empty", () => { + expect(stripReferencesFooter(formatReferences([ref]))).toBe(""); + }); +}); diff --git a/src/lib/references.ts b/src/lib/references.ts index a169447..f42b899 100644 --- a/src/lib/references.ts +++ b/src/lib/references.ts @@ -117,5 +117,20 @@ export function formatReferences(refs: Reference[]): string { // No "...and N more" — if refs are ranked correctly, the cap is the answer const hint = "\n_React with 👍 or 👎 to help me give better answers in the future._"; - return `\n\n───\n*References:*\n${lines.join("\n")}${hint}`; + return `\n\n${REFERENCES_FOOTER_MARKER}\n${lines.join("\n")}${hint}`; +} + +// ── Footer stripping (#146) ────────────────────────────────────────── +// Prior bot replies are replayed into conversation history; if the +// system footer rides along, the model learns to imitate it and emits +// its own references block mid-answer. The marker is shared between +// format and strip so the pair can never drift apart. + +export const REFERENCES_FOOTER_MARKER = "───\n*References:*"; + +/** Remove the system-appended footer (marker to end of text). */ +export function stripReferencesFooter(text: string): string { + const idx = text.indexOf(REFERENCES_FOOTER_MARKER); + if (idx === -1) return text; + return text.slice(0, idx).trimEnd(); } diff --git a/src/lib/thread-filter.test.ts b/src/lib/thread-filter.test.ts index 65a330f..7d22674 100644 --- a/src/lib/thread-filter.test.ts +++ b/src/lib/thread-filter.test.ts @@ -203,3 +203,86 @@ describe("extractTranscriptTail", () => { expect(tail).not.toContain("msg 13"); }); }); + +// ── #146: system footer must not echo through conversation history ── + +describe("buildConversationHistory — footer stripping (#146)", () => { + const BOT = "B001"; + const FOOTER = "\n\n───\n*References:*\n • 📄 \n_React with 👍 or 👎 to help me give better answers in the future._"; + + it("strips the system footer from assistant turns so the model can't imitate it", () => { + const history = buildConversationHistory( + [ + { user: "U1", text: "how does auth work?", bot_id: undefined }, + { user: BOT, text: "Auth uses JWT." + FOOTER, bot_id: "B001" }, + ], + BOT, + ); + expect(history).toHaveLength(2); + expect(history[1].content).toBe("Auth uses JWT."); + expect(history[1].content).not.toContain("References"); + expect(history[1].content).not.toContain("React with"); + }); + + it("does NOT strip footer-lookalike text from USER turns", () => { + const userText = "why does your ───\n*References:*\n block show twice?"; + const history = buildConversationHistory( + [{ user: "U1", text: userText, bot_id: undefined }], + BOT, + ); + expect(history[0].content).toContain("*References:*"); + }); + + it("skips an assistant message that is footer-only (no empty turns)", () => { + const history = buildConversationHistory( + [ + { user: "U1", text: "question", bot_id: undefined }, + { user: BOT, text: FOOTER.trimStart(), bot_id: "B001" }, + { user: "U1", text: "follow-up", bot_id: undefined }, + ], + BOT, + ); + // Footer-only bot turn vanishes; the two user turns merge. + expect(history).toHaveLength(1); + expect(history[0].role).toBe("user"); + expect(history[0].content).toContain("question"); + expect(history[0].content).toContain("follow-up"); + }); +}); + +describe("extractTranscriptTail — footer stripping (#146 review)", () => { + const BOT = "B001"; + const FOOTER = "\n\n───\n*References:*\n • 📄 \n_React with 👍 or 👎 to help me give better answers in the future._"; + + it("bot entries reach the classifier transcript footer-free", () => { + const tail = extractTranscriptTail( + [ + { user: "U1", text: "how does auth work?", bot_id: undefined }, + { user: BOT, text: "Auth uses JWT." + FOOTER, bot_id: "B001" }, + ], + BOT, + ); + expect(tail).toContain("bot: Auth uses JWT."); + expect(tail).not.toContain("References"); + expect(tail).not.toContain("React with"); + }); + + it("a footer-only bot message is skipped, not an empty entry", () => { + const tail = extractTranscriptTail( + [ + { user: "U1", text: "question", bot_id: undefined }, + { user: BOT, text: FOOTER.trimStart(), bot_id: "B001" }, + ], + BOT, + ); + expect(tail).toBe("user: question"); + }); + + it("user entries quoting footer-lookalike text are NOT stripped", () => { + const tail = extractTranscriptTail( + [{ user: "U1", text: "why is *References:* doubled?", bot_id: undefined }], + BOT, + ); + expect(tail).toContain("*References:*"); + }); +}); diff --git a/src/lib/thread-filter.ts b/src/lib/thread-filter.ts index 5b7b51c..68ea5f2 100644 --- a/src/lib/thread-filter.ts +++ b/src/lib/thread-filter.ts @@ -4,6 +4,8 @@ * proper multi-turn conversation history from Slack thread messages. */ +import { stripReferencesFooter } from "./references"; + const MENTION_RE = /<@([A-Z0-9]+)>/g; /** @@ -83,11 +85,15 @@ export function buildConversationHistory( // Build raw turns with cleaned text const rawTurns: MessageParam[] = []; for (const m of recent) { - const text = truncate(cleanText(m.text ?? ""), MAX_MESSAGE_LENGTH); - if (!text) continue; // Skip empty messages - const role: "user" | "assistant" = m.user === botUserId || m.bot_id ? "assistant" : "user"; + // Strip the system-appended references footer from prior bot replies + // before they re-enter the model's context — otherwise the model + // imitates the footer and emits its own copy mid-answer (#146). + const raw = role === "assistant" ? stripReferencesFooter(m.text ?? "") : m.text ?? ""; + const text = truncate(cleanText(raw), MAX_MESSAGE_LENGTH); + if (!text) continue; // Skip empty messages + rawTurns.push({ role, content: text }); } @@ -143,10 +149,14 @@ export function extractTranscriptTail( const entries: string[] = []; for (let i = messages.length - 1; i >= 0 && entries.length < TRANSCRIPT_TAIL_MAX; i--) { const m = messages[i]; - const text = truncate(cleanText(m.text ?? ""), TRANSCRIPT_ENTRY_MAX_CHARS); + const speaker = m.user === botUserId || m.bot_id ? "bot" : "user"; + // Same rule as buildConversationHistory (#146): bot replies drop the + // system footer so reference bullets can't eat the classifier's + // per-entry budget or skew the gate. + const raw = speaker === "bot" ? stripReferencesFooter(m.text ?? "") : m.text ?? ""; + const text = truncate(cleanText(raw), TRANSCRIPT_ENTRY_MAX_CHARS); if (!text) continue; // Skip empty messages - const speaker = m.user === botUserId || m.bot_id ? "bot" : "user"; entries.push(`${speaker}: ${text}`); } return entries.reverse().join("\n");