fix(ai): widen the groundedness identifier gate past the literal product name - #222
Conversation
…uct name The identifier signal is the only thing that can withhold a response — #143 made claim wording penalty-only on purpose — so its coverage is the whole gate. It required the literal substring `copilotkit`, which exempted every name the model is actually likely to invent: `useCopilotAction`, `CopilotChat`, `CopilotSidebar`, `CopilotTextarea` are all neighbours of real API names and none of them contain the product name in full. extractCopilotKitIdentifiers('Call `useCopilotFabricated()` and `<CopilotInvented />`') before -> [] suppress: false after -> ['useCopilotFabricated', 'CopilotInvented'] suppress: true Answering that with a looser /copilot/i would have been worse than the bug: a false positive here withholds a CORRECT answer from a real reporter, and `copilot` on its own is an English word. So the guard is now a list of named SHAPES rather than a substring test — each one a form a name can only plausibly take when it names our API surface: 1. carries the product name outright (`CopilotKit`, `copilotKitInput`) 2. PascalCase component (`Copilot[A-Z0-9_]…`) 3. hook (`useCopilot[A-Z0-9_]…`) The required second capital is what keeps English out: bare `Copilot`, `copilots` and `copiloting` are prose about the product, not claims about an API that exists. `CSS_CLASS_PATTERN` widens to a `copilot` prefix on the same reasoning — the leading `.` marks a selector, so `.copilot-chat` is ours whatever follows. The early substring pre-filter in identifierSegments is gone rather than ported: the shape rules are anchored and cannot be applied to a still-wrapped token (`<CopilotChat />`), and the per-segment filter already decides the same question correctly after unwrapping. Deliberately still outside the gate: the `CoAgent` / `useCoAgent` family. No shape rule separates those from generic React vocabulary without a hand-maintained name list, and a stale allowlist fails in the direction that withholds correct answers. Pinned in both directions — 9 new tests, and the two mutations that matter were each observed RED: - reverting to /copilotkit/i -> 6 tests fail - loosening to the naive /copilot/i -> the English-word guard fails Closes #147
CPK-7928 1. #147 — widen the identifier guard (UNBLOCKED, start here)
outpost#147. No open PR touches The defect
if (!/copilotkit/i.test(token)) return [];and Why it is load-bearingAfter #143 the identifier signal is the sole withholding mechanism; claim wording is penalty-only by design. A gate that covers a fraction of the surface is closer to no gate than to a working one, and it currently reads as shipped. Care neededThe narrow scoping was deliberate — the docstring says so — to keep generic React vocabulary ( So this wants a deliberate answer to "what is a CopilotKit identifier", not a looser regex. The existing test suite is the guard: reverting the previous widening ( CPK-7980 Review + land PR #222 — widen the groundedness identifier gate (5 findings open)
CopilotKit/outpost#222 by jerelvelarde, opened 2026-08-19 04:36Z off Branch is Test-heavy ratio (102 new test lines to 66 source), which is the right shape for this change. Why this one deserves careful review rather than a skimThis is the highest-stakes response-quality change open. After #143, the identifier signal is the only thing that withholds an ungrounded answer — claim wording is penalty-only by design, because parsing English to make a publish/withhold decision failed three times in that PR. So this gate is load-bearing in both directions, and the danger is asymmetric:
The specific thing to check: does the new matcher pull in generic React vocabulary ( Worth confirming the 102 new test lines cover both directions — invented cr-loop complete 2026-08-19 — 5 mandatory findings, do not merge as-isTier 1 (predicates recorded: 179 LOC, single module, no shared mechanism, reversible). Panel of 3 — code-reviewer + silent-failure-hunter + pr-test-analyzer. Round 1 complete, 3/3. Convergence not declared; no fix cycle run (review-only, and this is Jerel's branch). Review comment posted. The concern I raised above turned out to be real, and all three agents found it independently. Mandatory set:
Open question for Jerel, not a finding: at Ledger + full reports: |
|
Hey @jerelvelarde, Replacing my earlier comment on this PR with a consolidated one. Same reason as #224: I re-ran the review with mutation testing enforced, and it moved things around, including proving that the fix I suggested last time doesn't work. Everything below is executed against the real module unless marked otherwise. Verdict: NEEDS CHANGES. One blocker. Everything else is non-blocking. ApprovedThe shape allowlist is the right design, and the three rules at Deleting the old Leaving Blocker
|
| Response text | Head | Base |
|---|---|---|
Your handler reads `state.copilotOpen` and toggles `ui.copilotWidth`, neither is a CopilotKit API. |
ids ["copilotOpen","copilotWidth"], suppress = true |
ids [], suppress = false |
Set `github.copilot.enable` to false, and check `settings.copilotInline` too. |
ids ["copilot","copilotInline"] |
ids [] |
a sentence.Copilot starts here |
ids ["Copilot"] |
ids [] |
So two echoed member accesses from a reporter's own code are enough to hit SUPPRESS_AT_UNSOURCED_IDENTIFIERS at :265 on their own, and a correct answer gets withheld from a real person with no error anywhere. That third row is Copilot bare, which groundedness.test.ts:109 asserts must never be an identifier. Right now `Copilot` is correctly ignored while .Copilot is a fabrication claim.
Correcting myself: I previously said to route the CSS capture through isCopilotKitIdentifier. I ran that, and it fails groundedness.test.ts:100, because copilotSidebarPanel and copilotOpen are shape-identical. That's the proof that shape can't rescue this pattern. Position can:
const CSS_CLASS_PATTERN = /(?<![\w$)\]])\.(copilot[A-Za-z0-9_-]*)/gi;Applied, 81/81 still pass and both false positives above clear. Note the flip side: they pass either way, so the suite can't currently tell the broken pattern from the fixed one. A regression test for state.copilotOpen and github.copilot.enable needs to land with the fix.
Non-blocking
:240— the new PascalCase rule is mostly inert in practice.JSX_WRAPPERonly unwraps prop-less JSX, soWrap it in `<CopilotFabricated debug={true} />` and `<CopilotInvented labels={{}} />`.yieldsids = []and publishes. A model almost always writes a component with props, so the fabricated-component half of Groundedness identifier signal exempts every name that isn't literally CopilotKit* (useCopilotAction, CopilotChat, …) #147 still gets through in its most common spelling. The regex predates the PR, but component coverage is the headline claim and the new tests only use the bare form (test.ts:82,:94). Allowing trailing attribute text (^<\/?\s*([A-Za-z_$][\w$.-]*)(?:\s[^<>]*)?\s*\/?>$) plus a props-bearing test would close it.:211-224— common spellings still escape. Verifiedids = []for: undotted kebab (copilot-ghost-panel, while the PR's kebab test attest.ts:100only covers the dotted form), camelCase invented names (copilotFabricate(),copilotInvented), and lowercase-after-prefix typos (useCopilotfabricated(),<Copilotinvented />). There's also an inconsistency worth a doc line:Copilot_Chatis an identifier because_is in the class, butcopilot_chatisn't, andCOPILOT_API_KEYisn't whileCOPILOTKIT_TOKENis.:220,:223— the^anchors aren't pinned.:328-333asserts they're load-bearing, but dropping both anchors passes 81/81. Something likegetCopilotXValue/myUseCopilotHookreturning[]would fix that.test.ts:769— the "English word 'copilot'" corpus row is non-discriminating. Its response has no backticks and no dots, so it never reaches either extraction path. Confirmed: replacing all shapes with a naive/copilot/i, and separately dropping the required second capital, each fail only the unit test attest.ts:109and never this row. Either give it a backticked`Copilot`or a.Copilot, or drop it, because as written it reads as protection it isn't providing.:464— the trade this PR makes, with the number visible. The widened gate can't distinguish a real API name from an invented one, so its false-positive rate is now bounded entirely by retrieval quality. Measured:Register it with `useCopilotAction()` and mount `<CopilotSidebar />`.against a retrievedCopilotChatpage that doesn't literally contain those strings gives suppress = true at head, false at base. Which means the five-ish names correct answers cite most are the ones most likely to be withheld. TheAPI_DOCScorpus row attest.ts:751only pins the case where the docs do contain them, so this direction is unpinned. I'm not asserting this must change, since you argued against an allowlist and that argument has merit. But those names aren't stale-prone, and an allowlist fails toward publishing, so it's worth deciding deliberately rather than by default.- Substring grounding at
~:450is what saved thegithub.copilot.enablerow above from suppressing: barecopilotis grounded by any source mentioning CopilotKit. Deliberate and documented, but the widening leans on it a lot harder now.
Still open from my earlier read, not re-verified this round
These came from the previous pass and the fresh review didn't cover them, so treat them as unconfirmed rather than dropped. Flagging honestly instead of quietly deleting them:
:244/:345—IDENTIFIER_PATHaccepts a leading#but the split strips only., so`#copilotKitPanel`may be recorded with the#attached and dedup separately from its dotted form, turning one invented name into two entries and hitting the threshold alone. Same root cause would mean ID selectors can never ground.test.ts:802-812— the "claim wording never withholds" assertion looks wrapped in anif, which would make it pass vacuously the moment extraction starts inventing identifiers, i.e. exactly the regression it exists to catch.test.ts:424— name claims throw coverage and empty sources, but the call passesCHAT_DOCSand asserts neither.:159-160—BARE_HOST_PATTERN's TLD list includesapp|dev|io|co|sh, so`copilotKitBridge.io`would be erased as a hostname before extraction sees it.:388-395— the "never throws" docblock, where a nullsourcesor null array element would throw a TypeError inside the publish/withhold decision.:477-479— the hedge reason quoting totalhedgeCountwhile onlyexcessHedgeswas charged.:193— backticked content over 80 chars silently skipped, undocumented and untested.
And the open question from last time still stands: at :441-448, s.sourceUrl is in the grounding haystack, so any identifier that's a substring of docs.copilotkit.ai grounds automatically. Deliberate or a laundering hole? If deliberate, a comment there stops the next reader re-flagging it.
Evidence
vitest 4.1.4 from the lockfile, vite 8.0.8, node v20.19.0, typescript 5.9.3, pnpm 10.33.4. pnpm install --frozen-lockfile clean in 6s.
Head 04a32c2 at 81 passed, base 724a53b at 72 passed, so +9 and nothing moved from pass to fail. One test file fails identically at both commits, @prisma/client throwing on import because --frozen-lockfile skips its postinstall, which is what makes that "1 failed" line interpretable as zero regressions. Prettier clean on both changed files. Vitest has to be rooted at packages/outpost; the repo-root config's include is scripts/__tests__/** and finds nothing.
Not verified: real-world frequency of the false-positive inputs above. I've shown they suppress, I have no corpus telling us how often a generated answer contains two copilotX member accesses or two real API names absent from the retrieved page. Also didn't exercise the pipeline end to end, so the severity of the blocker rests on the documented suppress contract at the three call sites rather than on observed behavior.
Happy to take the blocker as a patch, it's a one-line regex plus the regression test.
The widened `.copilot*` capture pushes straight into `hits` without passing
through `isCopilotKitIdentifier`, so a leading `.` was the entire guard. In TS
and in prose a `.` marks member access far more often than a class, which made
a reporter's own `state.copilotOpen` and `ui.copilotWidth` two unsourced
identifiers — the suppression bar on its own. A correct answer withheld from a
real person, with nothing logged.
Requiring the `.` not to follow an identifier character, `)` or `]` separates
the two. Shape cannot: `copilotSidebarPanel` (ours) and `copilotOpen` (someone's
local state) are shape-identical, so routing the capture through the shape gate
fails the legitimate case instead. Position is the only thing that tells them
apart. Cost is `div.copilotPanel`, a tag-qualified selector, now missed — rarer
than member access, and it fails toward publishing.
Also unwrap JSX that carries props. `JSX_WRAPPER` only matched the prop-less
form, so `<CopilotFabricated debug={true} />` yielded nothing — the
fabricated-component half of #147 escaping in its commonest spelling, since a
model almost always gives an invented component props.
Tests: the suite could not previously tell the broken selector rule from the
fixed one. Now dropping the lookbehind fails 2, reverting the JSX change fails
1, dropping the `^` anchors fails 1, and a naive `/copilot/i` fails 5 where it
used to fail 1 — the "English word" corpus row had no backticks and no dot, so
it reached neither extraction path and passed whatever the rules said.
86 tests in the file, 1025 in the package, all passing.
|
Blocker fixed in The blockerYou're right that the leading Took your pattern as-is: const CSS_CLASS_PATTERN = /(?<![\w$)\]])\.(copilot[A-Za-z0-9_-]*)/gi;And your correction is the useful part of the finding, not a footnote — Covered in both directions:
Mutation: dropping the lookbehind now fails 2. It failed 0 before, which was your point. Also fixed: the inert PascalCase ruleAgreed, and this one bothered me more than the blocker — component coverage is the headline of #147 and Also fixed: the two test-quality findings
86 in the file, 1025 in the package, prettier clean. Not taking, with reasonsThe allowlist question at Undotted kebab, camelCase invented names, lowercase-after-prefix typos, and the The carried-forward items (
Re-review when you have a moment. |
|
Hey @jerelvelarde, Verified
Every member-access input returns Your stated cost is real but narrower than the comment implies: Also checked the thing I'd have worried about in the widened JSX regex: over-capture is impossible by construction. One thing worth folding in, your callThe
Both lines are byte-identical at And I was wrong about two testsI'd flagged
There is a real weakness there though, worth a line when you're next in the file ( |
NathanTarbert
left a comment
NathanTarbert
left a comment
There was a problem hiding this comment.
Verified aca57a3 — blocker fixed, all four mutation claims reproduce exactly, nothing legitimate regressed. Details in the comment above. The #-prefix path is pre-existing rather than a regression here, so it's filed as #234 rather than held against this PR.
2 participants
Closes #147.
The identifier signal is the only thing that can withhold a response — #143 made claim wording penalty-only on purpose — so its coverage is the gate. It required the literal substring
copilotkit, which exempted every name the model is actually likely to invent.useCopilotAction,CopilotChat,CopilotSidebar,CopilotTextareaare all neighbours of real API names, and none of them contain the product name in full. A model inventing a neighbour of a real hook is a far more natural failure than inventing a.copilotKit*CSS class — so the gate was blind to the likelier fabrication while reading as shipped.Why not just loosen the regex
The issue asks for "a deliberate answer to what is a CopilotKit identifier, not a looser regex", and that framing is right.
/copilot/iwould have been worse than the bug: a false positive here withholds a correct answer from a real reporter, andcopiloton its own is an English word we and our users both use in prose.So the guard is now a list of named shapes, each one a form a name can only plausibly take when it names our API surface:
/copilotkit/iCopilotKit,copilotKitInput,CopilotKitProvider/^Copilot[A-Z0-9_]/CopilotChat,CopilotSidebar,CopilotRuntime/^useCopilot[A-Z0-9_]/useCopilotAction,useCopilotReadableThe required second capital is the load-bearing detail. It is what keeps English out: bare
Copilot,copilotsandcopilotingare prose about the product, not claims about an API that exists.CSS_CLASS_PATTERNwidens fromcopilotkitto acopilotprefix on separate reasoning — the leading.marks a selector rather than a word, so.copilot-chatis ours whatever follows.The early substring pre-filter in
identifierSegmentsis removed rather than ported. The shape rules are anchored, so they cannot be applied to a still-wrapped token (<CopilotChat />), and the per-segment filter at the bottom already decides the same question correctly after unwrapping.IDENTIFIER_PATHstill rejects prose, so dropping it costs a little work on non-identifier tokens and buys the gate every name it used to exempt.Deliberately still outside the gate
The
CoAgent/useCoAgentfamily. Those are ours too, but no shape rule separates them from generic React vocabulary without a hand-maintained name list — and a stale allowlist fails in the direction that withholds correct answers. Documented in the code rather than left as an omission someone rediscovers.Verification
9 new tests, 81 passing in
groundedness.test.ts(up from 72). The corpus gets rows in both directions, since the widening is only half-pinned if it is tested solely where it suppresses:suppress: trueuseCopilotAction/CopilotChatagainst sources that contain them →suppress: false,unsourcedIdentifiers: []suppress: truecopilotprose → nothing extractedBoth mutations that matter were observed RED and restored to GREEN:
/copilotkit/i(the bug)/copilot/iThat second one is the important one — it means the false-positive edge is a real test and not decoration.
Full
packages/outpostsuite: 61 test files, 1020 tests passing, zero failures — against 1011 onmain, so exactly the 9 new tests and nothing disturbed.Not addressed here
Issue #147 lists two narrower related gaps that I deliberately left alone, because each needs its own decision rather than riding along:
BARE_HOST_PATTERNstrips scheme-less hosts against a 9-TLD list, so a citation on.cloudor.helpstill leaks a phantom identifier — while widening the list makes it likelier that a genuine identifier ending in a TLD-like segment gets blanked as a host. That is a real two-sided tradeoff, not an oversight.Happy to take either as a follow-up.