fix(ENG-1279): let Tier 3 decide on score, not the model's word - #74
Merged
Merged
Conversation
The Tier 3 verdict's `decision` string is the model's argmax — an implicit 0.5 cut nobody chose. Reading it discards the distribution the model already computed, which costs two things: - The operating point is unreachable. Security callers rarely want 50/50; the v4.3 pilot reaches 95.9% recall at the shipped model's exact false-positive rate, but only at a 0.622 cut. Argmax cannot express that. - Every retrain silently moves the cut. A pilot corpus change shifted confidences down enough that recall read 96% -> 61% at the fixed 0.5 cut while ranking barely moved (AUC 0.978 vs 0.951) — a calibration shift misread as a quality regression. `tier3.blockThreshold` makes the cut an explicit config value, so retrains optimize ranking and moving the operating point is a one-line change. Opt-in by design: unset (the default) keeps the provider's `decision` authoritative, so this is a no-op until an operator sets a threshold. That also protects providers written against the old `score` doc, which said "confidence" without pinning the direction — thresholding those blindly would invert on allows. `score` is now documented as P(block), and a configured threshold with an unusable score falls back to `decision` and warns once rather than taking the tier offline. Verification: 349/349 defender specs green (12 new); tsc --noEmit clean; biome check + lint clean on ./src. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR makes Tier 3 decisions optionally depend on the model’s probability score (verdict.score as P(block)) rather than the generated decision word (argmax at an implicit 0.5 cut), by introducing an opt-in tier3.blockThreshold configuration and applying it consistently in both tier3-only and cascade override paths.
Changes:
- Add
tier3.blockThresholdoption and anisTier3Block()helper to decide block/allow viascore >= threshold(with fallback todecisionwhen score is unusable). - Update Tier 3 verdict documentation to define
scoreas P(block) and describe thresholding behavior. - Add specs covering opt-in behavior, 0.5 parity with argmax, warn-once fallback, invalid threshold handling, and cascade override behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/types.ts | Updates Tier 3 verdict docs to define score as P(block) and document blockThreshold semantics. |
| src/core/prompt-defense.ts | Adds tier3.blockThreshold parsing and isTier3Block(); applies thresholding at tier3-only and cascade override decision sites. |
| specs/tier3.spec.ts | Adds new tests for threshold opt-in behavior, fallback/warn-once behavior, and cascade override application. |
| README.md | Documents tier3.blockThreshold and how it changes the operating point. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses three Copilot review findings on #74, all rooted in the same gap: `validateTier3Verdict` only ever checked `decision`, so an untyped JS provider's `score` reached both the decision path and the public `DefenseResult.tier3` unchecked. - Normalize `score` in `validateTier3Verdict`: anything that is not a finite number in [0, 1] is dropped to `undefined`. This is the single choke point both spread sites (tier3_only and the cascade override) already pass through, so neither can leak a string/bigint/out-of-range value onto the exported `score?: number` contract. - Drop the raw provider value from the warn message. `JSON.stringify` throws on bigint and circular objects, and the tier3_only call site sits outside the provider try/catch — so a non-fatal log could have taken down the whole defense call. The message now reports the condition, not the value. - `isTier3Block` can then trust a `number` as usable P(block). Also pins two boundary cases the earlier specs left loose: `score` exactly equal to the threshold (`>=`, not `>`), and `blockThreshold` of 0 and 1 being accepted rather than rejected by the inclusive range check. Verification: 355/355 defender specs green (6 new); tsc --noEmit clean; biome check + lint clean on ./src. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
o-mauri
approved these changes
Jul 22, 2026
OMauriStkOne
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tier 3's verdict
decisionstring is the model's argmax — an implicit 0.5 cut nobody chose. Reading it discards the distribution the model already computed, and that costs us two things:This adds
tier3.blockThreshold. When set, the defender decides onverdict.score(P(block)) via a newisTier3Block()helper, applied at both decision sites —runTier3Onlyand the cascade escalation override. Retrains then optimize ranking, and moving the operating point is a one-line config change instead of a GPU run.Design calls worth reviewing
default: 0.5. The oldscoredoc said "confidence in [0, 1]" without pinning direction — a provider reporting "0.95 confident in my allow" would have its decision inverted by blind thresholding. Unset (the default) is byte-identical to today's code path for every existing provider.scoreis now documented as P(block), and setting a threshold is the operator asserting their provider conforms.0.5reproduces argmax exactly.scorefalls back todecisionand warns once (the existingtier3MissingProviderWarnedpattern) rather than taking the tier offline.types.ts's "Tier 3 is authoritative — the defender does not re-threshold the score" rule is rewritten, since that invariant is what this PR changes.Test plan
tsc --noEmitcleanbiome check+biome lintclean on./srcscore— inert until thenFollow-ups (not in this PR)
verdict.scoreas P(block). A provider that only parses the decision word needs to additionally request logprobs and take a two-way softmax over theblock/allowalternatives at the decision slot. The threshold does nothing until that lands.🤖 Generated with Claude Code