Sanitize and distill inbound bodies before docs search; stop double-processing Discord forum posts - #144
Sanitize and distill inbound bodies before docs search; stop double-processing Discord forum posts#144jpr5 wants to merge 5 commits into
Conversation
The raw inbound message body was forwarded verbatim as the Pathfinder
search-docs query — up to 8000 characters of Discord mention tokens,
custom emoji, pasted channel sidebars, and GitHub issue-template
boilerplate went straight to the embedder. The only sanitizer in the
codebase ran on the OUTBOUND reply.
Two stages, both shared by every platform and by the dashboard Q&A route
(apps/web/src/app/api/qa/route.ts), since all of them funnel through
AIPipeline.generateSupportResponse:
1. sanitizePlatformMarkup (shared/text.ts) strips Discord mentions,
custom emoji, timestamps, Slack link/mention syntax, HTML comments,
issue-template checklists and boilerplate headings, and pasted
sidebar rows — deterministically, leaving prose and fenced code
intact.
2. SearchQueryBuilder (ai/query.ts) distills the sanitized body into a
focused query with Haiku, mirroring TicketClassifier: short bodies
short-circuit with no LLM call, and a failed distillation degrades
to a heuristic that prefers the sentences carrying the question
mark rather than dropping the search.
Retrieval gets the distilled query; generation and confidence scoring
get the sanitized body, because distillation is lossy on purpose and the
answer still needs the reporter's full context and code.
Every relayed query landed in Pathfinder's query_log EXACTLY twice, 0.15-0.3s apart, with two different session ids. Not the retry path — that re-issues after a failure, not simultaneously. Discord dispatches BOTH ThreadCreate and MessageCreate for a new forum post. handleThreadCreate ingests the starter message and enqueues an AI_RESPONSE job; handleMessageCreate then finds the ticket that was just created, treats the same message as a reply, and enqueues a SECOND AI_RESPONSE job for the same ticket. Two jobs, two workers, two retrievals, two Claude generations, for one question. The shadow-mode path duplicates identically (handleShadowThreadCreate then handleShadowMessage). The queue's own claim is fine — it uses FOR UPDATE SKIP LOCKED — so the duplication is at enqueue, not at claim. A thread's starter message shares the thread's ID, so skip it in handleMessageCreate. Genuine replies, which carry their own message ID, are unaffected.
Two ways a thread reached the AI pipeline when it should not have: - An empty MONITORED_CHANNEL_IDS meant "monitor every channel", so a missing env var silently opted the whole guild in. It now fails closed, with a one-time warning. - There was no question detection, so announcements and release-note threads spent a full retrieval + generation cycle. isSupportRequest (shared/text.ts) accepts anything with a question mark, help phrasing, an error signature, or a direct @-mention of the bot, and considers the thread title as well as the body — forum posts often put the question in the title. Deliberately permissive: a missed announcement costs nothing, a missed support request costs a customer.
Fallout from merging main. This test survived the textual merge and then failed, which is the useful kind of conflict: `message-create.test.ts` had no textual conflict at all, so nothing flagged it. It asserted that a genuine reply enqueues AI_RESPONSE. Since #172 and #191, `InboundHandler` never enqueues for a reply on any platform — Outpost answers once per ticket, on the opening message, and a human owns the thread after that. The assertion was written before that rule existed. What the test is actually here to prove is that the `message.id === threadId` gate distinguishes the starter message from a reply, so it now asserts the reply still gets its Message record, plus an explicit check that nothing was enqueued. That pins the one-answer rule on this path too. The gate stays load-bearing: neutering it fails the two starter-message tests.
CPK-7931 4. PR #144 — rebase + land query hygiene (conflicting; feeds answer quality)
CopilotKit/outpost#144 — Filed under response quality because it is the retrieval half of answer quality: it touches Two things to settle before rebasingIt overlaps #169. The inbound-dedupe half addresses the same duplicate-ticket class as #169 (redelivered events create duplicate tickets, so a conversation can get more than one AI answer). Decide whether #144 closes #169, partially addresses it, or should drop that half now that #191 arbitrates one-answer-per-ticket at the database. Doing this after #191 merges makes the answer much clearer. It shares a file with #191 and one with #150. Rebase after #191 lands, not before. Currently a draft, so it may also need a scope decision about whether it ships as one PR or splits the way #187 did. |
|
Merged Three textual conflicts, all pure unions — One semantic conflict with no textual conflict at all, which is the part worth knowing about. That test asserted a reply enqueues What the test is actually there to prove is that Your dedup gate is untouched and still load-bearing. Neutering Verified: Two things I did not do:
Ready for review. I haven't approved it — worth someone else confirming the test-intent change above reads right to them, since I changed an assertion rather than making it pass. |
Two bugs on the inbound path: raw message bodies were forwarded verbatim as docs-search queries, and one Discord forum post was ingested twice.
Both are invisible today because
SHADOW_MODEgates only the platform post-back (ai-response.ts:178) — retrieval and Claude generation run fully either way.Bug 1 — raw inbound bodies reached the embedder
Chain:
thread-create.ts→inbound.ts:180(truncate(content, 8000)) →ai-response.ts:87→pipeline.ts:72→pathfinder.ts:291. Nothing sanitized or narrowed the text; the only sanitizer in the codebase (formatter.ts:149) runs on the OUTBOUND reply.Three defects, all addressed:
sanitizePlatformMarkup(shared/src/text.ts) removes Discord mentions / custom emoji / timestamps, Slack link and mention syntax, HTML comments, issue-template checklists and boilerplate headings, and pasted channel-sidebar rows. Deterministic, and it leaves prose and fenced code alone. It lives insharedso every platform gets it.SearchQueryBuilder(ai/src/query.ts) distills the sanitized body into a focused query with Haiku, mirroringTicketClassifier's shape. Short bodies (≤200 chars) short-circuit with no LLM call; a failed distillation degrades to a heuristic that prefers the sentences carrying the question mark rather than dropping the search.MONITORED_CHANNEL_IDSnow fails closed instead of monitoring every channel, andisSupportRequestgates ingestion on a question mark, help phrasing, an error signature, or a direct @-mention of the bot — considering the thread title as well as the body, since forum posts often put the question in the title.Retrieval gets the distilled query; generation and confidence scoring get the sanitized body, because distillation is lossy on purpose and the answer still needs the reporter's full context and code.
apps/web/src/app/api/qa/route.ts:62needed no change — it calls the sameAIPipeline.generateSupportResponse, so it inherits the fix.Red-green — captured at the real Pathfinder wire boundary
Driven through the real
AIPipeline+ realPathfinderClientagainst a local HTTP server speaking real MCP Streamable-HTTP (nothing touchesmcp.copilotkit.ai); Anthropic calls go throughaimock. Input is a 727-char Discord post containing<#1205139168783503400>,<@!284920412034990081>,<:copilotkit:1187213988392189962>, a pasted channel sidebar, and a GitHub issue template.RED — before the change, the entire body crosses the wire:
GREEN — same repro, unchanged, on the fixed code:
727 chars of markup → an 81-char query, and still 100 focused chars when the distiller LLM is down.
Bug 2 — root cause: two Discord events, one message
Not multiple replicas racing the queue, and not the retry path.
Events.ThreadCreateandEvents.MessageCreateboth fire for a new forum post, and both are wired up (apps/discord-bot/src/index.ts:25,27):handleThreadCreateingests the starter message and enqueuesAI_RESPONSE.handleMessageCreatethen fires for that same starter message,findTicketByThreadIdfinds the ticket that was just created, andInboundHandler.handleReplyenqueues a secondAI_RESPONSEfor the same ticket.Two jobs → two workers → two Pathfinder sessions → two identical
query_logrows ~0.2s apart from two egress IPs. The shadow path duplicates identically (handleShadowThreadCreatethenhandleShadowMessage).createJobhas no idempotency key, so nothing collapses them.The queue's claim is not at fault —
worker.tsusesUPDATE ... WHERE id IN (SELECT ... FOR UPDATE SKIP LOCKED), which is correct for concurrent replicas. The duplication is at enqueue.Fix: a thread's starter message shares the thread's own ID, so
handleMessageCreateskips it. Genuine replies carry their own message ID and are unaffected.Red-green — real event handlers, real outbound calls
One forum post driven through the unmodified
handleThreadCreate+handleMessageCreate, then the realPathfinderClientonce per enqueued job against the MCP stub.RED:
GREEN — same repro, unchanged:
Tests
35 new regression tests, following the repo's conventions (
aimockfor every LLM call, no hand-stubbed responses):shared/src/__tests__/text.test.ts— 14 tests over the sanitizer and the support-request gate.ai/src/query.test.ts— 14 tests over the distiller and its heuristic fallback, incl. "never returns the raw body" and "sends the sanitized body, not the raw one".ai/src/pipeline.test.ts— searches with the distilled query, generates from the sanitized body, counts distiller tokens.discord-bot— starter-message dedupe (normal + shadow), genuine replies still processed, fail-closed channels, announcement rejection, title-only questions, shadow-mode gating.Local gate, all green from the repo root: prettier (new files), eslint on every changed file,
turbo run typecheck10/10,turbo run test(825 in@copilotkit/outpost, up from 794; 64 indiscord-bot, up from 56; 501 web; all others unchanged),turbo run build10/10.Deliberately not changed
Job. It would need a Prisma migration against the production DB, and it is not the root cause — the queue's claim is already correct and the duplicate came from a duplicate event path. Worth doing as defence in depth, but as its own change.ai-response.ts:178still gates only the platform post-back, so retrieval and generation run in shadow mode. Flagging rather than touching it: it is why both bugs were invisible, but changing it is a separate decision.mainis not prettier-clean (several bot files and tests fail--checkat HEAD), so only the new files were formatted; the edited files' added lines are prettier-conformant and the untouched lines were left as-is to avoid diff noise.