feat(ai): retrieve over CopilotKit's source, not only its docs - #242
Conversation
jerelvelarde
left a comment
There was a problem hiding this comment.
Reviewed deeply, then put every blocking finding through an adversarial round that tried to refute it. One survived. The retrieval plumbing itself is solid — the allSettled coercion, the cap and the interleave are all real, backed by tests that exercise the real AIPipeline against a stubbed client rather than asserting against their own mock, and every mutation the body names does kill the test it names.
Blocker — the anchoring fix is asymmetric, and opens the mirror-image hole in docs blocks
pathfinder.ts:307-317. /^\s*PATH:\s*(.+)$/im is matched against the entire block, body included. ^\s* allows code indentation and i allows lowercase, so a documentation snippet whose CONTENT contains a line-initial path: yields a truthy path. Then title = path ?? TITLE makes the code fragment beat the real header, and source = path ? blobUrl(repository, path) : SOURCE throws the docs URL away — repository is undefined on a docs block, so blobUrl returns undefined and there is no URL at all.
Run this through the real functions:
SNIPPET 1
TITLE: Self-hosting the CopilotKit Runtime
SOURCE: https://docs.copilotkit.ai/guides/self-hosting
CONTENT:
```ts
const handler = copilotRuntimeNextJSAppRouter({
path: "/api/copilotkit",
});
→ `title = '"/api/copilotkit",'`, `sourceUrl = undefined`.
This is the same defect you fixed for code blocks — unanchored headers letting a body line win — reintroduced on the other side. And it lands where it hurts most given the rest of this PR: a docs page that loses its URL cannot be cited, and #241's `source-link-or-handoff` rule then collapses that answer into a two-sentence handoff. `copilotRuntimeNextJSAppRouter({ path: ... })` is not an exotic snippet; it is in the self-hosting docs.
The discriminator is already stated in your adjacent comment — a code block has no `TITLE`:
```ts
const titleHeader = block.match(/^\s*TITLE:\s*(.+)$/im)?.[1]?.trim();
const isCode = !titleHeader && !!path;
title = titleHeader ?? path ?? 'Documentation';
source = isCode ? blobUrl(repository, path) : sourceHeader;
Better still, split each block at the first CONTENT: and only ever match headers in the part above it.
Worth settling before merge, your call on each
blobUrl fails silently. pathfinder.ts:18-23 returns undefined with no log when REPOSITORY is missing or is not an https://github.com/ URL. If the server ever emits a bare slug (CopilotKit/CopilotKit), an SSH URL, or drops the header for one index, every code hit reaches the prompt with nothing to cite and leaves no trace. That is the same silent-degradation shape this PR exists to remove, one layer up.
The prompt cannot tell code from docs. generator.ts:236 — GROUNDING_RULES now asserts "Code entries are shown with their file path", but buildSystemPrompt renders both kinds as an identical [Source N: <title> (relevance: X)]. A docs page titled api-reference/components/CopilotKit and a code hit titled packages/react-core/src/index.ts are indistinguishable. So "where code and docs disagree, the code is what ships" is only resolvable by guessing, which is exactly the instruction you most need to be reliable now that the "you have NOT read the source" line is gone.
The cap over-fetches then discards. pipeline.ts:188 — with both tools at defaultLimit: 8 and a .slice(0, 8) on the interleave, a purely docs-answerable question that used to get 8 docs snippets now gets 4, with the other 4 going to code hits that only cleared min_score: 0.3. Every ticket pays for 16 and throws away 8. Lowering each tool's limit instead costs nothing and keeps docs recall intact on the majority path.
search()'s doc comment overclaims. It says keeping the tool name private removes "the silent no-retrieval failure mode this whole change exists to remove". A server-side rename of search-code produces exactly that: JSON-RPC error → one console.error → [] → indistinguishable from "no code matched", for as long as nobody reads the logs. SearchTool is a compile-time union; it cannot know what the server exposes. The tools/list probe you already do would make this checkable at startup.
Not a finding, but the one I'd want a follow-up on
You note generateStreamingResponse still calls searchDocs only. Given the groundedness gate is now the only mechanism that can withhold a response, and assessGroundedness excludes path-shaped tokens and blanks URLs before it looks, this capability ships with no fail-closed backstop on the streaming path. Worth a linked issue rather than a line in the body.
The prompt rewrites are the right call and the reasoning for both is sound — CONFIDENCE_SYSTEM_PROMPT marking down exactly the answers this PR makes possible was a genuinely subtle catch, and exporting it so the two prompts can be pinned against each other by test is the right shape.
CPK-8077 Phase 2 — Turn on code search and AG-UI search (doc step 4)
Doc step 4. Biggest single quality win, and smaller than it sounds. Root cause #1 in the doc: for any question whose answer lives in the source — most of the hard ones — the agent is guessing from general React knowledge. The capability is already live
It also never calls So this is wiring, not construction. Ships with the prompt change, not after it
The doc's decisions that land with this:
Fixes, measurablyAppendix case A directly. Also the doc's "zero escalations where the answer turned out to be sitting in the code all along" — which Phase 1's harness can score, since case A's answer is one code search away. RelatedDoesn't remove the need for #220 (the Pathfinder corpus audit — empty FAQ, thin troubleshooting, no pricing docs, untouched since 2026-08-17). Code search compensates for a thin docs corpus; it doesn't fix it. That's content work and can run in parallel. |
Addresses the review on #242. The anchoring fix in the previous commit was asymmetric. `/^\s*PATH:/im` was matched against the whole block, body included, and `^\s*` allows code indentation while `i` allows lowercase — so a DOCUMENTATION snippet whose content quotes source code got a truthy `path`, was treated as a code block, and lost its docs URL. Verified against the real function with a snippet from the self-hosting guide: TITLE: Self-hosting the CopilotKit Runtime SOURCE: https://docs.copilotkit.ai/guides/self-hosting CONTENT: const handler = copilotRuntimeNextJSAppRouter({ path: "/api/copilotkit", }); before: title '"/api/copilotkit",' sourceUrl undefined after: title 'Self-hosting the CopilotKit Runtime' sourceUrl 'https://docs.copilotkit.ai/guides/self-hosting' Same defect as the one fixed for code blocks, reintroduced on the docs side — a body line winning over a real header. A docs page with no URL cannot be cited, and #241's source-link-or-handoff rule then collapses that answer into a two-sentence handoff, so the cost lands on the reporter. Headers are now read only from the region above `CONTENT:`, which removes the class rather than the two spellings that happened to be noticed. A block is code when it has a PATH and no TITLE, derived from the headers rather than from PATH alone. Also from the same review: - `SearchResult.kind` carries docs-vs-code through to the prompt. GROUNDING_RULES asserts "code entries are shown with their file path" and that the code wins a conflict with the docs, but buildSystemPrompt rendered both as an identical `[Source N: title]` — so the instruction most in need of being reliable, now that "you have NOT read the source" is gone, was resolvable only by guessing. Sources are labelled SOURCE CODE / DOCS, and an unlabelled source stays unlabelled rather than being asserted as either. - Each tool now gets half the retrieval budget instead of a full `defaultLimit` each. Over-fetching paid for 16 snippets to keep 8, and cost docs recall on the majority path: a purely docs-answerable question would have got 4 docs hits rather than 8, the rest going to code that merely cleared min_score. - `blobUrl` logs instead of returning undefined silently. A bare slug, an SSH remote, or a missing REPOSITORY on one index would leave EVERY code hit with nothing to cite and no trace — the silent-degradation shape this change exists to remove, one layer up. - `search()`'s doc comment claimed private tool names remove the silent no-retrieval mode. They do not: `SearchTool` is a compile-time union and cannot know what the server exposes, so a server-side rename still yields one console.error and []. Comment corrected and pointed at #244. Verification: ai package 293 -> 303, full repo turbo run test 10/10, typecheck clean. On mutations: reverting the header region alone, or the isCode rule alone, does NOT fail a test — three layers independently prevent this (title reads TITLE before PATH, isCode requires no TITLE, headers come only from above CONTENT:). Reverting the two structural ones together fails two tests. Dropping the prompt label, and restoring the full per-tool limit, each fail the tests that name them. Refs CPK-8077
8dc951e to
fa12023
Compare
Phase 3 of the response-quality work (CPK-8078), first slice. The doc's flowchart hangs on one arrow: "if the draft breaks a rule, it doesn't get cleaned up and posted — it collapses into the two-sentence version." This is that arrow, and it runs the same rules the harness scores with, from the same module, so the thing measured and the thing enforced cannot drift. ## The two prerequisites Jerel named on #241, which had to come first Both would have made the linter withhold correct answers, which is the same failure direction as the groundedness gate suppressing one — the bug #234 was filed for. 1. The citation rule tested whether a link LOOKED like ours, so the URL was both the citation and the laundering: a reply could write its invented hook name inside a docs.copilotkit.ai link — /hooks/useCopilotFabricated — and satisfy the rule with a page that does not exist. The identifier rule cannot catch it either, because assessGroundedness blanks URLs before it looks. The check now matches against the URLs actually retrieved. 2. Pathfinder's plain-text fallback (textSearch) sets sourceUrl: undefined on every result, so a CORRECT answer built from it has nothing it could cite. Under a flat requirement that answer fails forever and, once these rules gate publishing, collapses into a handoff every time the fallback is in play. Closing 2 needed a third state, so RuleResult gains `applicable`: "did not cite" and "had nothing citable" are different facts. A not-applicable rule is never a failure, never withholds a draft, and is not counted in a pass rate — which also fixes a rule that was inapplicable everywhere reading as a clean sweep, since `passed === total` is trivially true at 0/0. formatReport prints `n/a` for those rather than `ok`. ## Report mode is the default `lintDraft` computes the verdict and changes nothing unless asked to enforce. Enforcing means a misfiring rule withholds a correct answer from a real person, so the sequence is: run in report mode, read what it would have collapsed against real traffic, then enforce once the false-positive rate is known rather than assumed. `wouldCollapse` carries the counterfactual so report mode is worth running. It returns a verdict, never replacement copy. The caller substitutes its own, and in the pipeline that is the existing SUPPRESSED_RESPONSE_TEXT — that copy already promises a human follow-up, and #231 records what happens when two layers each add their own promise. ## Not wired into the pipeline here, deliberately The wiring belongs in pipeline.ts, which #242 is already editing on another branch. Landing both would collide over the same function for no benefit, since a report-mode linter changes nothing until someone reads its output. Wiring follows once #242 is in. Verification: ai package 318 -> 334, full repo turbo run test 10/10, typecheck clean. Four mutations, each killing the tests that name it: the laundering-permissive citation check, an always-applicable citation rule, report mode withholding, and enforce counting inapplicable rules as failures. Two tests were updated rather than patched around: both cited arbitrary docs-shaped URLs absent from their fixture's sources, which is precisely the laundering the new check closes. Stacked on #241 (feat/response-quality-eval-harness) because it consumes that rule module; rebases onto whatever that review lands. Refs CPK-8078
|
Thanks Jerel — the blocker was real, and it was my own fix reintroducing the same defect on the other side. Fixed in The asymmetryReproduced it against the real function with your snippet from the self-hosting guide before changing anything: Headers are now read only from the region above The other four, all taken
On mutations, stated plainlyReverting the header region alone, or the
Your |
Phase 2 of the response-quality work (CPK-8077). Root cause 1 in the Agent's Output Doc: for any question whose answer lives in the source — most of the hard ones — the agent had only the docs and was otherwise guessing from general React knowledge. A reporter asked whether Deep Agents supports subagents; the docs do not mention it, so the agent said it had no timeline and sent them to GitHub to ask. Subagents work today and one code search returns the proof. This is wiring, not construction. Verified against tools/list on https://mcp.copilotkit.ai/mcp, the server already exposes search-code, search-ag-ui-code and search-ag-ui-docs alongside search-docs, all four with an identical schema (query, limit, min_score, version). The client called one of them. Two parsing defects had to be fixed for any of it to work: - parseSnippets accepted a block only if it matched /TITLE:/, and code results carry REPOSITORY and PATH with no TITLE. Every code hit would have been dropped and searchCode would have returned [] while looking like it worked. A retrieval source that silently contributes nothing is worse than one that errors, because the answer just quietly has less to stand on. - the header regexes were unanchored, so the first `title:`/`source:` ANYWHERE in a block won — and a code block's body is source code, where `title: "Chat"` and `source: 'user'` are everyday object literals. A real run-handler.ts parsed to title `"Chat", source: 'user' };` and sourceUrl `'user' };`, which reached the prompt as `[Source 1: "Chat", source: 'user' };] URL: 'user' };` and buried the path the reply had to cite. Code blocks now take their title from PATH and their URL from a github blob URL built off REPOSITORY + PATH. Retrieval runs both tools and merges with allSettled, not all: one index being down used to throw away the other source's results and answer from nothing. Code leads the interleave, matching the stated source-first precedence, and the merged list is capped at defaultLimit — without the cap the prompt carried up to 2x the sources it did before, and code snippets are line-numbered file excerpts far larger than doc snippets, so input tokens per ticket roughly doubled with a real path to a context-length error that lands in the generator's catch and publishes the apology fallback. The results are also coerced to arrays. allSettled reports a non-promise or an undefined return as *fulfilled*, so a client answering with anything else reached the merge and threw on .length — in the one class whose contract is that it never crashes. Two prompts moved, and both had to: - GROUNDING_RULES opened with "You have NOT read CopilotKit's source code. Never write or imply otherwise." True while retrieval was docs-only, false the moment this landed, and the instruction that told the model to disclaim its best evidence. Replaced with the narrower honest boundary, plus "documentation silence is not evidence a feature is missing" and "where code and docs disagree, the code is what ships". - CONFIDENCE_SYSTEM_PROMPT said the assistant "could not read CopilotKit's source" and to score LOW when a response names a file. A correct code-grounded answer was exactly the shape it marked down, and the pipeline takes min(generator, scorer) — so the scorer would have clawed back the win. Now exported so the two prompts can be pinned against each other by test, since a contradiction between them is invisible at runtime. AG-UI is deliberately NOT in the pipeline's default path. The methods exist and are tested, but firing them on every CopilotKit question buys noise and spend with no way to tell when they are relevant; choosing strategy from the kind of question asked is the doc's step 5 and needs the classifier. searchDocs is deliberately not folded into the shared helper: it falls back to a plain-text docs search on error, where the code tools have no analogue and return []. Verification: ai package 270 -> 293, full repo turbo run test 10/10, typecheck clean. Mutations checked, each killing exactly the tests that name it: TITLE-only filter, dropped blob URL, concatenate instead of interleave, Promise.all semantics, no cap, and the false line restored to the confidence prompt. Anchoring PATH/REPOSITORY specifically is defence in depth rather than load-bearing — a header always precedes CONTENT, so no test can distinguish it; the load-bearing half is preferring PATH over TITLE. Two existing tests broke honestly and were rewritten rather than patched around: the GROUNDING_RULES test asserted the now-false "have NOT read" line, and pipeline-groundedness stubbed only searchDocs. Refs CPK-8077
Addresses the review on #242. The anchoring fix in the previous commit was asymmetric. `/^\s*PATH:/im` was matched against the whole block, body included, and `^\s*` allows code indentation while `i` allows lowercase — so a DOCUMENTATION snippet whose content quotes source code got a truthy `path`, was treated as a code block, and lost its docs URL. Verified against the real function with a snippet from the self-hosting guide: TITLE: Self-hosting the CopilotKit Runtime SOURCE: https://docs.copilotkit.ai/guides/self-hosting CONTENT: const handler = copilotRuntimeNextJSAppRouter({ path: "/api/copilotkit", }); before: title '"/api/copilotkit",' sourceUrl undefined after: title 'Self-hosting the CopilotKit Runtime' sourceUrl 'https://docs.copilotkit.ai/guides/self-hosting' Same defect as the one fixed for code blocks, reintroduced on the docs side — a body line winning over a real header. A docs page with no URL cannot be cited, and #241's source-link-or-handoff rule then collapses that answer into a two-sentence handoff, so the cost lands on the reporter. Headers are now read only from the region above `CONTENT:`, which removes the class rather than the two spellings that happened to be noticed. A block is code when it has a PATH and no TITLE, derived from the headers rather than from PATH alone. Also from the same review: - `SearchResult.kind` carries docs-vs-code through to the prompt. GROUNDING_RULES asserts "code entries are shown with their file path" and that the code wins a conflict with the docs, but buildSystemPrompt rendered both as an identical `[Source N: title]` — so the instruction most in need of being reliable, now that "you have NOT read the source" is gone, was resolvable only by guessing. Sources are labelled SOURCE CODE / DOCS, and an unlabelled source stays unlabelled rather than being asserted as either. - Each tool now gets half the retrieval budget instead of a full `defaultLimit` each. Over-fetching paid for 16 snippets to keep 8, and cost docs recall on the majority path: a purely docs-answerable question would have got 4 docs hits rather than 8, the rest going to code that merely cleared min_score. - `blobUrl` logs instead of returning undefined silently. A bare slug, an SSH remote, or a missing REPOSITORY on one index would leave EVERY code hit with nothing to cite and no trace — the silent-degradation shape this change exists to remove, one layer up. - `search()`'s doc comment claimed private tool names remove the silent no-retrieval mode. They do not: `SearchTool` is a compile-time union and cannot know what the server exposes, so a server-side rename still yields one console.error and []. Comment corrected and pointed at #244. Verification: ai package 293 -> 303, full repo turbo run test 10/10, typecheck clean. On mutations: reverting the header region alone, or the isCode rule alone, does NOT fail a test — three layers independently prevent this (title reads TITLE before PATH, isCode requires no TITLE, headers come only from above CONTENT:). Reverting the two structural ones together fails two tests. Dropping the prompt label, and restoring the full per-tool limit, each fail the tests that name them. Refs CPK-8077
fa12023 to
cb35b14
Compare
Phase 3 of the response-quality work (CPK-8078), first slice. The doc's flowchart hangs on one arrow: "if the draft breaks a rule, it doesn't get cleaned up and posted — it collapses into the two-sentence version." This is that arrow, and it runs the same rules the harness scores with, from the same module, so the thing measured and the thing enforced cannot drift. ## The two prerequisites Jerel named on #241, which had to come first Both would have made the linter withhold correct answers, which is the same failure direction as the groundedness gate suppressing one — the bug #234 was filed for. 1. The citation rule tested whether a link LOOKED like ours, so the URL was both the citation and the laundering: a reply could write its invented hook name inside a docs.copilotkit.ai link — /hooks/useCopilotFabricated — and satisfy the rule with a page that does not exist. The identifier rule cannot catch it either, because assessGroundedness blanks URLs before it looks. The check now matches against the URLs actually retrieved. 2. Pathfinder's plain-text fallback (textSearch) sets sourceUrl: undefined on every result, so a CORRECT answer built from it has nothing it could cite. Under a flat requirement that answer fails forever and, once these rules gate publishing, collapses into a handoff every time the fallback is in play. Closing 2 needed a third state, so RuleResult gains `applicable`: "did not cite" and "had nothing citable" are different facts. A not-applicable rule is never a failure, never withholds a draft, and is not counted in a pass rate — which also fixes a rule that was inapplicable everywhere reading as a clean sweep, since `passed === total` is trivially true at 0/0. formatReport prints `n/a` for those rather than `ok`. ## Report mode is the default `lintDraft` computes the verdict and changes nothing unless asked to enforce. Enforcing means a misfiring rule withholds a correct answer from a real person, so the sequence is: run in report mode, read what it would have collapsed against real traffic, then enforce once the false-positive rate is known rather than assumed. `wouldCollapse` carries the counterfactual so report mode is worth running. It returns a verdict, never replacement copy. The caller substitutes its own, and in the pipeline that is the existing SUPPRESSED_RESPONSE_TEXT — that copy already promises a human follow-up, and #231 records what happens when two layers each add their own promise. ## Not wired into the pipeline here, deliberately The wiring belongs in pipeline.ts, which #242 is already editing on another branch. Landing both would collide over the same function for no benefit, since a report-mode linter changes nothing until someone reads its output. Wiring follows once #242 is in. Verification: ai package 318 -> 334, full repo turbo run test 10/10, typecheck clean. Four mutations, each killing the tests that name it: the laundering-permissive citation check, an always-applicable citation rule, report mode withholding, and enforce counting inapplicable rules as failures. Two tests were updated rather than patched around: both cited arbitrary docs-shaped URLs absent from their fixture's sources, which is precisely the laundering the new check closes. Stacked on #241 (feat/response-quality-eval-harness) because it consumes that rule module; rebases onto whatever that review lands. Refs CPK-8078
jerelvelarde
left a comment
There was a problem hiding this comment.
Re-reviewed. The blocker is closed, and closed structurally rather than by patching the two spellings that happened to get noticed — headers are read only from block.slice(0, contentAt), and isCode = !titleHeader && !!path. ai/src is 303 tests.
I ran the exact case from my review through the suite. It is now a fixture (pathfinder.test.ts:252-288), and reverting both structural fixes reproduces the original defect precisely:
AssertionError: expected undefined to be 'https://docs.copilotkit.ai/guides/self-hosting'
AssertionError: expected 'code' to be 'docs'
That is the failure I described — a docs page losing its URL and being misclassified — asserted rather than argued.
Mutations, run:
| Mutation | Result |
|---|---|
headerRegion → whole block |
303 passed (survives) |
isCode → !!path |
303 passed (survives) |
| both together | 2 failed / 301 passed |
| code/docs prompt label removed | 1 failed / 302 passed |
per-tool limit → full defaultLimit |
2 failed / 301 passed |
Your statement of this was accurate, including the part that does not flatter the change — the two structural layers are redundant, so neither is independently pinned. I would take the follow-up rather than leave it: a test per layer, or an explicit comment that the redundancy is deliberate and untested. As it stands, someone simplifying one layer away in six months gets a green suite and a silently reduced defence.
Approving.
Three notes, none blocking
- The prompt labels are the fix I care most about here.
SearchResult.kindreachingbuildSystemPromptasSOURCE CODE/DOCSis what makes "where code and docs disagree, the code is what ships" resolvable instead of guessable, and leaving an unlabelled source unlabelled rather than asserting a default is the right call. isCodeis correct today and fragile tomorrow. It depends on code results never carrying aTITLE. That holds against the currenttools/list, and it is a server-side contract you do not control — if a code hit ever gains a title,sourcefalls back toheader('SOURCE'), which is absent, and the file path silently stops being citable. A one-line comment naming the dependency would be enough.blobUrllogging closes the trace gap, and pinning the streaming-path question as #245 rather than a line in the PR body is the right disposition. Your observation that #243/#244/#245 all describe the same underlying thing — a second copy of the pipeline nobody exercises — reads as the real finding. Parity or deletion, but not a third improvement that skips it.
Phase 3 of the response-quality work (CPK-8078), first slice. The doc's flowchart hangs on one arrow: "if the draft breaks a rule, it doesn't get cleaned up and posted — it collapses into the two-sentence version." This is that arrow, and it runs the same rules the harness scores with, from the same module, so the thing measured and the thing enforced cannot drift. ## The two prerequisites Jerel named on #241, which had to come first Both would have made the linter withhold correct answers, which is the same failure direction as the groundedness gate suppressing one — the bug #234 was filed for. 1. The citation rule tested whether a link LOOKED like ours, so the URL was both the citation and the laundering: a reply could write its invented hook name inside a docs.copilotkit.ai link — /hooks/useCopilotFabricated — and satisfy the rule with a page that does not exist. The identifier rule cannot catch it either, because assessGroundedness blanks URLs before it looks. The check now matches against the URLs actually retrieved. 2. Pathfinder's plain-text fallback (textSearch) sets sourceUrl: undefined on every result, so a CORRECT answer built from it has nothing it could cite. Under a flat requirement that answer fails forever and, once these rules gate publishing, collapses into a handoff every time the fallback is in play. Closing 2 needed a third state, so RuleResult gains `applicable`: "did not cite" and "had nothing citable" are different facts. A not-applicable rule is never a failure, never withholds a draft, and is not counted in a pass rate — which also fixes a rule that was inapplicable everywhere reading as a clean sweep, since `passed === total` is trivially true at 0/0. formatReport prints `n/a` for those rather than `ok`. ## Report mode is the default `lintDraft` computes the verdict and changes nothing unless asked to enforce. Enforcing means a misfiring rule withholds a correct answer from a real person, so the sequence is: run in report mode, read what it would have collapsed against real traffic, then enforce once the false-positive rate is known rather than assumed. `wouldCollapse` carries the counterfactual so report mode is worth running. It returns a verdict, never replacement copy. The caller substitutes its own, and in the pipeline that is the existing SUPPRESSED_RESPONSE_TEXT — that copy already promises a human follow-up, and #231 records what happens when two layers each add their own promise. ## Not wired into the pipeline here, deliberately The wiring belongs in pipeline.ts, which #242 is already editing on another branch. Landing both would collide over the same function for no benefit, since a report-mode linter changes nothing until someone reads its output. Wiring follows once #242 is in. Verification: ai package 318 -> 334, full repo turbo run test 10/10, typecheck clean. Four mutations, each killing the tests that name it: the laundering-permissive citation check, an always-applicable citation rule, report mode withholding, and enforce counting inapplicable rules as failures. Two tests were updated rather than patched around: both cited arbitrary docs-shaped URLs absent from their fixture's sources, which is precisely the laundering the new check closes. Stacked on #241 (feat/response-quality-eval-harness) because it consumes that rule module; rebases onto whatever that review lands. Refs CPK-8078
Phase 3 of the response-quality work (CPK-8078), first slice. The doc's flowchart hangs on one arrow: "if the draft breaks a rule, it doesn't get cleaned up and posted — it collapses into the two-sentence version." This is that arrow, and it runs the same rules the harness scores with, from the same module, so the thing measured and the thing enforced cannot drift. ## The two prerequisites Jerel named on #241, which had to come first Both would have made the linter withhold correct answers, which is the same failure direction as the groundedness gate suppressing one — the bug #234 was filed for. 1. The citation rule tested whether a link LOOKED like ours, so the URL was both the citation and the laundering: a reply could write its invented hook name inside a docs.copilotkit.ai link — /hooks/useCopilotFabricated — and satisfy the rule with a page that does not exist. The identifier rule cannot catch it either, because assessGroundedness blanks URLs before it looks. The check now matches against the URLs actually retrieved. 2. Pathfinder's plain-text fallback (textSearch) sets sourceUrl: undefined on every result, so a CORRECT answer built from it has nothing it could cite. Under a flat requirement that answer fails forever and, once these rules gate publishing, collapses into a handoff every time the fallback is in play. Closing 2 needed a third state, so RuleResult gains `applicable`: "did not cite" and "had nothing citable" are different facts. A not-applicable rule is never a failure, never withholds a draft, and is not counted in a pass rate — which also fixes a rule that was inapplicable everywhere reading as a clean sweep, since `passed === total` is trivially true at 0/0. formatReport prints `n/a` for those rather than `ok`. ## Report mode is the default `lintDraft` computes the verdict and changes nothing unless asked to enforce. Enforcing means a misfiring rule withholds a correct answer from a real person, so the sequence is: run in report mode, read what it would have collapsed against real traffic, then enforce once the false-positive rate is known rather than assumed. `wouldCollapse` carries the counterfactual so report mode is worth running. It returns a verdict, never replacement copy. The caller substitutes its own, and in the pipeline that is the existing SUPPRESSED_RESPONSE_TEXT — that copy already promises a human follow-up, and #231 records what happens when two layers each add their own promise. ## Not wired into the pipeline here, deliberately The wiring belongs in pipeline.ts, which #242 is already editing on another branch. Landing both would collide over the same function for no benefit, since a report-mode linter changes nothing until someone reads its output. Wiring follows once #242 is in. Verification: ai package 318 -> 334, full repo turbo run test 10/10, typecheck clean. Four mutations, each killing the tests that name it: the laundering-permissive citation check, an always-applicable citation rule, report mode withholding, and enforce counting inapplicable rules as failures. Two tests were updated rather than patched around: both cited arbitrary docs-shaped URLs absent from their fixture's sources, which is precisely the laundering the new check closes. Stacked on #241 (feat/response-quality-eval-harness) because it consumes that rule module; rebases onto whatever that review lands. Refs CPK-8078
… layer The non-blocking follow-ups from Jerel's reviews of #241 and #242. ## The fixtures really do ship, and not re-exporting them was never the fix Verified against a built `dist` rather than reasoned about: `tsc` emits per file and `index.ts` imports `./eval/harness.js`, so while HISTORICAL_FAILURES and TARGET_SHAPE lived in `harness.ts` the reconstructed bad replies shipped in `dist/eval/harness.js` no matter what the entry point declared. `grep -rl "useCopilotFabricatedRender" dist/` hit it. They now live in `eval/__fixtures__/historical-failures.ts`, and `ai/tsconfig.json` excludes `**/*.test.ts`, `**/__tests__/**` and `**/__fixtures__/**` from the build. `harness.ts` no longer imports them, which it cannot: a compiled module importing an excluded one emits a broken build. Excluding test material also fixes something wider that was never raised — every `.test.ts` in the package was being compiled into `dist`. That is now 0 files. Post-build audit: no fixture reply text, no invented hook name, no `__fixtures__` directory. `@copilotkitnext` still appears in `dist/eval/rules.js`, and has to — that is the rule which bans it. A bundle grep for the dead package name hits the rule, not a fabricated example of it. The comment in `index.ts` now says that instead of claiming an exclusion it did not deliver. ## Each parser layer is pinned separately Three layers independently prevent a docs block being read as code — the title prefers TITLE over PATH, `isCode` requires the absence of TITLE, and headers are read only from above `CONTENT:`. The redundancy is deliberate, and it meant no single layer was pinned: reverting any one alone left the suite green, so someone simplifying one away in six months would have got a clean run and a quietly reduced defence. Two tests isolate a layer each: - a block carrying BOTH TITLE and PATH must read as docs and keep its SOURCE. Kills the title-order revert and the `isCode` revert. - a docs block with no SOURCE header and a line-initial `SOURCE:` inside its content must not adopt that line as its citation. Kills the header-region revert. Verified: each single-layer revert now fails exactly one test, where before all three survived alone. `isCode` also gains a comment naming what it depends on — that a code hit never carries a TITLE, which is a server-side contract we do not own. If a code result ever gains one, `source` falls back to an absent `SOURCE` header and the file path silently stops being citable. The both-headers test is what makes that change in behaviour visible. Verification: ai package 377 tests, typecheck 10/10, test 10/10.
… layer The non-blocking follow-ups from Jerel's reviews of #241 and #242. ## The fixtures really do ship, and not re-exporting them was never the fix Verified against a built `dist` rather than reasoned about: `tsc` emits per file and `index.ts` imports `./eval/harness.js`, so while HISTORICAL_FAILURES and TARGET_SHAPE lived in `harness.ts` the reconstructed bad replies shipped in `dist/eval/harness.js` no matter what the entry point declared. `grep -rl "useCopilotFabricatedRender" dist/` hit it. They now live in `eval/__fixtures__/historical-failures.ts`, and `ai/tsconfig.json` excludes `**/*.test.ts`, `**/__tests__/**` and `**/__fixtures__/**` from the build. `harness.ts` no longer imports them, which it cannot: a compiled module importing an excluded one emits a broken build. Excluding test material also fixes something wider that was never raised — every `.test.ts` in the package was being compiled into `dist`. That is now 0 files. Post-build audit: no fixture reply text, no invented hook name, no `__fixtures__` directory. `@copilotkitnext` still appears in `dist/eval/rules.js`, and has to — that is the rule which bans it. A bundle grep for the dead package name hits the rule, not a fabricated example of it. The comment in `index.ts` now says that instead of claiming an exclusion it did not deliver. ## Each parser layer is pinned separately Three layers independently prevent a docs block being read as code — the title prefers TITLE over PATH, `isCode` requires the absence of TITLE, and headers are read only from above `CONTENT:`. The redundancy is deliberate, and it meant no single layer was pinned: reverting any one alone left the suite green, so someone simplifying one away in six months would have got a clean run and a quietly reduced defence. Two tests isolate a layer each: - a block carrying BOTH TITLE and PATH must read as docs and keep its SOURCE. Kills the title-order revert and the `isCode` revert. - a docs block with no SOURCE header and a line-initial `SOURCE:` inside its content must not adopt that line as its citation. Kills the header-region revert. Verified: each single-layer revert now fails exactly one test, where before all three survived alone. `isCode` also gains a comment naming what it depends on — that a code hit never carries a TITLE, which is a server-side contract we do not own. If a code result ever gains one, `source` falls back to an absent `SOURCE` header and the file path silently stops being citable. The both-headers test is what makes that change in behaviour visible. Verification: ai package 377 tests, typecheck 10/10, test 10/10.
Addresses the blocker on #257. Jerel was right, and the regression was mine: the exclusions went into `ai/tsconfig.json`, which is also what `typecheck` reads (`tsc --project ai/tsconfig.json --noEmit`) — so excluding test material stopped it being typechecked at all. Reproduced his proof before changing anything. With `const __probe: number = 'definitely not a number'` in `eval/rules.test.ts`: - merged main: error TS2322: Type 'string' is not assignable to type 'number' - #257 as pushed: pnpm typecheck clean, 10/10 So the PR traded "tests ship in dist" for "test type errors reach main", which is the worse half — vitest exercises runtime behaviour, so a test asserting against a shape that no longer exists surfaces as a confusing failure or not at all. It landed directly after #242 added `SearchResult.kind`, which is exactly the kind of change whose test fallout wants a compiler. `ai/tsconfig.build.json` now carries the exclusions and `build:ai` points at it. The default config goes back to seeing everything, so anything inheriting it inherits the safer default. Both properties verified together: typecheck catches the injected error again, and a clean build emits 18 JS files with 0 test files and no `__fixtures__`. He asked for a test that fails when a test file stops being typechecked. There is one now — `__tests__/typecheck-config.test.ts` asserts that whatever `typecheck` reads does not exclude test material, that the build config does, and that `build:ai` invokes the build config rather than the inclusive one. Checked in both directions: re-adding the exclusion to `tsconfig.json` fails it, and pointing `build:ai` back at `tsconfig.json` fails it. Two notes on writing that guard, since both bugs were the same shape as the change itself: - The first version stripped tsconfig comments by hand, and its block-comment pattern matched inside the exclude values — a doubled-star glob followed by a slash-star extension reads as a comment opener. It collapsed three patterns into one mangled string and failed against config that was correct. It now uses `ts.parseConfigFileTextToJson`. - Explaining that in a docstring then broke the file, because writing those globs literally inside a block comment closes it early. They are described rather than quoted. Not addressed here, and worth its own change: `db`, `queue` and `shared` have no such split, so their tests are still compiled into their published output. Same packaging wart, three more packages, and the same fix applies to each.
Phase 2 of the response-quality work — CPK-8077, from the Agent's Output Doc.
Root cause 1 in that doc: for any question whose answer lives in the source — most of the hard ones — the agent had only the docs and was otherwise guessing from general React knowledge. A reporter asked whether Deep Agents supports subagents; the docs don't mention it, so the agent said it had no timeline and sent them to GitHub to ask. Subagents work today and one code search returns the proof.
This is wiring, not construction
Probed
tools/listonhttps://mcp.copilotkit.ai/mcprather than assuming. The server already exposessearch-code,search-ag-ui-codeandsearch-ag-ui-docsalongsidesearch-docs— all four with an identical schema (query,limit,min_score,version). The client called one of them, and never calledtools/list, so the names were hardcoded and could drift silently.Two parsing defects had to be fixed for any of it to work
Neither was visible from reading the code — both turned up from calling the real tool and reading the real response.
1. Every code hit would have been dropped.
parseSnippetsaccepted a block only if it matched/TITLE:/, and code results carryREPOSITORYandPATHwith noTITLE. SosearchCodewould have returned[]on every call while looking like it worked. A retrieval source that silently contributes nothing is worse than one that errors — the answer just quietly has less to stand on.2. The header regexes were unanchored, so the first
title:/source:anywhere in a block won. A code block's body is source code, wheretitle: "Chat"andsource: 'user'are everyday object literals. Run against a realistic snippet:That reached the prompt as
[Source 1: "Chat", source: 'user' };]/URL: 'user' };,blobUrlnever ran, and the file path the reply was supposed to cite was gone. Code blocks now take their title fromPATHand their URL from a GitHub blob URL built offREPOSITORY+PATH— because "a repo link is a real answer", and without a URL a code-sourced answer has nothing to cite and the Phase 1 rules force it into a handoff.Retrieval
Both tools run in parallel, merged with
allSettled, notall— one index being down used to throw away the other source's results and answer from nothing. Code leads the interleave, matching the stated source-first precedence. The merged list is capped atdefaultLimit: without it the prompt carried up to 2× the sources it did before, and code snippets are line-numbered file excerpts far larger than doc snippets, so input tokens per ticket roughly doubled — with a real path to a context-length error that lands in the generator's catch and publishes the apology fallback.Results are also coerced to arrays.
allSettledreports a non-promise orundefinedreturn as fulfilled, so a client answering with anything else reached the merge and threw on.length, in the one class whose contract is that it never crashes. Four existing calibration tests caught that.Both prompts had to move, and that's the subtle half
GROUNDING_RULESopened with "You have NOT read CopilotKit's source code. Never write or imply otherwise." True while retrieval was docs-only; false the moment this lands. It's the instruction that told the model to disclaim its best evidence, and it's the mechanism behind the Deep Agents answer. Replaced with the narrower honest boundary — retrieved code is fair to cite, un-retrieved files aren't, a repro and a test run remain impossible — plus documentation silence is not evidence a feature is missing and where code and docs disagree, the code is what ships.CONFIDENCE_SYSTEM_PROMPTsaid the assistant "could not read CopilotKit's source" and to score LOW when a response names a file. A correct code-grounded answer was exactly the shape it marked down, and the pipeline takesmin(generator, scorer)— so the scorer would have clawed back the win. Now exported, so the two prompts can be pinned against each other by test; a contradiction between them is otherwise invisible at runtime.Deliberate choices, happy to be overruled
mainassumed for blob URLs. The tool response carries no ref. A wrong branch 404s rather than pointing somewhere misleading.searchDocsnot folded into the shared helper. It falls back to a plain-text docs search on error; the code tools have no analogue and return[].Verification
aipackage 270 → 293. Full repoturbo run test10/10.tsc --noEmitclean.Mutations checked, each killing exactly the tests that name it: TITLE-only filter · dropped blob URL · concatenate instead of interleave ·
Promise.allsemantics · no cap · the false line restored to the confidence prompt.One mutation survived, and it's worth stating: anchoring
PATH/REPOSITORYspecifically is defence in depth, not load-bearing — a header always precedesCONTENT, so no test can distinguish anchored from unanchored there. The load-bearing half is preferringPATHoverTITLE.Two existing tests broke honestly and were rewritten rather than patched around: the
GROUNDING_RULEStest asserted the now-false "have NOT read" line (and gained a guard so it can't come back), andpipeline-groundednessstubbed onlysearchDocs.Follow-ups filed, not fixed here
Three findings from the review that want their own change — see the linked issues. Briefly: the synthetic
score: 1.0on a marginal top code hit inflates retrieval confidence on any question returning code;search()collapsing to[]makes a code-index outage indistinguishable from "no hits"; andgenerateStreamingResponsestill callssearchDocsonly, so it will silently regress this fix the moment anyone streams.Depends on nothing else in flight — branched from
main, independent of #239, #240 and #241.