diff --git a/packages/outpost/ai/src/eval/harness.test.ts b/packages/outpost/ai/src/eval/harness.test.ts index 32d45ec..5b0ffb6 100644 --- a/packages/outpost/ai/src/eval/harness.test.ts +++ b/packages/outpost/ai/src/eval/harness.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from 'vitest'; import { scoreCases, formatReport, HISTORICAL_FAILURES, TARGET_SHAPE } from './harness.js'; import { RULES } from './rules.js'; +import type { SearchResult } from '../types.js'; describe('scoreCases', () => { it('reports every rule for every case', () => { @@ -38,6 +39,24 @@ describe('the documented failures are all caught', () => { expect(failed).toContain('no-banned-phrases'); }); + // Looked up by id rather than index: the cases above are positional, so + // inserting anywhere but the end silently reassigns which case each of those + // assertions is about. + it('catches case E for the praise opener and the self-positioning paragraph', () => { + const caseE = HISTORICAL_FAILURES.find( + (c) => c.id === 'case-e-mcp-headers-self-commentary', + ); + if (!caseE) throw new Error('case-e fixture is missing'); + + const failed = scoreCases([caseE]).cases[0].failed; + + // `failed` carries rule ids, not which phrase fired, so this pins the + // fixture rather than the pattern — the opener alone would satisfy it. + // Which phrases are caught is pinned in rules.test.ts, where reverting the + // widened pattern fails six cases. + expect(failed).toContain('no-banned-phrases'); + }); + it('catches case B for the dead package and the invented name', () => { const failed = scoreCases([HISTORICAL_FAILURES[1]]).cases[0].failed; expect(failed).toContain('no-dead-package'); @@ -73,6 +92,37 @@ describe('an empty case list', () => { }); }); +// A not-applicable rule carries `passed: false`, so an unfiltered failing-cases +// block printed `n/a` for a rule and then listed it as a failure two lines later — +// re-creating the double-counting in the human-readable output. +describe('the report does not list an unevaluated rule as a failure', () => { + const urlLess: SearchResult[] = [ + { title: 'CopilotChat', content: 'Use the `CopilotChat` component.', score: 0.9 }, + ]; + const report = () => + formatReport( + scoreCases([ + { + id: 'c1', + question: 'q', + reply: 'Great question! ' + 'padding word '.repeat(30), + sources: urlLess, + provenance: 'test', + }, + ]), + ); + + it('marks the citation rule n/a in the per-rule table', () => { + expect(report()).toContain('n/a cites-or-is-a-short-handoff: 0/0'); + }); + + it('does not repeat it under the failing cases', () => { + const failingBlock = report().split('Failing cases:')[1] ?? ''; + expect(failingBlock).toContain('no-banned-phrases'); + expect(failingBlock).not.toContain('cites-or-is-a-short-handoff'); + }); +}); + describe('formatReport', () => { it('leads with the clean count and names each failing rule', () => { const text = formatReport(scoreCases(HISTORICAL_FAILURES)); diff --git a/packages/outpost/ai/src/eval/harness.ts b/packages/outpost/ai/src/eval/harness.ts index 6755625..fe9bbf3 100644 --- a/packages/outpost/ai/src/eval/harness.ts +++ b/packages/outpost/ai/src/eval/harness.ts @@ -76,18 +76,26 @@ export function scoreCases(cases: EvalCase[]): EvalReport { return { id: c.id, results, - failed: results.filter((r) => !r.passed).map((r) => r.rule), + // A rule that could not be evaluated is not a failure. + failed: results.filter((r) => r.applicable && !r.passed).map((r) => r.rule), }; }); + // `total` counts only the cases where the rule could be evaluated, so a rule + // that was inapplicable everywhere reads as 0/0 rather than as a clean sweep. const perRule = Object.fromEntries( - RULES.map((rule) => [ - rule, - { - passed: scored.filter((s) => !s.failed.includes(rule)).length, - total: scored.length, - }, - ]), + RULES.map((rule) => { + const applicable = scored.filter( + (s) => s.results.find((r) => r.rule === rule)?.applicable, + ); + return [ + rule, + { + passed: applicable.filter((s) => !s.failed.includes(rule)).length, + total: applicable.length, + }, + ]; + }), ) as Record; return { @@ -103,14 +111,23 @@ export function formatReport(report: EvalReport): string { const lines = [`${report.cleanCases}/${report.totalCases} cases clean`, '']; for (const rule of RULES) { const { passed, total } = report.perRule[rule]; - lines.push(` ${passed === total ? 'ok ' : 'FAIL'} ${rule}: ${passed}/${total}`); + // `n/a` rather than `ok` when nothing exercised the rule — `passed === total` + // is trivially true at 0/0, which is how an unevaluated rule used to read as + // a clean sweep. + const verdict = total === 0 ? 'n/a ' : passed === total ? 'ok ' : 'FAIL'; + lines.push(` ${verdict} ${rule}: ${passed}/${total}`); } const dirty = report.cases.filter((c) => c.failed.length > 0); if (dirty.length) { lines.push('', 'Failing cases:'); for (const c of dirty) { lines.push(` ${c.id}`); - for (const r of c.results.filter((r) => !r.passed)) { + // Filtered on `applicable` as well: a not-applicable rule carries + // `passed: false`, so without this the report printed `n/a` for a rule + // two lines above and then listed it as a failure — re-creating in the + // human-readable output exactly the double-counting removed from + // `perRule`. + for (const r of c.results.filter((r) => r.applicable && !r.passed)) { lines.push(` - ${r.rule}: ${r.detail}`); } } @@ -203,6 +220,34 @@ export const HISTORICAL_FAILURES: EvalCase[] = [ sources: CHAT_DOCS, provenance: 'https://github.com/CopilotKit/CopilotKit/issues/6423', }, + { + id: 'case-e-mcp-headers-self-commentary', + question: + 'v2 MCP sse servers silently drop the headers auth config — the documented example sends no Authorization header', + // CopilotKit#6927, posted 2026-09-06T17:40Z, 23 seconds after the issue + // opened. The reporter had already done the work: a reproduction, the + // wire-level symptom and a proposed fix. The reply opened by praising the + // write-up, then spent a paragraph announcing what it had not done, then + // handed the question back to engineering. + // + // Kept as a fixture because it is the failure the narrower + // `read the source` pattern missed on the verb alone: only the praise + // opener fired, so the self-positioning paragraph — the part that makes + // the reply worse than silence — published intact. + reply: + '## Thanks for this detailed report\n\n' + + 'This is an exceptionally thorough write-up — the reproduction output, the proposed ' + + 'fix, and the note about test coverage are all exactly what the maintainers need to ' + + 'evaluate this quickly.\n\n' + + "To be clear about my position: I haven't run this code or inspected the source, so " + + "I can't confirm the root cause or validate the fix independently. What I can say is " + + 'that the behavior you are describing is consistent with the kind of mismatch that ' + + 'can happen when a parameter type changes shape across SDK versions.\n\n' + + 'Engineering will need to verify the internal behavior and decide on the right fix.', + sources: [], + provenance: + 'https://github.com/CopilotKit/CopilotKit/issues/6927#issuecomment — posted 2026-09-06, before #241/#242 merged', + }, ]; /** diff --git a/packages/outpost/ai/src/eval/linter.test.ts b/packages/outpost/ai/src/eval/linter.test.ts new file mode 100644 index 0000000..e4efd2c --- /dev/null +++ b/packages/outpost/ai/src/eval/linter.test.ts @@ -0,0 +1,141 @@ +import { describe, it, expect } from 'vitest'; +import { lintDraft, describeVerdict } from './linter.js'; +import type { SearchResult } from '../types.js'; + +const DOCS: SearchResult[] = [ + { + title: 'CopilotChat', + content: 'Use the `CopilotChat` component with the `instructions` prop.', + score: 0.9, + sourceUrl: 'https://docs.copilotkit.ai/reference/components/chat/CopilotChat', + }, +]; + +/** A reply that breaks nothing: grounded, cited, no banned phrasing. */ +const GOOD = + 'Use the `CopilotChat` component with the `instructions` prop. ' + + 'https://docs.copilotkit.ai/reference/components/chat/CopilotChat'; + +/** Case D's shape: praise opener, self-commentary, no citation, over the cap. */ +const BAD = + 'Great question! Here is what I cannot do from here: I cannot read the source. ' + + 'To summarise what you have found, the versions disagree. '.repeat(6); + +describe('report mode', () => { + it('is the default, so wiring it in cannot change what publishes', () => { + expect(lintDraft(BAD, DOCS).mode).toBe('report'); + }); + + // The whole point of the mode: learn what enforcement would do to real + // traffic before letting it withhold anything from a real person. + it('publishes a failing draft while recording that it would not have', () => { + const verdict = lintDraft(BAD, DOCS); + + expect(verdict.publish).toBe(true); + expect(verdict.wouldCollapse).toBe(true); + expect(verdict.failed).toContain('no-banned-phrases'); + }); + + it('publishes a clean draft and says nothing would have collapsed', () => { + const verdict = lintDraft(GOOD, DOCS); + + expect(verdict.publish).toBe(true); + expect(verdict.wouldCollapse).toBe(false); + expect(verdict.failed).toEqual([]); + }); +}); + +describe('enforce mode', () => { + it('withholds a failing draft', () => { + const verdict = lintDraft(BAD, DOCS, 'enforce'); + + expect(verdict.publish).toBe(false); + expect(verdict.wouldCollapse).toBe(true); + }); + + it('publishes a clean draft', () => { + expect(lintDraft(GOOD, DOCS, 'enforce').publish).toBe(true); + }); + + // A rule that could not be evaluated must never withhold an answer. The live + // case: Pathfinder's plain-text fallback returns results with no sourceUrl, so + // a correct answer built from it has nothing it could cite — enforcing a + // citation there would collapse every such answer. + it('does not withhold on a rule that could not be evaluated', () => { + const urlLess: SearchResult[] = [ + { title: 'CopilotChat', content: 'Use the `CopilotChat` component.', score: 0.9 }, + ]; + const long = 'Use the `CopilotChat` component as documented. '.repeat(12); + + const verdict = lintDraft(long, urlLess, 'enforce'); + + expect(verdict.failed).not.toContain('cites-or-is-a-short-handoff'); + expect(verdict.publish).toBe(true); + }); + + it('reports every rule either way, so a log line can show the whole picture', () => { + expect(lintDraft(GOOD, DOCS, 'enforce').results).toHaveLength(6); + }); +}); + +describe('the reasons', () => { + it('name the rule and the offending text', () => { + const verdict = lintDraft('Great question! ' + 'padding word '.repeat(10), DOCS); + + expect(verdict.reasons.join(' ')).toContain('no-banned-phrases'); + expect(verdict.reasons.join(' ')).toContain('praise opener'); + }); +}); + +describe('describeVerdict', () => { + it('distinguishes a report-mode near-miss from an enforced collapse', () => { + expect(describeVerdict(lintDraft(BAD, DOCS), 'ticket-1')).toContain( + 'published anyway (report mode)', + ); + expect(describeVerdict(lintDraft(BAD, DOCS, 'enforce'), 'ticket-1')).toContain( + 'collapsed to a handoff', + ); + }); + + it('stays quiet-but-informative on a clean draft', () => { + expect(describeVerdict(lintDraft(GOOD, DOCS), 'ticket-1')).toBe( + '[DraftLinter] ticket-1: clean (report)', + ); + }); +}); + +// The harness scores the doc's metric — zero invented API names — while the +// pipeline's groundedness gate suppresses only at two. A linter that collapsed at +// one would withhold answers production publishes. +describe('the metric and the publish gate are not the same', () => { + const SRC: SearchResult[] = [ + { + title: 'CopilotChat', + content: 'Use the `CopilotChat` component.', + score: 0.9, + sourceUrl: 'https://docs.copilotkit.ai/reference', + }, + ]; + const cite = ' https://docs.copilotkit.ai/reference'; + + it('fails the metric on one invented name but still publishes', () => { + const reply = + 'Call `useCopilotFabricated()` with CopilotChat as documented in the reference guide.' + + cite; + const verdict = lintDraft(reply, SRC, 'enforce'); + + expect(verdict.results.find((r) => r.rule === 'grounded-identifiers')?.passed).toBe(false); + expect(verdict.publish).toBe(true); + expect(verdict.failed).not.toContain('grounded-identifiers'); + }); + + it('withholds at the production threshold of two', () => { + const reply = + 'Call `useCopilotFabricated()` and `` as documented in the guide.' + + cite; + const verdict = lintDraft(reply, SRC, 'enforce'); + + expect(verdict.publish).toBe(false); + expect(verdict.failed).toContain('grounded-identifiers'); + }); +}); diff --git a/packages/outpost/ai/src/eval/linter.ts b/packages/outpost/ai/src/eval/linter.ts new file mode 100644 index 0000000..87abb9f --- /dev/null +++ b/packages/outpost/ai/src/eval/linter.ts @@ -0,0 +1,106 @@ +/** + * The draft linter — step 3 of "Fix the Agent's Output". + * + * 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. A short honest reply is always the fallback."* This module is that + * arrow. It runs the same rules the eval harness scores with — deliberately the + * same module, so the thing measured and the thing enforced cannot drift — and + * returns a decision. + * + * ## It returns a verdict, not copy + * + * `lintDraft` never produces replacement text. The caller substitutes its own, + * and in the pipeline that is the existing `SUPPRESSED_RESPONSE_TEXT` used by the + * groundedness gate. That is deliberate: that copy already promises a human + * follow-up, and #231 records what happens when two layers each add their own + * promise — the reporter is told twice. One owner for user-facing copy, one + * promise. + * + * ## Report-only by default + * + * `mode` defaults to `'report'`, which computes the verdict and changes nothing. + * Enforcing means a rule that misfires withholds a correct answer from a real + * person — the same failure direction as the groundedness gate suppressing one, + * which is the bug #234 was filed for. 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. + * + * A rule that could not be evaluated never collapses a draft. See + * `RuleResult.applicable` — Pathfinder's plain-text fallback returns results with + * no `sourceUrl` at all, so a correct answer built from it has nothing it could + * cite, and enforcing a citation there would collapse every such answer. + */ + +import { checkReply } from './rules.js'; +import type { RuleId, RuleResult } from './rules.js'; +import type { SearchResult } from '../types.js'; + +export type LintMode = 'report' | 'enforce'; + +export interface LintVerdict { + /** + * Whether the caller should publish the draft. + * + * Always true in `'report'` mode, whatever the rules found — reporting is for + * learning what enforcement would do, so it must not change behaviour. + */ + publish: boolean; + /** True when at least one applicable rule failed, regardless of mode. */ + wouldCollapse: boolean; + /** The applicable rules the draft broke, in rule order. */ + failed: RuleId[]; + /** One line per failure, naming the offending text. */ + reasons: string[]; + /** Every rule's result, for logging and for the eval harness. */ + results: RuleResult[]; + mode: LintMode; +} + +/** + * Judge a draft against the reply rules. + * + * `sources` must be the results the draft was generated from — the citation and + * identifier rules are lookups against them, so passing a different set silently + * turns the two strictest rules into no-ops. + */ +export function lintDraft( + reply: string, + sources: SearchResult[], + mode: LintMode = 'report', +): LintVerdict { + const results = checkReply(reply, sources); + // Gates on `blocksPublish`, not on `passed`. The harness scores the doc's + // metric — zero invented API names — while the pipeline's groundedness gate + // suppresses only at two, and a linter that collapsed at one would withhold + // answers production publishes. See RuleResult.blocksPublish. + const broken = results.filter((r) => r.applicable && r.blocksPublish); + + return { + publish: mode === 'report' ? true : broken.length === 0, + wouldCollapse: broken.length > 0, + failed: broken.map((r) => r.rule), + reasons: broken.map((r) => `${r.rule}: ${r.detail}`), + results, + mode, + }; +} + +/** + * One log line describing what the linter decided, for report-mode runs. + * + * Report mode is only worth anything if somebody can read the outcome, and #197 + * is why this returns a string for the caller to log rather than reaching for a + * logger itself: nothing in this package has adopted the structured logger, so a + * caller-owned `console` line is the honest option rather than inventing a + * second convention here. + */ +export function describeVerdict(verdict: LintVerdict, context: string): string { + if (!verdict.wouldCollapse) { + return `[DraftLinter] ${context}: clean (${verdict.mode})`; + } + const action = verdict.publish + ? 'would have collapsed to a handoff, published anyway (report mode)' + : 'collapsed to a handoff'; + return `[DraftLinter] ${context}: ${action} — ${verdict.reasons.join(' | ')}`; +} diff --git a/packages/outpost/ai/src/eval/rules.test.ts b/packages/outpost/ai/src/eval/rules.test.ts index a4f157c..64e8f2b 100644 --- a/packages/outpost/ai/src/eval/rules.test.ts +++ b/packages/outpost/ai/src/eval/rules.test.ts @@ -14,7 +14,8 @@ const DOCS = [source('Use the `CopilotChat` component with the `instructions` pr /** Convenience: the ids of every rule the reply violated. */ const broken = (reply: string, sources: SearchResult[] = DOCS): string[] => checkReply(reply, sources) - .filter((r) => !r.passed) + // A not-applicable rule is not a failure. See RuleResult.applicable. + .filter((r) => r.applicable && !r.passed) .map((r) => r.rule); describe('checkReply', () => { @@ -67,18 +68,30 @@ describe('cites-or-is-a-short-handoff', () => { expect(broken(wordy)).toContain('cites-or-is-a-short-handoff'); }); - it('passes a long answer that links the docs', () => { + // Both of these cite the URL the fixture actually retrieved. An arbitrary + // docs-shaped URL no longer counts — see "the citation rule consults the + // retrieved sources" below for why. + it('passes a long answer that links the docs page it was given', () => { const wordy = 'You can configure this in several ways. '.repeat(12) + - ' https://docs.copilotkit.ai/guides/configuration'; + ' https://docs.copilotkit.ai/reference/components/chat/CopilotChat'; expect(broken(wordy)).not.toContain('cites-or-is-a-short-handoff'); }); - it('passes a long answer that links a repo file, since code is a real answer', () => { + it('passes a long answer that links a retrieved repo file, since code is a real answer', () => { + const CODE = [ + { + title: 'packages/runtime/src/agent.ts', + content: 'export function streamSubgraphEvents() {}', + score: 0.9, + sourceUrl: + 'https://github.com/CopilotKit/CopilotKit/blob/main/packages/runtime/src/agent.ts', + }, + ]; const wordy = 'This is handled by the adapter. '.repeat(12) + ' https://github.com/CopilotKit/CopilotKit/blob/main/packages/runtime/src/agent.ts'; - expect(broken(wordy)).not.toContain('cites-or-is-a-short-handoff'); + expect(broken(wordy, CODE)).not.toContain('cites-or-is-a-short-handoff'); }); it('passes a short handoff with no link at all', () => { @@ -90,6 +103,148 @@ describe('cites-or-is-a-short-handoff', () => { // "Nothing found -> two sentences, done." A no-answer that runs to 400 words is // the single thing the doc says changes the feel of the product most. +// Both of these block promoting the rules to a linter, where a false positive +// costs a reporter a correct answer. +describe('the citation rule consults the retrieved sources', () => { + const wordy = (tail: string) => 'You can configure this in several ways. '.repeat(12) + tail; + + // The URL was both the citation and the laundering: a reply could write its + // invented hook name INSIDE a docs.copilotkit.ai link and satisfy the rule, + // because the rule only checked that the link looked like ours. + it('rejects a link that matches no retrieved source', () => { + expect( + broken(wordy('See https://docs.copilotkit.ai/hooks/useCopilotFabricated'), DOCS), + ).toContain('cites-or-is-a-short-handoff'); + }); + + it('accepts a link that matches a retrieved source', () => { + expect( + broken( + wordy('See https://docs.copilotkit.ai/reference/components/chat/CopilotChat'), + DOCS, + ), + ).not.toContain('cites-or-is-a-short-handoff'); + }); + + it('accepts a retrieved source URL carrying an anchor or query', () => { + expect( + broken( + wordy('See https://docs.copilotkit.ai/reference/components/chat/CopilotChat#slots'), + DOCS, + ), + ).not.toContain('cites-or-is-a-short-handoff'); + }); +}); + +// Pathfinder's plain-text fallback (`textSearch`) sets sourceUrl: undefined on +// every result, so under a flat requirement a CORRECT answer built from it could +// never cite and would always collapse into a handoff. The rule has to know the +// difference between "did not cite" and "had nothing citable". +// Zero retrieval and retrieval-without-URLs are different facts. Treating them +// the same let the doc's Case A — a long, uncited reply built on nothing — publish +// under enforcement, which is the single input where citing matters most. +describe('when retrieval returned nothing at all', () => { + it('still requires a citation, so a long uncited non-answer fails', () => { + const failed = broken( + 'Deep Agents probably supports subagents in some form. '.repeat(8), + [], + ); + expect(failed).toContain('cites-or-is-a-short-handoff'); + }); + + it('still lets a short handoff through', () => { + expect( + broken('Nothing in the docs or source covers this. Routing it to the team.', []), + ).toEqual([]); + }); +}); + +// A bare `includes` accepted anything appended to a retrieved URL, so the +// laundering just moved a level deeper — retrieval routinely returns section and +// index URLs. +describe('citation boundaries', () => { + const SECTION = [ + { + title: 'Reference', + content: 'Use the `CopilotChat` component.', + score: 0.9, + sourceUrl: 'https://docs.copilotkit.ai/reference', + }, + ]; + const wordy = (tail: string) => 'You can configure this in several ways. '.repeat(12) + tail; + + it('rejects an invented path appended to a retrieved URL', () => { + expect( + broken( + wordy('See https://docs.copilotkit.ai/reference/hooks/useCopilotFabricated'), + SECTION, + ), + ).toContain('cites-or-is-a-short-handoff'); + }); + + it('accepts the retrieved URL itself, with an anchor', () => { + expect( + broken(wordy('See https://docs.copilotkit.ai/reference#slots'), SECTION), + ).not.toContain('cites-or-is-a-short-handoff'); + }); + + // Scheme and host are case-insensitive in practice, and both models and + // reporters echo mixed-case hostnames. A case-sensitive compare withheld a + // correctly-cited answer. + it('accepts a mixed-case scheme and host', () => { + expect(broken(wordy('See HTTPS://DOCS.COPILOTKIT.AI/reference'), SECTION)).not.toContain( + 'cites-or-is-a-short-handoff', + ); + }); +}); + +// Naming a live package alone excused the dead one with no requirement that the +// two be related — which waved through Case B's own failure mode, since naming +// both as if both were current IS version-mixing. +describe('the dead-package carve-out needs migration framing', () => { + it('rejects naming both packages as if both were current', () => { + expect( + broken( + 'Install `@copilotkit/react-core` and also add `@copilotkitnext/react` for the newer surface here.', + ), + ).toContain('no-dead-package'); + }); + + it('accepts a genuine migration instruction', () => { + expect( + broken( + "You're importing `@copilotkitnext/react`, which merged into `@copilotkit/react-core` v2 — switch the import.", + ), + ).not.toContain('no-dead-package'); + }); +}); + +describe('when no retrieved source carries a URL', () => { + const URL_LESS = [ + { title: 'CopilotChat', content: 'Use the `CopilotChat` component.', score: 0.9 }, + ]; + + it('marks the citation rule not applicable rather than failing it', () => { + const result = checkReply( + 'You can configure this in several ways. '.repeat(12), + URL_LESS, + ).find((r) => r.rule === 'cites-or-is-a-short-handoff'); + + expect(result?.applicable).toBe(false); + expect(result?.detail).toMatch(/none carries a url/i); + }); + + it('does not count a not-applicable rule as a failure', () => { + expect(broken('You can configure this in several ways. '.repeat(12), URL_LESS)).toEqual([]); + }); + + it('still applies every other rule', () => { + expect(broken('Great question! Call `useCopilotFabricated()`.', URL_LESS)).toEqual( + expect.arrayContaining(['no-banned-phrases', 'grounded-identifiers']), + ); + }); +}); + describe('the handoff cap', () => { it('fails a handoff that pads past the cap', () => { const padded = @@ -206,3 +361,140 @@ describe('no-dead-package', () => { expect(broken('Install `@copilotkit/react-core` first.')).not.toContain('no-dead-package'); }); }); + +// CopilotKit#6927, 2026-09-06. The reply praised the write-up, then spent a +// paragraph announcing what it had not done, then handed the question back. Only +// the praise opener fired against the rule set as it stood: `read the source` +// missed "inspected the source" on the verb alone, so the self-positioning +// paragraph — the part that makes the reply worse than silence — published. +describe('self-commentary the narrower patterns let through (CopilotKit#6927)', () => { + const cited = 'See https://docs.copilotkit.ai/integrations/built-in-agent/mcp-servers.'; + + it.each([ + "I haven't run this code or inspected the source, so I can't confirm the root cause.", + 'I have not inspected the source, so I cannot confirm this.', + "I haven't read the code in question.", + "I didn't review the implementation before answering.", + "I haven't looked at the codebase for this.", + 'I did not examine the code.', + ])('flags %j', (line) => { + expect(broken(`${line} ${cited}`)).toContain('no-banned-phrases'); + }); + + it('flags the self-positioning framing on its own', () => { + expect( + broken( + `To be clear about my position: the header is dropped before the transport. ${cited}`, + ), + ).toContain('no-banned-phrases'); + }); + + // The docblock on BANNED_PHRASES is explicit that a false positive costs a + // reporter a correct answer, because a linter failure collapses the draft into + // a handoff. These are the shapes closest to the patterns that must NOT fire: + // a reporter describing their own testing, and the agent describing the code + // rather than itself. + it.each([ + "I haven't run this on Windows yet, but the repro is attached.", + "You haven't inspected the source here — the header is dropped in SSEClientTransport.", + // These two flagged under an earlier draft that allowed a 60-character gap + // between the verb and its object. Both are the reporter describing their + // own testing, and collapsing either into a handoff costs them an answer. + "I haven't run the repro yet — can you share the code you used?", + "I haven't run into this, but the implementation forwards headers only for stdio.", + "I haven't been able to reproduce it with the code you posted.", + 'The transport does not read the headers option, so nothing reaches the wire.', + 'Run the code in the reproduction and the Authorization header is absent.', + 'To be clear about the behaviour: headers are accepted but never forwarded.', + ])('does not flag %j', (line) => { + expect(broken(`${line} ${cited}`)).not.toContain('no-banned-phrases'); + }); +}); + +// The needle strips its own trailing `/#?`, so a reply reproducing a +// canonicalised URL VERBATIM arrived at the boundary check with `/` next and was +// read as a path continuation — "cited nothing" against a reply quoting the +// retrieved URL exactly. Doc sites canonicalise with a trailing slash, so every +// row below is ordinary traffic rather than an exotic input, and the failure was +// a false positive: the direction that silently argues against ever enforcing. +// +// Every existing fixture URL in this file is bare, which is why this read as +// covered. +describe('trailing slashes on either side of the citation', () => { + // Past HANDOFF_WORD_CAP, or the handoff branch satisfies the rule and the + // citation half is never exercised. + const LONG = + ' The header is dropped before the transport is constructed, so nothing reaches the wire and the server sees an anonymous request instead of an authenticated one.'.repeat( + 3, + ); + const cites = (reply: string, sourceUrl: string): boolean => { + const result = checkReply(reply + LONG, [ + { title: 'Reference', content: 'CopilotChat instructions prop', score: 0.9, sourceUrl }, + ]); + return result.find((r) => r.rule === 'cites-or-is-a-short-handoff')!.passed; + }; + + const BARE = 'https://docs.copilotkit.ai/reference'; + const SLASHED = `${BARE}/`; + + it('counts a reply that reproduces a canonicalised URL exactly', () => { + expect(cites(`See ${SLASHED}`, SLASHED)).toBe(true); + }); + + it('counts a reply that adds a slash the retrieved URL did not have', () => { + expect(cites(`See ${SLASHED}`, BARE)).toBe(true); + }); + + it('counts a markdown link where both sides carry the slash', () => { + expect(cites(`See [docs](${SLASHED})`, SLASHED)).toBe(true); + }); + + it('counts a slashed URL carrying a query', () => { + expect(cites(`See ${SLASHED}?v=2`, SLASHED)).toBe(true); + }); + + // The control, and the reason the boundary check exists at all: stepping over + // ONE slash must not excuse a further path segment. Retrieval routinely + // returns a section URL, and a reply can invent a page beneath it. + it('still refuses a deeper path invented under the retrieved URL', () => { + expect(cites(`See ${BARE}/hooks/useCopilotFabricated`, BARE)).toBe(false); + }); + + it('still counts an anchor on a bare retrieved URL', () => { + expect(cites(`See ${BARE}#slots`, BARE)).toBe(true); + }); +}); + +// `blocksPublish` is exported API. lintDraft filters on `applicable` before +// reading it, but a consumer reading it alone must not be handed `true` for a +// rule that could not be evaluated — that is the draft `applicable` exists to +// protect: Pathfinder's plain-text fallback, where a correct answer has nothing +// it could possibly cite. +describe('blocksPublish is never true for a rule that could not be evaluated', () => { + it('does not block when retrieval returned results but none carries a URL', () => { + const longUncited = 'The header is dropped before the transport is constructed. '.repeat( + 12, + ); + const citation = checkReply(longUncited, [ + { title: 'Reference', content: 'CopilotChat instructions prop', score: 0.9 }, + ]).find((r) => r.rule === 'cites-or-is-a-short-handoff')!; + + expect(citation.applicable).toBe(false); + expect(citation.passed).toBe(false); + expect(citation.blocksPublish).toBe(false); + }); + + // Zero retrieval is a different fact and must still block: a long uncited + // reply built on nothing is the doc's Case A. + it('still blocks a long uncited reply built on zero retrieval', () => { + const longUncited = 'The header is dropped before the transport is constructed. '.repeat( + 12, + ); + const citation = checkReply(longUncited, []).find( + (r) => r.rule === 'cites-or-is-a-short-handoff', + )!; + + expect(citation.applicable).toBe(true); + expect(citation.blocksPublish).toBe(true); + }); +}); diff --git a/packages/outpost/ai/src/eval/rules.ts b/packages/outpost/ai/src/eval/rules.ts index 86f1c50..d980321 100644 --- a/packages/outpost/ai/src/eval/rules.ts +++ b/packages/outpost/ai/src/eval/rules.ts @@ -83,8 +83,30 @@ const BANNED_PHRASES: Array<{ pattern: RegExp; why: string }> = [ // Case D's "What I can't do from here" section, and the rule against the // agent performing its own humility. { pattern: /\bwhat i (?:can'?t|cannot) do\b/i, why: 'self-commentary about its own limits' }, + // Widened from `read the source` after CopilotKit#6927 (2026-09-06), where the + // agent opened with "I haven't run this code or inspected the source". The + // narrower pattern missed it on the verb alone, so the whole self-positioning + // paragraph published. + // + // The verb binds DIRECTLY to its object, with no free gap between them. An + // earlier draft allowed up to 60 characters and flagged two ordinary reporter + // sentences in testing — "I haven't run the repro yet, can you share the code + // you used?" and "I haven't run into this, but the implementation forwards + // headers only for stdio". Both are the reporter talking about their own + // testing, and in the linter a false positive collapses a correct answer into + // a handoff. So the object list is closed and adjacency is required: this + // matches the agent saying it did not look at the code, not someone saying + // they have not run something. { - pattern: /\bi (?:haven'?t|have not) read the source\b/i, + pattern: + /\bi (?:haven'?t|have not|did ?n'?t|did not|do not|don'?t)\s+(?:\w+\s+){0,2}?(?:run|read|inspect(?:ed)?|review(?:ed)?|examine(?:d)?|look(?:ed)? at)\s+(?:this|the)\s+(?:source|code|codebase|implementation)\b/i, + why: 'self-commentary about its own limits', + }, + // Same reply's framing device. The agent announcing its own epistemic + // standing is the thing the doc bans; it is never information the reporter + // asked for, and it reads as hedging a correct answer. + { + pattern: /\bto be clear about my (?:position|limits|limitations)\b/i, why: 'self-commentary about its own limits', }, { @@ -137,9 +159,80 @@ const DEAD_PACKAGE = /@copilotkitnext\b/i; */ const LIVE_PACKAGE = /@copilotkit\/[a-z-]+/i; -/** A link that constitutes a citation: a docs page or a file in the repo. */ -const CITATION_LINK = - /https?:\/\/(?:[a-z0-9-]+\.)*(?:copilotkit\.ai|github\.com\/CopilotKit|github\.com\/ag-ui-protocol)\/\S+/i; +/** + * The migration framing that makes naming the dead package legitimate. + * + * Requiring only that a live `@copilotkit/` package appear somewhere was too + * loose: it excused the dead one with no requirement that the two be related, + * which waved through the exact failure Case B documents. "Install + * `@copilotkit/react-core` and also add `@copilotkitnext/react` for the newer + * surface" passed — and naming both packages as if both were current IS + * version-mixing, so the rule became a no-op on its own worst case. + */ +const MIGRATION_FRAMING = + /\b(?:merged into|replaced by|moved to|superseded by|switch (?:the )?(?:import|to)|instead of|use .{0,20}instead|no longer (?:exists|published|maintained)|is (?:dead|retired|deprecated))\b/i; + +/** + * True when the reply links a source it was actually given. + * + * Checked against the retrieved `sources` rather than against a pattern for + * "looks like one of our URLs". Under a pattern test 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 that either, because `assessGroundedness` blanks URLs before it looks. + * + * Substring rather than equality, so a cited URL may carry an anchor or a query + * the retrieved one did not (`…/CopilotChat#slots`). + */ +function citesARetrievedSource(reply: string, sources: SearchResult[]): boolean { + // Scheme and host lowercased on both sides: they are case-insensitive in + // practice, and models and reporters both echo mixed-case hostnames. A + // case-sensitive compare withheld a correctly-cited answer. + const normalise = (text: string) => + text.replace(/[a-z]+:\/\/[^/\s]+/gi, (m) => m.toLowerCase()); + const haystack = normalise(reply); + + return sources.some((s) => { + if (!s.sourceUrl) return false; + const needle = normalise(s.sourceUrl).replace(/[/#?]+$/, ''); + let from = 0; + for (;;) { + const at = haystack.indexOf(needle, from); + if (at === -1) return false; + // A bare `includes` accepted anything APPENDED to a retrieved URL, so + // the laundering simply moved one level deeper: retrieval routinely + // returns a section or index URL, and + // `…/reference/hooks/useCopilotFabricated` counted as citing + // `…/reference`. The character after the match has to end the URL + // rather than continue its path. + // The needle has its own trailing `/#?` stripped, so a reply that + // reproduces a canonicalised URL VERBATIM lands here with `/` as the + // next character. Treating that as a path continuation made the rule + // report "cited nothing" against a reply quoting the retrieved URL + // exactly — a false positive, in the direction that looks + // conservative, which is the worst kind to leave in a measurement + // this PR exists to collect. Doc sites canonicalise with a trailing + // slash, so it is an ordinary input rather than an exotic one. + // + // Only a FURTHER path segment continues the URL, so step over one + // slash and judge what follows it: `…/reference/` ends, while + // `…/reference/hooks/useCopilotFabricated` still does not cite + // `…/reference`. + let next = haystack[at + needle.length]; + if (next === '/') next = haystack[at + needle.length + 1]; + if ( + next === undefined || + /[\s)\]}.,;"'<>]/.test(next) || + next === '#' || + next === '?' + ) { + return true; + } + from = at + 1; + } + }); +} export const RULES = [ 'says-something', @@ -155,7 +248,37 @@ export type RuleId = (typeof RULES)[number]; export interface RuleResult { rule: RuleId; passed: boolean; - /** Why it failed, naming the offending text. Empty when it passed. */ + /** + * False when the rule could not be evaluated at all, as opposed to evaluated + * and passed. A not-applicable rule is never a failure and is never counted + * in a pass rate. + * + * The case that forced the distinction: Pathfinder's plain-text fallback + * (`textSearch`) sets `sourceUrl: undefined` on every result, so a CORRECT + * answer built from it has nothing it could possibly cite. Under a flat + * requirement that answer fails the citation rule forever and — once these + * rules gate publishing — collapses into a handoff every time the fallback is + * in play. "Did not cite" and "had nothing citable" are different facts. + */ + applicable: boolean; + /** + * Whether this failure should stop the draft publishing, as opposed to being + * worth reporting. + * + * The two are not the same, and collapsing them made the linter stricter than + * the pipeline it sits beside. `grounded-identifiers` is the case that forced + * the split: the doc's success criterion is *zero* invented API names, so one + * occurrence has to show up in a score — but `groundedness.ts` suppresses only + * at SUPPRESS_AT_UNSOURCED_IDENTIFIERS = 2, reasoning that "one could be a + * formatting artifact; two is a pattern of fabrication". Gating a publish at + * one withholds answers production would publish, which is the + * false-withholding direction both modules warn about. + * + * So the harness scores against `passed` and the linter gates on + * `blocksPublish`. For every other rule the two agree. + */ + blocksPublish: boolean; + /** Why it failed, or why it was not applicable. Empty when it passed. */ detail: string; } @@ -174,7 +297,9 @@ function countWords(text: string): number { */ export function checkReply(reply: string, sources: SearchResult[]): RuleResult[] { const words = countWords(reply); - const cites = CITATION_LINK.test(reply); + const cites = citesARetrievedSource(reply, sources); + // Whether citing was possible at all. See RuleResult.applicable. + const anySourceHasUrl = sources.some((s) => !!s.sourceUrl); // A reply that cites nothing is only acceptable as a short handoff, so the // two rules below are the two halves of that single sentence in the doc. @@ -184,10 +309,28 @@ export function checkReply(reply: string, sources: SearchResult[]): RuleResult[] const banned = BANNED_PHRASES.filter(({ pattern }) => pattern.test(reply)); const hedged = HEDGED_NAME_PATTERNS.filter((pattern) => pattern.test(reply)); + // Each rule's verdict, bound once. `blocksPublish` used to restate these + // expressions, twice over for no-dead-package — which is the drift this + // module exists to prevent, reintroduced inside the module itself. + const saysSomething = words >= MIN_REPLY_WORDS; + const citationSatisfied = cites || isShortEnoughForHandoff; + const citationApplicable = !( + sources.length > 0 && + !anySourceHasUrl && + !isShortEnoughForHandoff + ); + const noBannedPhrases = banned.length === 0; + const noHedgedNames = hedged.length === 0; + const deadPackageOk = + !DEAD_PACKAGE.test(reply) || (LIVE_PACKAGE.test(reply) && MIGRATION_FRAMING.test(reply)); + return [ { rule: 'says-something', - passed: words >= MIN_REPLY_WORDS, + // Metric and gate agree for this rule. + blocksPublish: !saysSomething, + applicable: true, + passed: saysSomething, detail: words >= MIN_REPLY_WORDS ? '' @@ -195,9 +338,15 @@ export function checkReply(reply: string, sources: SearchResult[]): RuleResult[] }, { rule: 'grounded-identifiers', + applicable: true, + // The doc's criterion is ZERO invented API names, so one fails the + // metric. The publish gate is the pipeline's own threshold, so one does + // not withhold the answer. See RuleResult.blocksPublish. passed: groundedness.unsourcedIdentifiers.length === 0, + blocksPublish: groundedness.suppress, detail: groundedness.unsourcedIdentifiers.length - ? `names not present in any source: ${groundedness.unsourcedIdentifiers.join(', ')}` + ? `names not present in any source: ${groundedness.unsourcedIdentifiers.join(', ')}` + + (groundedness.suppress ? '' : ' (below the suppression threshold)') : '', }, { @@ -206,29 +355,55 @@ export function checkReply(reply: string, sources: SearchResult[]): RuleResult[] // which presented five independent signals as six and double-counted // every failure in both the per-rule table and the report. rule: 'cites-or-is-a-short-handoff', - passed: cites || isShortEnoughForHandoff, - detail: - cites || isShortEnoughForHandoff - ? '' - : `${words} words with no docs or repo link, over the ${HANDOFF_WORD_CAP}-word handoff cap; a reply this long has to cite what it came from`, + // `applicable &&` is load-bearing, not defensive. When retrieval + // returned results but none carries a URL, `passed` is false because + // nothing WAS cited — but nothing COULD be, so this must not stop a + // publish. lintDraft already filters on `applicable`; computing it + // here means a consumer reading `blocksPublish` on its own cannot + // collapse the very draft `applicable` was added to protect. + blocksPublish: citationApplicable && !citationSatisfied, + // Not-applicable ONLY when retrieval returned results that happen to + // carry no URL — Pathfinder's plain-text fallback, where a correct + // answer has nothing it could cite. + // + // `sources: []` is a different fact and must still fail: a long, + // uncited reply built on zero retrieval is the doc's Case A, the + // exact input where the citation requirement matters most. Treating + // the two the same let that reply publish under enforcement. + applicable: citationApplicable, + passed: citationSatisfied, + detail: !citationApplicable + ? 'not evaluated: retrieval returned results but none carries a URL, so nothing could be cited' + : citationSatisfied + ? '' + : `${words} words and no link to a retrieved source, over the ${HANDOFF_WORD_CAP}-word handoff cap; a reply this long has to cite what it came from`, }, { rule: 'no-banned-phrases', - passed: banned.length === 0, + // Metric and gate agree for this rule. + blocksPublish: !noBannedPhrases, + applicable: true, + passed: noBannedPhrases, detail: banned.map(({ why }) => why).join('; '), }, { rule: 'no-hedged-names', - passed: hedged.length === 0, + // Metric and gate agree for this rule. + blocksPublish: !noHedgedNames, + applicable: true, + passed: noHedgedNames, detail: hedged.length ? 'hedges an API name, which means it is guessing' : '', }, { rule: 'no-dead-package', - passed: !DEAD_PACKAGE.test(reply) || LIVE_PACKAGE.test(reply), - detail: - DEAD_PACKAGE.test(reply) && !LIVE_PACKAGE.test(reply) - ? 'mentions @copilotkitnext without naming the @copilotkit/ package that replaced it' - : '', + applicable: true, + passed: deadPackageOk, + blocksPublish: !deadPackageOk, + detail: !DEAD_PACKAGE.test(reply) + ? '' + : LIVE_PACKAGE.test(reply) + ? 'names @copilotkitnext alongside a live package but not as a migration — reads as if both are current, which is the version-mixing failure' + : 'mentions @copilotkitnext without naming the @copilotkit/ package that replaced it', }, ]; } diff --git a/packages/outpost/ai/src/index.ts b/packages/outpost/ai/src/index.ts index 071cbcd..451385c 100644 --- a/packages/outpost/ai/src/index.ts +++ b/packages/outpost/ai/src/index.ts @@ -47,14 +47,21 @@ export type { TopIssueInput, ScoredTopIssue, } from './front-door.js'; -// The rule set and the scorer are API — Phase 3's linter consumes them. -// HISTORICAL_FAILURES and TARGET_SHAPE deliberately are NOT: they are test data, -// and exporting them put reconstructed bad replies containing -// `useCopilotFabricatedRender` and `@copilotkitnext/react` into dist and the -// worker image, where they would surface in a bundle grep for the dead package. -// Import them from './eval/harness.js' directly in tests and offline runners. +// The rule set, the scorer and the linter are API. HISTORICAL_FAILURES and +// TARGET_SHAPE are not re-exported here because they are test data, not a public +// surface — import them from './eval/harness.js' directly in tests and offline +// runners. +// +// Note what this does NOT do: `tsc` emits per file and `index.ts` imports +// `./eval/harness.js` for `scoreCases`, so `dist/eval/harness.js` still ships +// `HISTORICAL_FAILURES` with its reconstructed replies — a bundle grep for +// `@copilotkitnext` will still hit them. Keeping them out of the build needs the +// fixtures moved outside the compiled graph, which is a separate change; the +// earlier version of this comment claimed a guarantee it did not deliver. export { checkReply, RULES, HANDOFF_WORD_CAP, MIN_REPLY_WORDS } from './eval/rules.js'; export type { RuleId, RuleResult } from './eval/rules.js'; export { scoreCases, formatReport } from './eval/harness.js'; +export { lintDraft, describeVerdict } from './eval/linter.js'; +export type { LintMode, LintVerdict } from './eval/linter.js'; export type { EvalCase, CaseScore, EvalReport } from './eval/harness.js'; export * from './types.js';