Skip to content

fix(ai): score responses on groundedness and withhold ungrounded ones - #143

Merged
NathanTarbert merged 10 commits into
mainfrom
fix/ai-groundedness-gate
Jul 31, 2026
Merged

fix(ai): score responses on groundedness and withhold ungrounded ones#143
NathanTarbert merged 10 commits into
mainfrom
fix/ai-groundedness-gate

Conversation

@NathanTarbert

@NathanTarbert NathanTarbert commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #142 — base is fix/ai-response-epistemic-guardrails, merge that one first.

What this fixes

Confidence scoring measured how good the docs match was and never looked at whether the answer stayed inside it.

assessConfidence didn't read the response at all — the parameter was _response, unused — and the LLM scorer's rubric actively rewarded specificity: "cites specific features, APIs, or code patterns," with no check that the citations were real. So a confident fabrication scored exactly as high as a cited answer, and #142's prompt rules had nothing enforcing them.

New ai/groundedness.ts does the enforcing. No model call — regexes over the response plus a substring lookup against the sources it was generated from, so it's cheap, deterministic and unit-testable.

  • Unverified claims — "bug confirmed", "root cause is", "known issue", "the fix is", "I reproduced/tested this". Any one of these suppresses the response.
  • Unsourced identifiers — CopilotKit names the response invents. One is a penalty, two suppress. Scoped deliberately narrow to CSS classes and backticked tokens carrying our own name, so useRef, useLayoutEffect and @copilotkit/react-core never trip it.
  • Hedge density — two markers free, then a small penalty each. Never suppresses on its own; hedging is a smell, not a fabrication.

Wired into four places:

  • generator.assessConfidence reads the response and deducts the penalty
  • pipeline.ts applies the penalty after feedback calibration, so aggregate 👍 can't buy back a fabrication, and clamps a suppressed response below the escalation gate
  • confidence.ts rubric now weighs groundedness above specificity
  • ai-response.ts withholds a suppressed response and escalates

The behavior change worth reviewing closely

Confidence never gated the post-back. A low score only picked the disclaimer and queued an escalation job — the response posted either way (ai-response.ts:203). So a penalty on its own would have lowered a number and changed nothing about what reached a public thread.

Now a suppressed response never posts as written — the reporter gets the safe replacement copy — and escalation fires regardless of score.

Correction (was wrong in an earlier version of this description): the draft persists as the BOT Message only. suggestedResponse holds formatted.text, which for a suppressed draft is the replacement copy, not the draft — so an agent looking at the ticket's suggestion field sees boilerplate and has to find the draft in the message thread. Message.content (raw draft) and the posted text also diverge under suppression, so anyone auditing bot output from the DB reads the draft as though it were published. Both are tracked in #148, which now also covers stashing the published text alongside it.

That means some tickets get no immediate bot reply where they used to get one. That's the intended trade — silence beats a confident fabrication on a public issue — but it is a real change in what reporters see, so flagging it rather than burying it.

One thing a test caught

The penalty cap alone wasn't sufficient. Top retrieval score plus maximum positive calibration minus the capped penalty lands on exactly 0.4, and the escalation gate is < 0.4 — so the worst-case response would have silently declined to escalate. Hence the explicit clamp to just below the gate, which also keeps the dashboard, the disclaimer and the escalation logic agreeing with each other.

Tests

  • groundedness.test.ts — 23 cases: each claim pattern, sourced vs unsourced identifiers, title-as-well-as-body matching, the false-positive guards, penalty cap, empty/partial input, and the verbatim #6167 response as a fixture asserting it gets suppressed
  • pipeline.test.ts — penalty deducted from the final score, suppression flagged, and calibration ordering (a boost can't rescue a fabrication)
  • ai-response.test.ts — a suppressed response never posts, escalates even with a 0.95 score, and still leaves the draft on the ticket

npm test 840 passing across all 10 packages · npm run typecheck clean · npm run build clean. Lint and prettier are unchanged from #142 — both were already failing on clean main.

Confidence scoring measured retrieval quality and ignored whether the answer
stayed inside what was retrieved. assessConfidence never read the response at
all (the parameter was literally _response, unused), and the LLM scorer's rubric
rewarded specificity - "cites specific features, APIs, or code patterns" -
without asking whether the citations were real. A confident fabrication
therefore scored exactly as high as a cited answer.

New ai/groundedness.ts runs no model call: regexes over the response plus a
substring lookup against the sources it was generated from.

- Unverified claims ("bug confirmed", "root cause is", "the fix is", claims of
  having reproduced or tested) - any one suppresses the response
- CopilotKit identifiers absent from every source - one penalizes, two suppress.
  Scoped to CSS classes and backticked tokens carrying our own name, so generic
  React vocabulary and @CopilotKit package specifiers never trip it
- Hedge density beyond a two-marker allowance - penalizes, never suppresses

Wiring:

- generator.assessConfidence now reads the response and deducts the penalty
- pipeline applies the penalty AFTER feedback calibration, so aggregate thumbs-up
  can't buy back a fabrication, and clamps a suppressed response below the
  escalation gate
- confidence.ts rubric weighs groundedness above specificity
- ai-response.ts withholds a suppressed response from the platform and escalates

That last one is the point. Confidence never gated the post-back - a low score
only picked the disclaimer and queued an escalation - so lowering a number would
not have kept a fabrication out of a public thread. A suppressed response now
never posts, still persists as suggestedResponse for a human to edit, and
escalates regardless of score.

The clamp exists because a test found the penalty cap insufficient on its own: a
top retrieval score plus maximum positive calibration minus the capped penalty
lands on exactly 0.4, and the escalation gate is < 0.4, so it would have
silently declined to escalate.

Tests: 23 cases in groundedness.test.ts including the verbatim #6167 response as
a fixture, 4 pipeline cases covering penalty/suppression/calibration ordering,
and 3 handler cases proving no post, escalation regardless of score, and the
draft surviving for a human.

@jerelvelarde jerelvelarde left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The design here is right, and the part I want to call out first is that you found the real gap: assessConfidence scoring retrieval quality while ignoring the answer meant the guardrails in #142 had nothing behind them. A deterministic regex-plus-substring check with no model call is the correct shape for enforcement — cheap, ordered predictably, and actually testable. Deducting after calibration so aggregate 👍 can't buy back a fabrication is a genuinely subtle call and the comment explaining it is the kind of thing that stops someone reordering it in six months. Using the verbatim #6167 response as a fixture is the right way to pin a regression. And flagging the "some tickets now get no immediate reply" behavior change in the description rather than burying it is exactly right.

One correctness bug that I think needs fixing before this lands, then some smaller things.

The groundedness penalty is applied twice

assessGroundedness is called at two sites and both subtract the penalty:

  1. generator.ts:246assessConfidence now ends with return Math.max(0, retrievalScore - penalty), so the penalty is baked into generatedResponse.confidenceScore.
  2. pipeline.ts:141finalConfidenceScore = Math.max(0, finalConfidenceScore - groundedness.penalty), subtracting the same value again.

In between, pipeline.ts:123 does combinedConfidenceScore = Math.min(generatedResponse.confidenceScore, confidenceAssessment.score). Whenever the generator's (already-penalized) score is the lower of the two — which a large penalty makes likely, since it's the thing pushing it down — the penalty is counted twice. When the LLM scorer happens to be lower, it's counted once. So the deduction is both doubled and path-dependent on which signal wins the Math.min.

Worked example on the single-identifier case, penalty 0.15, avg relevance 0.80 over 3 sources:

retrievalScore   = 0.80 + 0.15 bonus      = 0.95
generator score  = 0.95 − 0.15            = 0.80   ← penalty #1
combined         = min(0.80, 0.85)        = 0.80
calibration 0                             = 0.80
pipeline         = 0.80 − 0.15            = 0.65   ← penalty #2

Intended 0.80, actual 0.65. One invented identifier costs 0.30, not the 0.15 the constant declares.

The suppression clamp hides this for anything that trips suppress, so the visible damage is confined to the deliberately-designed middle tier — "one is a penalty, two suppress." That's the band where a single invented identifier can now get pushed under AI_CONFIDENCE.ESCALATE and fire a spurious escalation on a response that was meant to post with a disclaimer.

I'm fairly confident this is unintended rather than deliberate, because the description's own arithmetic assumes single application:

Top retrieval score plus maximum positive calibration minus the capped penalty lands on exactly 0.4, and the escalation gate is < 0.4

That only reaches 0.4 if the penalty is subtracted once (1.0 − 0.6). Applied twice it lands at 0, comfortably below the gate, and the explicit clamp you added wouldn't have been necessary at all.

Why the tests didn't catch it

pipeline.test.ts:43 mocks the whole ResponseGenerator class, and sampleGeneratedResponse.confidenceScore is hardcoded to 0.85. So assessConfidence — penalty site #1 — never executes in any pipeline test. That's what makes the 0.85 − 0.15 = 0.70 comment on the new test read as correct: it is correct against the mock, and unreachable in production. Any regression test for this has to exercise a real ResponseGenerator, or assert at the generator level directly.

Suggested fix

Keep the ordering guarantee you documented — the deduction must land after calibration, otherwise a positive boost can partially offset it — and remove the duplicate:

  • assessConfidence goes back to returning the raw retrievalScore and does not deduct.
  • The generator computes the assessment once and carries it on GeneratedResponse (e.g. a groundedness field), so pipeline consumes it instead of recomputing.
  • pipeline remains the single place that deducts, still after calibration.

That drops the penalty to one application, keeps the post-calibration invariant, and removes the redundant second assessGroundedness call per request as a side effect.

CI has never run the test suite on this PR

.github/workflows/ci.yml triggers on pull_request: branches: [main, staging]. This PR's base is fix/ai-response-epistemic-guardrails, so Lint, Typecheck & Test never fired — only zizmor and notify ran, and GitHub still reports mergeStateStatus: CLEAN. Worth knowing that the green tick here means less than usual. Once #142 is in, retarget this to main and let the full suite run before merging.

Smaller things

Suppression regexes are negation-blind. /\bknown\s+(?:bug|issue|regression)\b/i matches "this is not a known issue"; /\broot\s+cause\s*(?:is\b|:)/i matches "I can't determine what the root cause is without reproducing it"; /\bthe\s+fix\s+is\b/i matches "I don't know what the fix is." Those are all well-behaved responses of exactly the kind #142's prompt asks for, and each now yields silence for the reporter. Since suppression is user-visible absence rather than a lowered number, a negation lookbehind or tightening to assertive forms seems worth it.

The human draft is pre-poisoned with the AI disclaimer. ai-response.ts:172 sets suggestedResponse: pipelineResult.formatted.text, which for a suppressed response carries > ⚠️ This is an AI-generated response. We've escalated this to our engineering team…. reply-editor.tsx:59 prefills the agent's reply box with that string verbatim. So the "a human can edit and send it" path hands them a draft opening by declaring itself AI-generated and announcing an escalation. pipelineResult.response (already persisted raw as Message.content on line 158) is the better source for the editor. Pre-existing, but this PR promotes it from cosmetic to the primary path for suppressed responses. Happy to split it out if you'd rather keep this PR tight.

CSS_CLASS_PATTERN is case-sensitive. /\.(copilotKit[A-Za-z0-9_-]*)/g won't match an invented .copilotkit-input or .CopilotKitInput. Adding i costs nothing given the /copilotkit/i guard already downstream.

Substring matching gives false negatives. haystack.includes(id.toLowerCase()) means an invented copilotKitTextarea counts as grounded if any source happens to mention copilotKitTextareaWrapper. Fine for a cheap heuristic and it errs toward not suppressing, which is the safe direction — just worth a comment so the next reader doesn't assume it's exact.

I checked the obvious escape hatch and it's closed: nothing auto-posts suggestedResponsereply-editor.tsx is its only consumer and it's human-driven — so the suppression gate can't be bypassed that way.

Requesting changes on the double-penalty. Everything else is take-it-or-leave-it, and this is close.

@jerelvelarde
jerelvelarde changed the base branch from fix/ai-response-epistemic-guardrails to main July 28, 2026 11:03
@jerelvelarde

Copy link
Copy Markdown
Collaborator

#142 is merged, so I've retargeted this to main — the diff is unchanged (+644 across 10 files), which confirms it's cleanly just this PR's own work.

Worth noting why that mattered beyond housekeeping: while the base was fix/ai-response-epistemic-guardrails, ci.yml's pull_request: branches: [main, staging] trigger meant the Lint, Typecheck & Test job never ran on this branch — only zizmor and notify did, and GitHub still reported the PR as CLEAN. So the green tick was covering two jobs, neither of which runs a test. Now that it's on main the full suite should fire; if it doesn't pick up automatically from the base change alone, a no-op push will do it.

The double-penalty item from my review stands regardless — and note the suite as it exists wouldn't catch it even when it runs, since pipeline.test.ts:43 mocks the whole ResponseGenerator, so assessConfidence (the first of the two deduction sites) never executes in any pipeline test.

…gated claims

Review fixes for #143.

The penalty was deducted at two sites: generator.assessConfidence baked it into
GeneratedResponse.confidenceScore, and the pipeline subtracted the same value
again from the min() of the generator and scorer scores. Whenever the generator's
already-penalized score was the lower of the two, it landed twice.

The generator now assesses groundedness (it has the response and its sources in
hand) and passes the result through on GeneratedResponse.groundedness without
touching its own score, which goes back to measuring retrieval quality only. The
pipeline is the single place that deducts, still after calibration. That also
drops the redundant second assessGroundedness call per request.

No existing test could have caught this: pipeline.test.ts injects a mocked
ResponseGenerator, so assessConfidence never executes in any pipeline test. New
pipeline-groundedness.test.ts runs the real generator against aimock and pins the
arithmetic. Verified it fails on the pre-fix code (0.675 where 0.80 is expected)
and passes after.

Also from review:

- Claim patterns were negation-blind. "This is not a known issue", "I can't
  determine what the root cause is", "I don't know what the fix is" all suppressed
  the response, and those are exactly the answers the prompt asks for. Claims are
  now checked per-occurrence against a same-sentence negation lookbehind, so a
  negated mention no longer excuses an assertive one elsewhere in the response.
- CSS_CLASS_PATTERN is case-insensitive, so an invented `.copilotkit-input` or
  `.CopilotKitInput` is caught alongside `.copilotKitInput`.
- Documented that identifier matching is substring rather than exact-token, and
  why the resulting false negatives are the safe direction.

The suggestedResponse prefill item is filed separately — it is pre-existing, is
not in this PR's diff, and its severity depends on a change that isn't here yet.
@NathanTarbert

Copy link
Copy Markdown
Collaborator Author

Pushed 28fb1fb addressing the review. Thanks for the double-penalty catch — that was a real bug and your read of it was exact.

Double penalty. Fixed as you described: assessConfidence goes back to scoring retrieval quality only, and the generator now passes the assessment through on GeneratedResponse.groundedness for the pipeline to apply once, after calibration. The second assessGroundedness call per request is gone with it.

Your note that the suite couldn't catch this was the more useful half — pipeline.test.ts mocking the whole generator means assessConfidence never executes in any pipeline test, so no assertion I could add there would have failed. New pipeline-groundedness.test.ts runs the real generator against aimock and pins the arithmetic. I confirmed it fails on the pre-fix code (0.675 where 0.80 is expected) and passes after, so it's a genuine regression guard rather than a test written to agree with the new behavior.

Negation-blind regexes. Taken. Claims are now checked per-occurrence against a same-sentence negation lookbehind ([^.!?]{0,80}$-anchored, so "That's confirmed. I have not tested it." still counts). Checking every occurrence rather than the first means a negated mention doesn't excuse an assertive one later in the same response — both directions have tests. Your framing that suppression is user-visible absence rather than a lowered number is what made this worth fixing properly.

Case-sensitive CSS pattern. Fixed, with tests for .copilotkit-input and .CopilotKitInput.

Substring matching. Left as-is, comment added explaining that it errs toward not suppressing and that that's the safe direction.

suggestedResponse prefill. Filed as #145 rather than folded in here, on your offer to split it. Two reasons: the line isn't in this PR's diff, and its severity depends on work that isn't here yet. Under the in-progress always-answers change, a suppressed draft's formatted.text is replaced with the no-answer copy — so the human's editor would prefill with the apology while the draft they need sits only in Message.content. That turns your cosmetic nit into the thing that breaks "a human can edit and send it," so it should land with that PR, where the reasoning is legible.

On CI: retargeting to main did it — the full suite is running on this push. Locally: 854 tests green across packages/outpost, 10/10 packages, typecheck and build clean.

The claim/negation core matched regexes against the raw response and then
looked backwards over a fixed character window. That failed in both
directions: it missed trailing negations, let a negation on one markdown
bullet cancel an assertion on the next, could slice `cannot` into a bare
`not`, and let hedges ("possibly the root cause is X") pose as negations.
Alongside it, URLs were mined for identifiers, `sourceUrl` was never
searched for grounding, dedup was case-sensitive while comparison was
case-insensitive, and the penalty was charged per matched pattern rather
than per claim.

Replaced with normalize (strip URLs, neutralize "no doubt"-style
intensifiers) → split into sentences on [.!?] and newlines → judge each
sentence independently, checking for a genuine negator in both
directions. Fenced code blocks are still in scope: #6167 put its invented
class names in a ```css fence.

Fixes, in order: trailing negation; hedges no longer negate; newline is a
sentence boundary; NEGATION_WINDOW deleted rather than reconciled; URLs
are never a source of identifiers; sourceUrl joins the grounding
haystack; identifier dedup is case-folded (first spelling reported);
claims are charged once per sentence per category, and the doc comment
now matches. Also added `unknown`/`undetermined` as negators and made a
`.` between digits a non-boundary so `v1.2.3` cannot strand a negation.

Tests: new table-driven corpus organized by RESPONSE SHAPE, one row per
shape with explicit expected suppression. 14 of its assertions failed
against the old implementation and pass now. Fixed the vacuous
'picks up backticked CopilotKit identifiers' row, which asserted [] for a
token containing no "copilotkit" and so passed via the wrong guard.

Call-site enumeration (grep -rn across packages/ and apps/, excluding
node_modules and dist):

- assessGroundedness — unchanged signature and return shape. Callers:
  ai/src/pipeline.ts:152 (penalty subtracted, suppress gates the post),
  ai/src/generator.ts:135,163 (assessment rides along on
  GeneratedResponse), ai/src/index.ts:6 (re-export),
  ai/src/pipeline-groundedness.test.ts:119. All assumptions hold: same
  arity, same field names, penalty still bounded by
  MAX_GROUNDEDNESS_PENALTY, suppress still a boolean gate. Only the
  values change, and the pipeline/queue tests that pin those values
  (0.15 for one invented identifier, suppression on two) still pass.
- extractCopilotKitIdentifiers — unchanged signature. Only call site
  outside its own tests is the re-export at ai/src/index.ts:7. Return is
  still string[] of bare identifiers; case-variant duplicates collapse
  and URL text no longer contributes, both strictly fewer entries.
- MAX_GROUNDEDNESS_PENALTY — untouched value 0.6. Re-exported at
  ai/src/index.ts:8; assertions in groundedness.test.ts still hold.
- GroundednessAssessment — all six fields unchanged. Consumed by
  ai/src/types.ts:9,66,222 and read in queue/src/handlers/ai-response.ts
  (reasons, suppress) — both still populated the same way.
- SUPPRESS_AT_UNSOURCED_IDENTIFIERS — newly exported (was private, same
  value 2) and added to ai/src/index.ts so tests pin the bar by name. No
  pre-existing references to break.
- NEGATION_LOOKBEHIND, NEGATION_WINDOW, isNegated — removed. grep -rn
  across packages/ and apps/ returns zero references anywhere.
- New module-private helpers (stripUrls, splitSentences) and constants
  (NEGATOR_PATTERN, INTENSIFIER_PATTERN, INTENSIFIER_REPLACEMENT,
  URL_PATTERN, URL_PLACEHOLDER, SENTENCE_BOUNDARY, ClaimCategory) — not
  exported; grep confirms zero references outside groundedness.ts.

Verified: vitest run --root packages/outpost → 874 passing across 56
files; tsc --noEmit for the ai project and the full package typecheck
both clean.
Withholding a reply was driven by regex-parsed English: claim phrases plus a
negation detector. That decision has now failed three times in three different
ways. The character-window version suppressed a good answer ("The root cause is
not obvious"); the sentence-scoped negator list waved fabrications through
because any negator anywhere in the sentence excused the claim:

  "This is a known issue with no workaround."              -> suppress false
  "I cannot reproduce it, but the root cause is a re-render." -> suppress false
  "Bug confirmed, though I have no repro steps."           -> suppress false, penalty 0

Natural-language negation is not regex-tractable, so it stops gating a
user-visible publish/withhold decision.

Restructure:

1. `suppress` is now driven ONLY by the objective signal — CopilotKit
   identifiers named in the response but absent from every retrieved source
   (`unsourcedIdentifiers.length >= SUPPRESS_AT_UNSOURCED_IDENTIFIERS`). That is
   checkable against the sources we handed the model; no English is parsed.

2. Claim phrases are penalty-only. Still detected, still charged
   `PENALTY_PER_UNVERIFIED_CLAIM`, still reported on `unverifiedClaims` and
   `reasons` — but they contribute nothing to `suppress`. The penalty still
   drags the score under the escalation gate, so a human is pulled in and the
   disclaimer still lands. The consequence of a misread is a lower score, never
   a withheld reply.

3. The negation machinery is deleted entirely — sentence splitter
   (`SENTENCE_BOUNDARY`, `splitSentences`), negator list (`NEGATOR_PATTERN`),
   and the intensifier carve-outs (`INTENSIFIER_PATTERN`,
   `INTENSIFIER_REPLACEMENT`). Nothing needs it once claims are penalty-only: a
   false positive on "the root cause is not obvious" costs 0.35 of confidence
   instead of the reporter's answer. Removing it removes the whole recurring
   class of bug rather than tuning it a fourth time.

4. Identifier extraction gaps closed, since this signal now carries the whole
   gate:
   - Scheme-less hostnames are stripped (`BARE_HOST_PATTERN`). `URL_PATTERN`
     only caught `https://` / `www.`, so `docs.copilotkit.ai/reference` still
     yielded the phantom identifier `copilotkit` — which under the new contract
     is not merely a wrong penalty, it is two phantoms away from withholding a
     correct citation. Anchored on a known TLD so `1.2.3` and
     `.copilotKitInput.copilotKitInputExpanded` are not mistaken for hosts.
   - `reasons` now agree with what was charged. `unverifiedClaims` was deduped
     globally while charging counted per-sentence categories, so the logged
     basis understated the deduction. Charging is now one per claim CATEGORY
     over the response and the reported labels ARE the charged ones —
     `penalty == unverifiedClaims.length * PENALTY_PER_UNVERIFIED_CLAIM`
     (pre-cap). Pattern order was adjusted so the more specific wording supplies
     the label ("known issue" reads as "claims a known bug").
   - The bare-identifier guard was widened (`identifierSegments`): call
     expressions (`useCopilotKitFoo()`, `useCopilotKitFoo({...})`), JSX
     (`<CopilotKitFoo />`, `</CopilotKitFoo>`, `<CopilotKitFoo>`) and dotted
     member forms (`window.copilotKitFoo` -> `copilotKitFoo`) all count now; a
     fabrication must not escape the gate on syntax alone. `@copilotkit/*`
     package specifiers, including subpaths, stay excluded. Results are ordered
     by position in the response so logs read like the answer.

5. Public API preserved: `assessGroundedness`, `extractCopilotKitIdentifiers`,
   `MAX_GROUNDEDNESS_PENALTY`, `SUPPRESS_AT_UNSOURCED_IDENTIFIERS` and every
   `GroundednessAssessment` field. Module stays deterministic — no model calls.

Known edge (unchanged code, stated for the record): a claim-only response that
starts at a perfect score AND carries the maximum positive feedback calibration
lands exactly ON `AI_CONFIDENCE.ESCALATE` (1.0 - 0.6 cap = 0.4) rather than
below it, so it is not escalated by score. Previously the suppression clamp in
pipeline.ts covered that case for claim wording; it now covers only the
identifier signal. Pinned by pipeline.test.ts "cannot be offset by positive
feedback calibration". Raising `MAX_GROUNDEDNESS_PENALTY` or clamping on
`unverifiedClaims.length > 0` in pipeline.ts would close it; both are behavior
changes outside this fix.

## Call-site enumeration

`grep -rn <symbol> packages apps`, excluding node_modules and dist.

REMOVED (zero remaining references, verified by grep):
- `NEGATOR_PATTERN` — (no references)
- `INTENSIFIER_PATTERN` — (no references)
- `INTENSIFIER_REPLACEMENT` — (no references)
- `SENTENCE_BOUNDARY` — (no references)
- `splitSentences` — (no references)
- `chargeableClaims` (local) — (no references)
All five were module-private in groundedness.ts and never exported from
ai/src/index.ts, so removal is invisible outside the file.

ADDED (module-private, no external call sites by design):
- `BARE_HOST_PATTERN`, `JSX_WRAPPER`, `CALL_EXPRESSION`, `IDENTIFIER_PATH`,
  `identifierSegments` — referenced only inside groundedness.ts; grep finds no
  other site. Behavior is pinned through the two exported functions.

CHANGED — `assessGroundedness(response, sources)` (signature and return type
unchanged; `suppress` and `unverifiedClaims` semantics changed):
- packages/outpost/ai/src/generator.ts:6,135 — assesses and passes the result
  through on `GeneratedResponse.groundedness` without reading `suppress` or
  `unverifiedClaims`. Assumption holds: it only forwards the object.
- packages/outpost/ai/src/generator.ts:163 — fallback path calls with `''`; the
  empty-response short circuit is untouched, still an all-zero assessment.
- packages/outpost/ai/src/pipeline.ts:10,152 — recompute path when a caller
  injects a generator that supplies no assessment. Assumption holds; same
  signature, same field set.
- packages/outpost/ai/src/pipeline.ts:153-158 — applies `penalty` once. Holds:
  penalty is still 0..MAX_GROUNDEDNESS_PENALTY, and claim wording still
  contributes to it, so the escalation path for a misworded answer is intact.
- packages/outpost/ai/src/pipeline.ts:166-168 — clamps a suppressed response to
  `SUPPRESSED_CONFIDENCE_CAP`. Still correct, now narrower: it fires on the
  identifier signal only. See "Known edge" above.
- packages/outpost/ai/src/pipeline.ts:209-213,223-224 — logs `reasons` and
  publishes `suppressed: groundedness.suppress`. Holds; `reasons` is unchanged
  in shape and now strictly more accurate about the penalty.
- packages/outpost/ai/src/pipeline-groundedness.test.ts:23,119 — arithmetic
  fixture uses a one-invented-identifier response (penalty 0.15, not
  suppressed) and a two-identifier response for the clamp. Both assumptions
  hold under the new gate; file needed no edit and passes unchanged.
- packages/outpost/ai/src/index.ts:6 — re-export only.

CHANGED — `extractCopilotKitIdentifiers(response)` (widened; signature and
return type unchanged):
- packages/outpost/ai/src/index.ts:7 — re-export only. No other production call
  site; it is used inside groundedness.ts and by tests. Assumption holds:
  still `(string) => string[]`, still deduped case-insensitively, now ordered by
  position and inclusive of call/JSX/dotted spellings.

UNCHANGED exports, confirmed still referenced and still valid:
- `MAX_GROUNDEDNESS_PENALTY` — ai/src/index.ts:8 (re-export), value 0.6 kept.
- `SUPPRESS_AT_UNSOURCED_IDENTIFIERS` — ai/src/index.ts:9 (re-export), value 2
  kept; it is now the sole input to `suppress`.
- `GroundednessAssessment` — ai/src/index.ts:11, ai/src/types.ts:9,66,222. Field
  set identical (`penalty`, `unverifiedClaims`, `unsourcedIdentifiers`,
  `hedgeCount`, `suppress`, `reasons`), so both `GeneratedResponse.groundedness`
  and `PipelineResult.groundedness` are unaffected. types.ts:226's comment
  ("`groundedness.suppress` … gates a post") is still accurate.

Queue handler — no source change needed, assumptions verified:
- packages/outpost/queue/src/handlers/ai-response.ts:186 — skips the public post
  on `pipelineResult.suppressed`. Holds; suppressed now means "named identifiers
  no source contains", which is exactly the unpublishable case.
- .../ai-response.ts:189,268 — renders `groundedness.reasons` into the internal
  note and the escalation reason. Holds; `reasons` still populated for both
  claims and identifiers.
- .../ai-response.ts:263,267,287,299,300 — escalate on
  `score < ESCALATE || suppressed`; unchanged semantics.
- `npx tsc --project queue/tsconfig.json --noEmit` is clean.

Tests touched:
- packages/outpost/ai/src/pipeline.test.ts — "marks a response that confirms a
  bug as suppressed" replaced by "penalizes a bug-confirming response into
  escalation without withholding it" plus "marks a response naming identifiers
  absent from the sources as suppressed"; the calibration test now pins
  0.85 + 0.15 - 0.6 = 0.4 and a new sibling proves a boost cannot lift a
  suppressed response over the gate.
- packages/outpost/queue/src/__tests__/ai-response.test.ts — the hand-built
  `suppressedResult` fixture described an impossible assessment under the new
  contract (suppress: true with one unsourced identifier). Given the two
  invented class names #6167 actually shipped, plus the matching reasons.

Verification: `npx vitest run --root packages/outpost --reporter=dot` → 56 files
/ 883 tests green. `npx tsc --noEmit -p packages/outpost/ai` clean; db, ai and
queue projects all compile.
Suppression was enforced at ONE consumer (the queue handler), so every other
consumer published ungrounded text. Reviewers found three bypasses:

  1. apps/web/src/app/api/qa/route.ts streamed `result.response` to the browser
     without ever reading `suppressed`.
  2. AIPipeline.generateStreamingResponse → ResponseGenerator.generateStream
     produced no assessment at all — no groundedness, no penalty, no
     suppression, no disclaimer. The gate did not exist on that path.
  3. The queue handler checked `pipelineResult.suppressed` BEFORE its
     SHADOW_MODE branch, so in shadow mode a suppressed response wrote no
     shadow record — the responses most worth studying stopped being logged.

Fixed at the boundary instead of at each consumer: the pipeline itself now
withholds the ungrounded text, so every consumer inherits the gate.

- `generateSupportResponse` swaps the new exported SUPPRESSED_RESPONSE_TEXT
  into `formatted` when `groundedness.suppress` is true. `response` still
  carries the ORIGINAL draft — a human picking up the escalation works from it —
  and `suppressed`/`groundedness` stay on the result for analytics. Publishing
  `formatted` is now safe by construction on every platform target.
- The replacement copy already promises a human follow-up, so it is paired with
  the plain AI_DISCLAIMER rather than stacking AI_DISCLAIMER_ESCALATED on top.
- `generateStreamingResponse` now buffers the model stream, assesses the
  complete text, then yields — either the original chunk boundaries or, when
  suppressed, only SUPPRESSED_RESPONSE_TEXT. Groundedness is a property of the
  WHOLE response and text already on the wire cannot be recalled, so an
  incremental gate is impossible; buffering costs time-to-first-token (it now
  equals total latency) and that tradeoff is documented on the method. The
  alternative — leaving it ungated while the module docs claim a gate — was not
  acceptable, and refusing outright would have deleted a working public API.
- The queue handler's `if (pipelineResult.suppressed)` arm is DELETED from the
  step-5b branch chain; it posts unconditionally with the pipeline's safe text.
  That removes the shadow-mode ordering bug by construction rather than
  reordering the branches. Escalation still fires on
  `confidenceScore < AI_CONFIDENCE.ESCALATE || pipelineResult.suppressed`.
- apps/web/src/app/api/qa/route.ts streams `result.formatted.text` instead of
  `result.response`, so it consumes the published text and never needs to read
  `suppressed`. Side effect: the SSE stream now carries the web disclaimer and
  footer, which is the text we actually intend to publish.

Tests (red-green verified against a temporary revert of each change):
- pipeline-groundedness.test.ts (real generator via aimock + real formatter):
  a suppressed draft publishes the replacement and never the draft on all five
  platform targets (discord/github/slack/teams/web, Discord `parts` included);
  `result.response` still equals the draft; the escalated disclaimer is not
  stacked on the replacement; a grounded draft is published untouched; plus the
  streaming gate contract.
- pipeline.test.ts: the formatter receives SUPPRESSED_RESPONSE_TEXT (not the
  draft) with `disclaimerText: AI_DISCLAIMER`; five generateStreamingResponse
  tests including one proving the WHOLE buffer is assessed, not per chunk.
- queue ai-response.test.ts: posts the safe replacement rather than staying
  silent; shadow mode records a suppressed response (the bug above); escalates
  on suppression at score 0.95 asserting on THAT call's return value — the
  previous version of this test asserted on a stale earlier result.
- qa-api.test.ts: streams the safe published text, never the suppressed draft.
  Assertions reassemble the SSE token events first — the route emits 8-char
  chunks, so a `not.toContain` on the raw payload passes vacuously.

Verification: `npx vitest run --root packages/outpost --reporter=dot` 893
passed / 56 files; `npx vitest run` in apps/web 503 passed / 45 files;
`npx tsc --noEmit -p packages/outpost/ai` clean (also queue and apps/web).

Call-site enumeration (grep -rn across packages/ and apps/, dist excluded)

  SUPPRESSED_RESPONSE_TEXT (ADDED)
    packages/outpost/ai/src/pipeline.ts:38                 declaration
    packages/outpost/ai/src/pipeline.ts:218                 non-streaming swap
    packages/outpost/ai/src/pipeline.ts:348                 streaming swap
    packages/outpost/ai/src/index.ts:19                     public re-export
    packages/outpost/ai/src/pipeline.test.ts:19,257,276,511,526
    packages/outpost/ai/src/pipeline-groundedness.test.ts:20,198,216,245,272
    referenced in prose: ai/src/types.ts:215,
      queue/src/handlers/ai-response.ts:15,184,
      queue/src/__tests__/ai-response.test.ts:158
    No non-test runtime consumer outside the ai package — consumers inherit the
    copy through `formatted`, they do not import the constant.

  PipelineResult (CHANGED — semantics of `response`/`formatted`/`suppressed`)
    packages/outpost/ai/src/types.ts:206                    declaration
    packages/outpost/ai/src/pipeline.ts:4,102               only typed usage
    Structural consumers (no type import, so no compile-time coupling):
      packages/outpost/queue/src/handlers/ai-response.ts:121 (generateSupportResponse)
        - :162  reads .response         → BOT Message content (the draft, intended)
        - :175  reads .formatted.text   → ticket.suggestedResponse (now safe copy)
        - :192  reads .suppressed       → log only, no longer a gate
        - :206  reads .formatted.text   → shadow-mode SYSTEM message
        - :247  reads .formatted        → adapter.postResponse
        - :272,276,296,308,309 read .suppressed → escalation + result payload
      apps/web/src/app/api/qa/route.ts:62 (generateSupportResponse)
        - :78  reads .formatted.text (was .response) → SSE token stream
        - reads .confidenceLevel, .searchResults for the metadata event
    No other file in packages/ or apps/ consumes a PipelineResult.

  AIPipeline.generateSupportResponse (CHANGED — `formatted` now gated)
    packages/outpost/ai/src/pipeline.ts:99                  definition
    packages/outpost/queue/src/handlers/ai-response.ts:121
    apps/web/src/app/api/qa/route.ts:62
    tests: ai/src/pipeline.test.ts, ai/src/pipeline-groundedness.test.ts,
      queue/src/__tests__/ai-response.test.ts (mocked),
      apps/web/src/__tests__/qa-api.test.ts (mocked)

  AIPipeline.generateStreamingResponse (CHANGED — buffers, assesses, gates)
    packages/outpost/ai/src/pipeline.ts:310                 definition
    No production caller anywhere in packages/ or apps/ — the only callers are
    the new tests (ai/src/pipeline.test.ts:496,508,523,534,552 and
    ai/src/pipeline-groundedness.test.ts:269,285). The web QA route uses the
    non-streaming path and chunks the finished text itself.

  ResponseGenerator.generateStream (UNCHANGED)
    packages/outpost/ai/src/generator.ts:172                definition
    packages/outpost/ai/src/pipeline.ts:333                 sole runtime caller
    packages/outpost/ai/src/generator.test.ts:252,268
    Raw model access below the gate, like the Anthropic client itself.

  AI_DISCLAIMER (UNCHANGED, new consumer)
    packages/outpost/ai/src/formatter.ts:22                 declaration
    packages/outpost/ai/src/pipeline.ts:17,234              NEW import + use
    packages/outpost/ai/src/index.ts:14, formatter.test.ts:3,11,17,91,
      pipeline.test.ts:20 (new), pipeline-groundedness.test.ts:23 (new)

  Nothing was removed from any public surface.
…e level

`ResponseGenerator.generate` set two public fields from a score that
deliberately excludes the groundedness penalty. `assessConfidence` is
retrieval-only by design — the pipeline is the single place that deducts,
so the penalty is not double-counted — but `autoSend` and
`confidenceLevel` were computed from it BEFORE the deduction. A response
whose `groundedness.suppress` was true therefore came back with
`autoSend: true` and `confidenceLevel: HIGH`. Two sources scoring 0.9 and
0.85 give a retrieval score of 0.975, which clears
`AI_CONFIDENCE.AUTO_RESPOND` (0.9) and `HIGH_THRESHOLD` (0.8) regardless
of how ungrounded the text is. Six reviewers reported it independently.

`autoSend` had no consumer anywhere in the repo — the only non-test
occurrences were its declaration and its two assignments (evidence
below). Rather than ship a public boolean that lies and that everybody
must keep correct for nobody, it is removed. The gate it purported to
implement is `PipelineResult.suppressed` plus the score-based escalation
in queue/handlers/ai-response.ts, both of which already work.

`confidenceLevel` stays — it is a public claim about the response — and
is now classified from the penalised value: retrieval score minus
`groundedness.penalty`, clamped to `SUPPRESSED_CONFIDENCE_CAP` when the
gate would withhold the text, exactly as the pipeline clamps its own
score. The penalised value is LOCAL to the classification.
`confidenceScore` is not mutated and stays retrieval-only, so it still
feeds the pipeline's `min()` undeducted and the penalty is still charged
exactly once. No second deduction is reintroduced.

`SUPPRESSED_CONFIDENCE_CAP` moves from a private const in pipeline.ts to
an export in types.ts so the generator and the pipeline cannot disagree
about what a withheld response is worth.

Tests: `pipeline-groundedness.test.ts` gains four cases against the real
generator over aimock — a suppressible response on 0.975-scoring sources
must classify LOW (it classified HIGH before this change), an ungrounded
but publishable response keeps its penalised level, a grounded response
on the same sources keeps HIGH, and `autoSend` is no longer a key on the
returned object. Red-green confirmed: both new invariants failed against
the prior code with `expected 'HIGH' to be 'LOW'` and
`expected true to be false`.

Call sites
----------
REMOVED `GeneratedResponse.autoSend` (was types.ts:52)
  - writer  packages/outpost/ai/src/generator.ts:142  (success path)   — deleted
  - writer  packages/outpost/ai/src/generator.ts:158  (fallback path)  — deleted
  - fixture packages/outpost/ai/src/pipeline.test.ts:76                — deleted
  - assert  packages/outpost/ai/src/generator.test.ts:115              — deleted
  - assert  packages/outpost/ai/src/generator.test.ts:129              — deleted
  - production readers: NONE (this is why it was removed)

  Post-removal grep (node_modules and dist excluded):
    $ grep -rn 'autoSend' packages/ apps/
    packages/outpost/ai/src/pipeline-groundedness.test.ts:232:  it('no longer exposes an autoSend field ...
    packages/outpost/ai/src/pipeline-groundedness.test.ts:244:      expect('autoSend' in generated).toBe(false);
  Zero remaining references outside the regression test that pins the
  removal. The all-filetypes grep (no --include filter) returns the same
  two lines, so no JSON, SQL, or Prisma surface referenced it either.

REMOVED private const `SUPPRESSED_CONFIDENCE_CAP` from pipeline.ts:35
  - sole reader packages/outpost/ai/src/pipeline.ts:167 — now reads the
    types.ts export; value is unchanged (`AI_CONFIDENCE.ESCALATE - 0.01`)

ADDED `SUPPRESSED_CONFIDENCE_CAP` (packages/outpost/ai/src/types.ts)
  - packages/outpost/ai/src/pipeline.ts:167  (suppressed-score clamp)
  - packages/outpost/ai/src/generator.ts     (classifyGroundedConfidence)

ADDED private `ResponseGenerator.classifyGroundedConfidence`
  - packages/outpost/ai/src/generator.ts, inside `generate()` — the only
    call site; private, so no external surface

CHANGED semantics of `GeneratedResponse.confidenceLevel`
  - writer packages/outpost/ai/src/generator.ts (success + fallback)
  - production readers: NONE. `AIPipeline.generateSupportResponse`
    computes `PipelineResult.confidenceLevel` itself from
    `finalConfidenceScore` (pipeline.ts:218) and never reads the
    generator's field, so the queue handler
    (queue/src/handlers/ai-response.ts:164,285,294) and the web
    dashboard (apps/web) are unaffected.
  - test readers: generator.test.ts:77,100,114,128 and the new cases in
    pipeline-groundedness.test.ts — all pass unchanged or updated here.

CHANGED doc comments on `GeneratedResponse.confidenceScore` and
`.confidenceLevel` (types.ts) so the field docs describe what the code
does. The old "Whether this response should be auto-sent" comment
described a gate nothing implemented and is gone with its field.

NOT CHANGED `AI_CONFIDENCE.AUTO_RESPOND` (shared/src/constants.ts:39).
Removing autoSend leaves it with no reader in src, but it is part of the
documented action-band set alongside SUGGEST/ESCALATE and is mirrored in
queue and github-app test fixtures. Deleting a public shared constant is
out of scope for this fix.

Verification
------------
  npx vitest run --root packages/outpost --reporter=dot
    -> 56 files passed, 878 tests passed
  npx tsc --noEmit -p packages/outpost/ai   -> exit 0
  npx tsc --noEmit -p packages/outpost/queue -> exit 0
Prettier reports the same four pre-existing deviations at HEAD as after
this change (generator.ts, pipeline.ts, generator.test.ts,
pipeline.test.ts), so nothing here regressed formatting; unrelated
reformatting was deliberately not bundled in.
Round-2 test-only pass over the groundedness-gate branch. Every guard below was
mutation-proven: the behaviour it names was removed from the source, the guard
was confirmed to fail, the source was restored, and the guard was confirmed to
pass. No production source behaviour changed.

- pipeline.test.ts: the shared generated-response fixture omits `groundedness`,
  so every test drove the `generatedResponse.groundedness ?? assessGroundedness()`
  RECOMPUTE branch and nothing exercised the generator-supplied pass-through —
  the core invariant of the single-deduction fix. Adds a sentinel assessment the
  recompute could not produce, plus an explicit test for the fallback branch.

- pipeline.test.ts: disclaimer rows. The score-0.95 row asserted on
  `disclaimerText` for a case where `addDisclaimer` is false, i.e. on copy that
  is never rendered; and the "neutral MEDIUM copy" row claimed a third variant
  that does not exist (both the LOW-not-escalated band and MEDIUM resolve to the
  same exported AI_DISCLAIMER_REVIEWED). Rows now read the whole format options
  object, pin copy by imported constant rather than string literal, and the HIGH
  case asserts the real fact — no disclaimer is rendered at all.

- pipeline-groundedness.test.ts: the double-application guard hard-coded
  `SCORER_SCORE - penalty * 2` (0.65), which is not a value the double-count bug
  can produce (it produces 0.675), so the guard passed with the bug restored —
  only a neighbouring equality check caught it. The value is now derived from
  named constants, the guard lives in its own test so it is the only assertion
  that can fail, and the surrounding arithmetic comment is corrected.

- groundedness.test.ts: the "tolerates sources with missing title or content"
  test passed EMPTY STRINGS, so the `?? ''` absence guards never executed. Now
  uses genuinely absent fields via a partial cast, asserts the resulting
  behaviour rather than only "does not throw", and the name matches what it does.

- queue/__tests__/ai-response.test.ts: `process.env.SHADOW_MODE = originalShadow`
  stores the STRING "undefined" when the var was unset, leaking a defined env var
  into every later test. All three sites now go through a `restoreShadowMode`
  helper that deletes when the original was absent (the pattern already used in
  onboarding-digest.test.ts), and the helper itself is pinned by two tests.

Verified holding at HEAD, no change needed: the "escalates even though the score
is above ESCALATE" test already asserts its own call's return value with a 0.95
score, and fails when `|| pipelineResult.suppressed` is deleted from the handler.

Suite: 56 files / 914 tests green (was 906). tsc --noEmit -p packages/outpost/ai
clean.
…catches

The groundedness penalty is capped at MAX_GROUNDEDNESS_PENALTY (0.6), so a
response with a perfect base score plus the maximum positive feedback
calibration lands on exactly 1.0 - 0.6 = AI_CONFIDENCE.ESCALATE. The escalation
gate tests `< ESCALATE`, so that worst case posted with a "we will review it"
disclaimer and paged nobody. The suppression clamp used to cover it incidentally;
once claim wording stopped driving suppression, the case was exposed.

A charged claim is now clamped below the gate outright. The split is deliberate:
claim wording never gates PUBLICATION (it is fallible English, and withholding a
reply on a misread is the failure this branch spent three attempts on), but it is
decisive for ESCALATION (the bot asserted something it cannot back, so a person
looks at it).

The existing calibration test documented the hole with `toBeLessThanOrEqual` and
now pins the fix with a strict `<`. Two cases added: a perfect score with maximum
boost, and a charged claim on a generator-SUPPLIED assessment, so the clamp is
verified on both the recompute and pass-through branches.

The pass-through sentinel is now claim-free by design, with a comment explaining
why: a charged claim clamps the score, which would mask the penalty arithmetic
that test exists to observe. Its identifier, hedge count and reason remain
impossible for a recompute, so it keeps its distinguishing power.

Red-green: reverting the clamp fails 3 tests in pipeline.test.ts; restoring it
returns 916/916 green across packages/outpost. Typecheck clean.

Call sites: SUPPRESSED_CONFIDENCE_CAP (types.ts:37) — 3 readers, all unchanged in
meaning: generator.ts:280 (level classification), pipeline.ts:187 (suppression
clamp), pipeline.ts new claim clamp. groundedness.unverifiedClaims — read in
pipeline.ts (this clamp, reasons logging) and queue/handlers/ai-response.ts
(escalation reason string); both still hold, the field's semantics are unchanged.
@NathanTarbert

Copy link
Copy Markdown
Collaborator Author

Pushed four more commits (28fb1fb..89cb3d5) and ran the CR loop over the result — three rounds, 33 reviewer agents, a convergence audit, and five fix agents. CI is green at 89cb3d5.

What changed since your review

Your double-penalty catch was fixed as you described, and then the loop found that the negation logic underneath it was unsalvageable. Two more attempts failed in two more directions — window-scoped, then sentence-scoped — before the design changed:

Suppression is now driven by the objective signal only. Identifiers named in the response but absent from every retrieved source. That is verifiable against the sources; no English parsing. The negation machinery (isNegated, NEGATION_WINDOW, NEGATOR_PATTERN, splitSentences) is deleted outright, not patched.

Claim wording is penalty-only. "Bug confirmed" still charges 0.35, still reports in unverifiedClaims, and now always clamps below the escalation gate — but it never withholds a reply. The consequence of a misread dropped from "reporter gets silence" to "score is lower and a human looks at it." Your negation-blindness finding is what pointed at this: once suppression is user-visible absence, the cost asymmetry decides the design.

The gate moved to the boundary. generateSupportResponse swaps the safe copy into formatted, so every consumer inherits it rather than each one re-implementing the check. The queue handler's suppressed-arm is deleted — which removes the shadow-mode ordering bug by construction rather than reordering it — and apps/web/src/app/api/qa/route.ts (which was streaming result.response and bypassing the gate entirely) now streams the published text.

autoSend is gone. Zero readers repo-wide; it was a public boolean computed from the pre-deduction score, so it read true for exactly the responses the gate withholds. Removed rather than repaired. confidenceLevel now classifies from the penalised value while confidenceScore stays retrieval-only, so the single-deduction invariant holds.

Six test guards repaired, each mutation-proven. Your instinct that the suite could not catch the double-penalty generalised: deleting || suppressed from the handler left 31/31 green, and deleting generatedResponse.groundedness ?? left 37/37 green. Both now fail when their behaviour is removed. The old 0.65 guard could never have caught the bug it was named for — double application yields 0.675.

916 tests, typecheck and build clean.

What I did not fix, and why

Three rounds in, every new bucket-(a) finding was in code I had written during that same cycle — a signal's meaning changed and some reader of the old meaning went unswept. Three instances (autoSend, then streaming + generator level + QA metadata). Patching the third set invites a fourth, so I stopped the loop rather than run a round I expected to produce more of my own cleanup:

Also confirmed and left alone as out-of-subject: temperature is sent unconditionally while the model is env-overridable, so a bump to Opus 4.7+/Sonnet 5 400s and every ticket silently degrades to the apology fallback. Worth its own issue — it is a live production hazard, not a latent one.

npm run lint cannot run at all in this repo (no eslint.config.*; ESLint 9 flat-config migration outstanding), identically on clean main. Prettier flags several touched files both before and after these commits, so formatting was left alone rather than bundled in.

Ready for another look.

@NathanTarbert

NathanTarbert commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Correction to my comment above — three of the four remaining reviewers landed after I posted it, and two of my coverage claims were incorrect.

"Discord parts included" is not true. The boundary test loops over formatted.parts asserting no part carries the draft, but the suppressed copy plus disclaimer is ~390 characters against Discord's 2000-char limit, so parts is undefined and the loop body never runs. The per-platform assertion on formatted.text does hold; the multi-part case is unasserted.

The "escalates on suppression at score 0.95" fixture describes an unreachable state. pipeline.ts clamps a suppressed response to below the escalation gate at two separate sites, so suppressed: true with a score of 0.95 cannot occur. The test still proves the handler ORs suppressed into its escalation condition — the mutation check confirms that — but the scenario it dramatises is fictional, and PipelineResult does not encode the invariant that would have made the fixture impossible to write.

And one more guard that isn't. pipeline-groundedness.test.ts — "reports the penalised level for an ungrounded but publishable response": deleting the penalty subtraction from classifyGroundedConfidence leaves all tests green. That test was added in this cycle specifically to pin the grounded-confidenceLevel fix, and it does not.

All three are folded into #146 rather than fixed here, alongside the parameterized-coverage lever. Same root cause as everything else in that issue: I verified the fixes by mutation where I remembered to, and the places I did not are exactly where the coverage turned out to be hollow.

The six commits and the CI-green result stand — what changed is my confidence in three specific assertions, not in the behaviour they were meant to cover. The behaviour is separately verified by probes recorded in the CR ledger.

@jerelvelarde jerelvelarde left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The gate placement is the best decision in here — formatted is safe by construction, so consumers publish it unconditionally and never have to know the gate exists. The QA route switching from result.response to result.formatted.text is that contract paying off immediately. Test coverage is unusually good: the verbatim #6167 fixture, per-platform suppression, calibration ordering, the double-count guard, and the escalate-despite-high-score path are all pinned.

A few things, none of them blocking.

1. Any single claim phrase now forces an escalation — worth quantifying

pipeline.ts clamps below the escalation gate on unverifiedClaims.length > 0. Two of the patterns are ordinary English in a correct, docs-grounded answer:

  • /\bthe\s+fix\s+is\b/i — "The fix is to pass the input prop."
  • /\bknown\s+(?:bug|issue|regression)\b/i — "This is a known issue, fixed in 1.9.2."

Both are things the bot should be able to say when the docs say them, and each one now creates an ESCALATION job plus the escalated disclaimer regardless of how good the answer is. The design is deliberate and the comment defends it — but the PR quantifies the withholding trade and not this one, and this is the one that hits ops volume every day rather than only on fabrications. Worth running over the last N real responses (or a week of shadow mode) to get a false-positive rate. Cheaper mitigation if it turns out noisy: skip the clamp when the claim's sentence is itself grounded in a source.

2. The description is wrong about suggestedResponse

The draft still persists as a BOT message and as suggestedResponse on the ticket, so a human can edit and send it.

The second half isn't true — ai-response.ts:156 sets suggestedResponse: pipelineResult.formatted.text, which for a suppressed draft is the canned replacement. The handler's own inline comment gets this right ("suggestedResponse holds the publishable text bots pick up"); only the PR body overstates it. Practically: an agent looking at the ticket's suggestion field sees boilerplate, and the draft is only findable in the BOT message thread.

Related follow-up: Message.content is the raw draft while the posted text is formatted.text. Suppression makes those diverge hard — the DB says the bot posted "## Bug Confirmed…" when the public thread got "I couldn't find an answer." Anyone auditing bot output from the DB gets the wrong answer. Worth stashing the published text in attachments.

3. AI_CONFIDENCE.AUTO_RESPOND is now dead

Removing autoSend leaves no src/ consumer, but shared/src/constants.ts:32 still documents it as in use by the generator. Drop it or fix the comment. (The autoSend removal itself is clean — nothing outside ai/src read it.)

4. classifyGroundedConfidence guards a field nobody reads

ResponseGenerator is only ever consumed by AIPipeline, which reads confidenceScore and recomputes the level itself, so GeneratedResponse.confidenceLevel has no production reader. Defensible as public-API hygiene since the class is exported from index.ts, but it's ~25 lines defending a value that never escapes.

5. "Exactly once" is narrower than it reads

True of groundedness.penalty. But confidence.ts now scores groundedness as rubric factor 5, and that enters through min(generator, scorer) before the deterministic deduction — so an ungrounded answer can be charged by two independent mechanisms. Probably fine as defense in depth and MAX_GROUNDEDNESS_PENALTY bounds it; just not the same claim the comments make.

6. Nits

  • Two consecutive if blocks in pipeline.ts both do Math.min(finalConfidenceScore, SUPPRESSED_CONFIDENCE_CAP). Collapse them.
  • generateStreamingResponse now buffers the whole draft, so TTFT == total latency. No caller in src/ today and the doc comment is honest about it, but a future wiring loses streaming silently.

Checked and clean

  • SUPPRESSED_CONFIDENCE_CAP = ESCALATE - 0.01 = 0.39 < 0.4 — clears the gate, no float boundary problem.
  • The generator's error path supplies groundedness, so the pipeline's ?? never recomputes over the fallback copy.
  • Type-only import breaks the types.tsgroundedness.ts cycle properly.
  • Shadow-mode ordering fix is right — no early return ahead of the SHADOW_MODE arm.
  • I stress-tested BARE_HOST_PATTERN for catastrophic backtracking, since it has the classic (a(b*a)?\.)+ shape. It's quadratic, not exponential — 6.4KB of adversarial hyphenated input runs in 26ms. Not a ReDoS.

#1 is the one I'd want an answer on before this is enabled in production, even if the answer is just "watching escalation volume in shadow mode first." #2 is a description fix. The rest are follow-ups.

Review follow-up on #143 (jerelvelarde, approved with six items).

The escalation clamp fired on `unverifiedClaims.length > 0`, which included two
patterns that a correct, docs-grounded answer uses in ordinary prose:

  "This is a known issue, fixed in 1.9.2."
  "The fix is to pass the `input` prop."

Both are things the bot SHOULD say when the docs say them, and each one created
an ESCALATION job plus the escalated disclaimer regardless of how good the answer
was. That is a person's attention spent on English rather than on fabrication,
and unlike the withholding trade-off it would have been paid every day.

Claims are now split by who is being quoted. ESCALATION_FORCING_CATEGORIES holds
the ones asserting that WE did something we cannot have done — confirmed a bug,
established a root cause, reproduced it. `known-issue` and `fix` are new/moved
categories that are priced but page nobody: the response still scores lower, so
a wording-heavy answer still drifts toward review, it just does not wake anyone.

`GroundednessAssessment` gains `forcesEscalation`, and the pipeline clamps on
`suppress || forcesEscalation` rather than on any charged claim. Publication is
unchanged — claim wording still never withholds anything.

One thing did not work on the first attempt: `known` was in the real-bug
pattern's adjective list, so "this is a known issue" matched both categories and
escalated anyway, defeating the split for its single most common phrasing.
Removed from that list, with a named regression test.

Also from the review:

- constants.ts: AUTO_RESPOND and SUGGEST are documented as feeding the pipeline.
  They have no reader in src/ — the action-based scheme they described was never
  implemented (everything posts; what varies is the disclaimer and whether a
  human is paged), and the last reader was autoSend, removed in 2f501ed. Doc now
  says so, and says to delete them rather than rewire them if a real auto-post
  gate is ever built.
- types.ts: "the penalty is charged exactly once" was narrower than it read. True
  of the deterministic penalty; the LLM scorer also weighs groundedness as rubric
  factor 5 and enters through the same min() before this deduction. Named as
  intentional defense in depth instead of overstated as a single mechanism.
- pipeline.ts: two consecutive if blocks clamped to the same cap. Collapsed.

Kept deliberately: classifyGroundedConfidence guards GeneratedResponse
.confidenceLevel, which has no production reader since AIPipeline recomputes the
level. But ResponseGenerator is exported from index.ts, so that field is public
API, and the alternative is shipping a public value that reads HIGH for a
response the gate would withhold — the exact bug class autoSend was.

Tests: escalates/does-not-escalate corpus over both claim kinds, the "known
issue" regression guard, mixed-sentence case, and pipeline coverage for a
supplied assessment on both sides of the split. Reverting the category split
fails 5 tests. 929 tests green across packages/outpost, 10/10 packages,
typecheck and build clean.

Call sites: GroundednessAssessment.forcesEscalation (ADDED) — written in
groundedness.ts assessGroundedness, read in pipeline.ts (the clamp); no other
reader. ClaimCategory (CHANGED, +known-issue) — module-private, no external
reference. AI_CONFIDENCE.AUTO_RESPOND / .SUGGEST (comment only, values
unchanged) — grep confirms zero src/ readers; queue and github-app test fixtures
still reference the numbers and are unaffected.
@NathanTarbert

Copy link
Copy Markdown
Collaborator Author

Pushed 3cf34b3. Thanks for the approval and for #1 in particular — you were right that it was the item with daily cost, and it turned out to be a design bug rather than something to quantify.

1. Claim phrases forcing escalation — fixed rather than measured.

You framed it as "worth running over the last N responses to get a false-positive rate," but both examples you gave are things the bot should be able to say when the docs say them, so I'd have been measuring how often a wrong design misfires. Split the claims by who is being quoted instead:

  • Escalates — asserts we verified something: bug confirmed, this is a real bug, root cause is, I reproduced.
  • Priced only — reports what the docs say: known issue, the fix is.

GroundednessAssessment gains forcesEscalation, and the pipeline clamps on suppress || forcesEscalation. Publication is untouched — claim wording still never withholds.

Your exact example didn't work on the first attempt. known was in the real-bug pattern's adjective list, so "this is a known issue" matched both categories and escalated anyway — defeating the split for its most common phrasing. Removed, with a named regression test. Verified:

"This is a known issue, fixed in 1.9.2."     escalates: false  penalty: 0.35
"The fix is to pass the `input` prop."       escalates: false  penalty: 0.35
"This is a real bug worth fixing in core."   escalates: true   penalty: 0.35
"Bug Confirmed: cursor resets."              escalates: true   penalty: 0.35
"I reproduced this locally."                 escalates: true   penalty: 0.35

Reverting the category split fails 5 tests.

2. Description was wrong — fixed in the body, not just replied to. Your read is right and the handler's inline comment was the accurate one. Your follow-up is the sharper half, though: Message.content holds the raw draft while the thread got the replacement, so the DB doesn't merely omit that a draft was withheld — it says the bot posted the fabrication. Anyone auditing bot output from the database reads the wrong answer. Added to #148 with your attachments suggestion (no migration needed).

3. AUTO_RESPOND / SUGGEST. Kept the constants, rewrote the doc to say plainly they have no reader, why the action-based scheme was never implemented, and that autoSend was the last reader. Also noted they should be deleted rather than rewired if a genuine auto-post gate is ever built — otherwise someone wires them to something that happens to want a 0.9 cutoff.

4. classifyGroundedConfidence — keeping it, and here's the argument. You're right there's no production reader; AIPipeline recomputes the level. But ResponseGenerator is exported from index.ts, so confidenceLevel is public API, and without those lines it reads HIGH for a response the gate would withhold. That is precisely the bug class autoSend was, and I'd rather spend 25 lines than ship a public field that lies. If we ever stop exporting the generator, this goes with it.

5. "Exactly once" — corrected. You're right that it's narrower than it reads. It's true of the deterministic penalty; confidence.ts scores groundedness as rubric factor 5 and enters through the same min() beforehand. Comment now names that as intentional defense in depth with MAX_GROUNDEDNESS_PENALTY bounding the deterministic half, rather than claiming one mechanism.

6. Nits. Two clamps collapsed. Streaming TTFT left as documented — tracked in #146 along with the fact that the streaming path skips the penalty, the disclaimer, and escalation entirely, which is the more serious half of that trap.

Also, thank you for stress-testing BARE_HOST_PATTERN for backtracking. I wrote that pattern and did not check it, and "quadratic, 26ms on 6.4KB of adversarial input" is exactly the kind of thing I'd have shipped on faith.

One thing I want to flag as more urgent than anything in your list. After this PR, unsourced identifiers are the only signal that withholds a response — and that check only matches names containing the literal string copilotkit. A fabricated useCopilotAction, CopilotChat, or CopilotSidebar is invisible to it, and those are the names all over our docs and therefore the most natural thing for a model to invent. Filed as #147, and it's the next thing I'm working on. Until it lands, the gate covers less than this PR's description implies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants