feat(suppression): location-scoped suppression — the negative twin of code-path memory (ADR-48)#25
Merged
Merged
Conversation
… code-path memory (ADR-48)
A dismissal was repo-wide: dismissing one intentional `console.log` taught Plex to
demote/suppress `no-console` everywhere, burying the next genuinely-stray one. Anchor
every dismissal to the `file#name` symbol it concerned and scope the suppression to it —
so the same rule still surfaces at every other symbol while the dismissed instance stays
quiet (the negative twin of code-path memory, ADR-47).
- SuppressionDecision gains repoWide + symbols; loadSuppressions derives them from the
dismissal incidents (fail-open to repoWide when any incident is symbol-less, a legacy
record, or an explicit pattern/category scope). First-principles + cross-repo stay
repo-wide in v1.
- submitVerdict resolves the dismissed finding's symbol/line from the brain finding
(hoisted above recordVerdict/learnSuppression so reject/waive/acknowledge all inherit
it); learnSuppression anchors the dismissal incident to the symbol; dedup + the Wilson
vote grouping become per-(pitfall, file, symbol).
- rankFindings' learnedSuppression applies a symbol-scoped decision only at a matching
file#name (RankOptions.suppressions → Map<tag, {tier, repoWide, symbols?}>).
- Deterministic findings now carry their enclosing symbol (analyzeSource enclosingSymbol)
so a codified rule like no-console can be symbol-scoped, not just agent findings.
- acknowledge/waive tightened the same way: Waiver.symbol + a waiverMatches symbol gate on
the file/line scopes (symbol-less waivers keep pure file/line matching; pattern/category
semantic scopes untouched).
Backward-compatible by construction (every pre-ADR-48 dismissal is symbol-less → repoWide;
no migration). The Wilson "weighted, never a one-click kill" property is preserved.
Tests: rank/waivers symbol-gate + back-compat units, suppression per-(file,symbol) dedup +
scope derivation, builtin/runner enclosing-symbol, and a new `suppress-scope` integration
scenario (dismiss run+handler → scoped to exactly those, extra still surfaces). Docs:
ADR-48, negative-knowledge.md, code-path-memory.md, package guides.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e + widen enclosing-symbol (PR #25 review) Address the Plex self-review of PR #25: - BUG: dedupeFindings kept the first-inserted finding's location wholesale and never reconciled location.symbol on merge. With the engine's `[...agent, ...det]` order, an agent finding (usually symbol-less) merging over its colliding deterministic `no-console` twin stripped the enclosing symbol → the finding became symbol-less → unsuppressable, defeating symbol-scoping on the exact headline case. Now the merge prefers a present symbol over an absent one (order-independent). - enclosingSymbol now also resolves object-literal function properties ({ onTap: () => … }) and class fields (f = () => …), not just const bindings — common in handler maps. - Document two accepted-for-v1 scope-vs-evidence asymmetries the review flagged (both err toward surfacing): scopeOf fails open per-pitfall on any symbol-less dismissal (back-compat, sticky); corrections are counted pitfall-wide vs per-(file,symbol) dismissals. (ADR-48.) The NUL-separator nit was a false positive — countsOf already uses `\0`, not a space. Tests: cross-source symbol-merge (both orders) in rank.test.ts; enclosing-symbol across declaration shapes (function/method/arrow/object-property/top-level) in builtin.test.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What changed
Location-scoped suppression (ADR-48) — the negative twin of code-path memory (ADR-47).
A dismissal was repo-wide: dismissing one intentional
console.log("it's the CLI logger") taught Plex to demote/suppressno-consoleeverywhere, silently burying the next genuinely-stray one. Now a dismissal anchors to thefile#namesymbol it concerned and scopes the suppression to it — the dismissed instance stays quiet, but the same rule still surfaces at every other symbol.submitVerdictresolves the dismissed finding'ssymbol/linefrom the brain finding (hoisted aboverecordVerdict/learnSuppressionso reject/waive/acknowledge all inherit it); the drift-stable dedup and the Wilson vote grouping (countsOf) become per-(pitfall, file, symbol).SuppressionDecisiongainsrepoWide+symbols;loadSuppressionsfails open torepoWidefor legacy / symbol-less / explicitpattern-repo/category-*dismissals.rankFindings'slearnedSuppression(nowRankOptions.suppressions: Map<tag, {tier, repoWide, symbols?}>) suppresses a symbol-scoped decision only at the matchingfile#name.enclosingSymbolin the TS-AST walker) — the load-bearing piece, sinceno-console(the motivating case) is a deterministic rule and was symbol-less before, so it would have stayed repo-wide.acknowledge/waivetightened the same way:Waiver.symbol+ awaiverMatchessymbol gate on thefile/linescopes (symbol-less waivers keep pure file/line matching; pattern/category semantic scopes untouched).Backward-compatible by construction — every pre-ADR-48 dismissal incident is symbol-less → reads as
repoWide; a verdict with no resolvable symbol fails open the same way. No migration. The Wilson "weighted, never a one-click kill" property is preserved (symbol-scoping rides on top of the tier).API / schema changes
core:Waiver.symbol?(new optional field).findings:RankOptions.suppressionsvalue type'suppress'|'demote'→LearnedSuppression {tier, repoWide, symbols?}(exported).engine:SuppressionDecisiongainsrepoWide+symbols?;loadSuppressionsnow exported from the engine barrel.v1 limits (deferred)
First-principles (embedding-keyed) and cross-repo C2 suppressions stay repo-wide (their identity is a title embedding / a cross-project rule, not a symbol); line-level granularity is out (symbol is the unit); the deterministic enclosing-symbol is the nearest named declaration (two same-named methods in one file could collide — rare, accepted).
How to test
pnpm typecheck·pnpm test:unit(545) ·pnpm test:integration(19 scenarios, incl. the newsuppress-scope) ·pnpm build·pnpm test:brain— all green.record_outcome reject(with anote) on aconsole.logfinding via the MCP tool, then re-review — the dismissed symbol stays quiet, a newconsole.logelsewhere surfaces.Docs
ADR-48;
docs/design/negative-knowledge.md(§Location scope);docs/design/code-path-memory.md(negative-twin cross-link); core/engine/findings/deterministic package guides; root scope list.🤖 Generated with Claude Code