review: adjudicated-corpus suppression drops its same-path key

[jwbron]

The adjudicated-corpus suppression was path-keyed, and that key is what let the webapp#41290 duplicate families re-post for two weeks after the author had adjudicated them: a settled defect's rephrasing routinely re-anchors on another file (the spec instead of the implementation, the test instead of the function), and every re-anchoring escaped the corpus. So let's just drop the same-path key in the adjudicated pass only.

The change is measured, not argued. On the frozen 41290 corpus (12 adjudicated threads, 33 hand-labeled candidates: true variants, vocabulary-sharing hard negatives, controls; kept privately in the planning tree since it's verbatim webapp text):

• path-keyed: 2/12 true variants suppressed, 1 false suppression (a same-path pair folding two distinct findings whose wording shares the file's vocabulary, a mistake the current matcher makes too)
• key dropped: 6/12 at the same single false suppression, correct family attribution on every match, controls clean

Tripled recall, zero added false suppressions. The remaining 6 escapees are heavily reworded members token floors can't reach at any key width; they're deliberately not chased (the corpus memory self-heals: an escapee that posts gets downvoted or acknowledged once and joins the corpus itself).

What licenses the wide match is the corpus's membership rule plus the blocking exemption: every member carries an explicit human act (a resolve or a 👎), a suppression here only ever eats a non-blocking candidate, and a defect that matters enough to block re-presents at blocking severity and posts. Everything else is unchanged: the open-thread corpus stays path-keyed (there a false cross-file match hides an undecided finding), and the other #332 fail-closed guards hold. Mechanically it's an ignorePath option on bestOpenThreadMatch whose only caller is the adjudicated pass, plus tests pinning the cross-file suppression, the blocking exemption, and the open-corpus asymmetry.

The review round tightened four things: an adjudicated thread staged without a usable path now stays inert under ignorePath instead of becoming a PR-wide matcher; the cross-file floor choice is documented as measured (every cross-file negative on the corpus fails jaccard at 0.168 against the 0.2 floor, the weakest true match sits at 7 shared bigrams, so the pr-level tier's floor of 8 would cost a true variant for no measured precision) and pinned by a fixture in the 6-8 bigram band where the two floors disagree; the interaction with the cross-file merge ordering is documented as deliberate and pinned in both directions (an open thread takes only its own file's copy and the sibling posts alone, an adjudicated thread takes the near-identical sibling copy too); and the changeset bumps review as a minor, matching the 1.17.0 precedent. The exact final diff was re-validated against the private fixture through the production pass: recall 6/12, the same single false suppression, correct attribution throughout.

KORE-2468

[changeset-bot]

🦋 Changeset detected

Latest commit: 80a7632

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
review	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Review live A/B

No reviewable delta: review.md is byte-identical in both arms (baseline origin/main, sha 6ceb1601f14e), so the extracted prompts and the orchestrator body match and no arms were run. Pass --force-arms for a deliberate wobble control.

[github-actions]

Changes requested — see inline comments.
Note: claim validation not assessed this run (claim-validator output unavailable).

_{review details}

_{review-v1.17.1 | schema 2 | depth full | re-review scoped blocking-only | enable holistic,completeness,test-adequacy,first-principles,conventions,documentation}

[jwbron]

Pushed 33479df addressing the round; inline replies aren't available from this session, so per finding:

The changeset (both blocking comments plus the conventions/documentation echoes): frontmatter now bumps review as a minor. On the level, the 1.17.0 precedent is the closer authority than the major-version line in the README: the downvote-adjudication change was the same kind of suppression-behavior widening and shipped as a minor, and this change only ever affects non-blocking candidates, so a consumer pinned to review-v1 keeps the contract that matters (what can block).

The cross-file floor (holistic/correctness/first-principles): turns out the corpus answers this directly, so I re-derived instead of arguing from the convention. Every cross-file negative pair on the frozen 41290 corpus fails jaccard, not bigrams: the strongest scores 0.168 against the 0.2 floor while reaching 13 shared bigrams, so jaccard is the guard that holds and a bigram-keyed CROSS_FILE_FLOOR polices the wrong axis. The weakest true cross-file match sits at exactly 7 shared bigrams, so the pr-level tier's floor of 8 drops recall from 6/12 to 5/12 and suppresses zero additional negatives. The floor stays at OTHER_LINE_FLOOR, but the calibration is now written into bestOpenThreadMatch's doc (re-derive, don't nudge, same as the other tiers) and pinned by a new fixture scoring 7 bigrams, inside the band where the two floors disagree, which also covers the test-adequacy suggestion about both floors being cleared by a wide margin.

The suppress-before-merge ordering (the correctness blocker): the interaction is real and it's deliberate, so it's now explicit rather than implied. The ordering rationale still holds for the open corpus, which stays path-keyed. For the adjudicated corpus, a settled thread on file A taking B's near-identical copy is the corpus's semantics: the human declined that exact ask once, the reach is capped at non-blocking, and a blocking re-presentation posts. Note the path key never protected the matching same-file case either; a fresh same-file instance of a settled defect was already suppressed, so this widens the reach of an accepted risk rather than adding a new failure class. mergeCrossFileDuplicates's ordering doc now says all of this (which also clears the stale-premise note), and dedup-crossfile.test.ts pins both directions: open thread on A suppresses A's copy and B posts alone; adjudicated thread on A takes both copies and nothing reaches the merge.

The anchorless-thread suggestion was a genuine catch: under ignorePath a corpus member staged without a path would have become a PR-wide matcher, flipping openThreadsFromStaged's documented fail-closed degradation open. The thread side now keeps a gate (no usable path, no match), tested for both the undefined and empty-string shapes, and the changeset names the guard.

The rest:

• best-match attribution across files has its own test now: two corpus members on different files both clear the floor, the higher-jaccard one wins, and the reversed staging order is asserted as a control
• the asymmetry sentence was circular as written, agreed; rewritten so the licensing is the membership rule (an explicit human act) plus the blocking exemption, not a claim that false matches are free
• the README's resolve bullet now says any wording, any line, any file, with the re-anchoring reason
• the module-header pointer and the coined "vocabulary-split" are fixed; the false pair folds two distinct same-file findings whose wording shares the file's vocabulary, and the doc, changeset, and PR body all spell it out now
• the pathless-claim doc branch: reworded so bestOpenThreadMatch's doc states the only ignorePath caller exempts pathless claims first, making OTHER_LINE_FLOOR the only live floor there

On the private corpus (the reproducibility question): it's verbatim webapp thread text and it leaked into this public repo once already on a deleted branch, so it stays in the planning tree. What's re-runnable here: the qualitative behaviors are pinned as fixtures (now including the floor band and both ordering directions), and the doc carries the exact numbers as the re-derivation target. The final diff was re-validated by running the frozen corpus through the production adjudicatedThreadsFromStaged + suppressAdjudicatedDuplicates path: recall 6/12, the same single same-path false suppression, correct family attribution on every match.

[github-actions]

Guidance for reviewers

Triage notes for reviewers: risky files by owning team, repeated changes, and files excluded from review.

github-actions (6 files)

File	Reason
dedup-adjudicated.ts	Carries the hunk that actually widens suppression ({ignorePath: true}), so a mis-calibration silently eats non-blocking findings repo-wide rather than only on the settled file.
dedup-threads.ts	New home of bestOpenThreadMatch, the matcher both suppression corpora run through, and the file carrying the new ignorePath branch; a bug here decides what is dropped before validation sees it.
dedup-text.ts	Now owns the calibrated similarity floors every dedup and suppression tier reads, so it is the single point where a nudged floor changes suppression on every consumer PR.
dedup.ts	The ~460 removed lines are a verbatim move to dedup-threads.ts/dedup-text.ts, so the residual risk is a dropped export rather than a rule change.
dedup-crossfile.ts	Enforcement layer, but the change is import rewiring to dedup-threads.ts plus the ordering-rationale doc, so a mistake surfaces as a broken import that typecheck catches.
dispatch.ts	The dispatcher's re-export surface that downstream lib modules and tests import from; the change is a re-export split with the same symbol set.

Common patterns

4 files: Import path repointed from ./dedup to ./dedup-threads for open-thread-suppression exports (suppressOpenThreadDuplicates, openThreadsFromStaged, stagedThreadShapeFailure, threadSuppressionUnavailableWarning, ThreadSuppression) following the module split.

- import {suppressOpenThreadDuplicates} from "./dedup";
+ import {suppressOpenThreadDuplicates} from "./dedup-threads";

5 files: Comment/doc cross-references to dedup.ts updated to dedup-threads.ts following the module split (sole change in each file).

- * text-similarity comparison (dedup.ts's `threadProse`). Every posted
+ * text-similarity comparison (dedup-threads.ts's `threadProse`). Every posted

Excluded from review (8 files)

Not individually reviewed — generated, formatting-only, or
fully explained by a common pattern above:

• workflows/review/lib/attribution.ts — pattern-only
• workflows/review/lib/dedup-cluster.ts — pattern-only
• workflows/review/lib/dedup-pr-level.test.ts — pattern-only
• workflows/review/lib/dedup.test.ts — pattern-only
• workflows/review/lib/forwarded-warnings.ts — pattern-only
• workflows/review/lib/stage-pr.ts — pattern-only
• workflows/review/lib/stage-threads.test.ts — pattern-only
• workflows/review/lib/threads.ts — pattern-only

_{review details}

_{review-v1.17.1 | schema 2 | depth full | re-review scoped blocking-only | enable holistic,completeness,test-adequacy,first-principles,conventions,documentation}

[github-actions]

All 16 prior review threads are resolved.
Note: divergence tripwire re-armed a full review (unreviewed share 0.97).

_{review details}

_{review-v1.17.1 | schema 2 | depth full | re-review scoped blocking-only | enable holistic,completeness,test-adequacy,first-principles,conventions,documentation}

[github-actions]

suggestion (non-blocking): The cross-file precision guard (jaccard ≥ 0.2) is asserted in prose but pinned by no fixture, while the bigram side got one. Introduced by this change. Before it, the same-path key meant the text floors only ever had to separate findings within one file, so pinning them against same-path fixtures was sufficient. Dropping the key on the adjudicated pass (bestOpenThreadMatch(claim, threads, {ignorePath: true}), dedup-adjudicated.ts:166) makes the text floors the only remaining guard — there is no path key and no line window left — and bestOpenThreadMatch's new doc names which of the three does the work: "the strongest cross-file NEGATIVE scores jaccard 0.168 against the 0.2 floor (its bigram counts reach 13, so jaccard is the guard that holds)" (dedup-threads.ts:266-268).

The added suite pins the positive margin carefully — the marginal fixture at 7 shared bigrams in the 6-8 band where OTHER_LINE_FLOOR and PR_LEVEL_FLOOR disagree — but nothing pins the negative one. I checked: the only negative in suppressAdjudicatedDuplicates's tests is the unrelated AddDate claim, and it inherits claim()'s default path, i.e. it is same-path and clears the floor by a wide margin; grepping the whole workflows/review/ tree, 0.168 appears once, in the dedup-threads.ts comment, and in no fixture. The corpus that establishes it is private (Khan/plans), so it is unreachable from CI. Meanwhile dedup-text.ts:59-62 tells the next editor to "re-derive them from dedup.test.ts's fixtures" — which contain no cross-file case at all.

Worth adding the symmetric fixture: a claim on a sibling file whose wording shares the settled thread's vocabulary heavily enough to reach the bigram floor but not the jaccard one, asserted to stay in kept. That converts the strongest documented cross-file negative into something CI can defend, and it is the case that fails silently — a false cross-file suppression drops a finding with no trace, whereas a missed one only costs a duplicate comment.

Lower-confidence observations (3)

• workflows/review/lib/dispatch.ts:43 suggestion (non-blocking): Pure type-only imports use import {type X} where the lib writes import type {X}. _{(conventions)}
• workflows/review/lib/dedup-threads.ts:267 thought (non-blocking): "Zero added false suppressions" is measured on one PR's 12×33 grid, but dropping the path key makes the risk scale with PR breadth. _{(first-principles)}
• .changeset/pra11-adjudicated-crossfile.md:2 suggestion (non-blocking, best-practice): Versioning — semver is the reviewer's behavior contract. _{(skill-auditor)}

_{review details}

_{found by correctness-reviewer}

[github-actions]

suggestion (non-blocking, documentation): Moved PR_LEVEL_FLOOR doc still says "this module scores", but the scoring left this file. The moved block says the pr-level tier is "the least anchor evidence this module scores, so it pays with the highest bigram floor", yet the file it now lives in scores nothing — it ends at export const PR_LEVEL_FLOOR = {jaccard: 0.2, overlap: 0.35, sharedBigrams: 8};, while openThreadScore and the claim.path === undefined ? PR_LEVEL_FLOOR : OTHER_LINE_FLOOR choice moved to the new dedup-threads.ts. The module header above was updated for the split; this sentence's "this module" was carried over unchanged.

A sketch, not a committable replacement:

 * an open thread: the least anchor evidence dedup-threads.ts scores, so it pays

_{review details}

_{found by documentation}

[github-actions]

suggestion (non-blocking): The widened adjudicated match has no cross-file hard-negative fixture, so only the recall side is pinned. Every other property this PR argues is pinned (dedup-adjudicated.test.ts:265-362: cross-file recall, the blocking exemption, the 7-bigram floor-tier band, the anchorless-thread gate, the open-corpus asymmetry), but the "zero added false suppressions" half is not: the only negative in the file, "keeps unrelated and pathless claims" (line 177), is a SAME-path AddDate claim sharing essentially no bigrams with the thread, so it is rejected by sharedBigrams >= 6 and never touches the jaccard guard. grep -rn ignorePath shows dedup-adjudicated.ts:166 is the sole caller, and 0.168 appears only in dedup-threads.ts:267's prose — the strongest cross-file negative (13 shared bigrams, jaccard 0.168 against a 0.2 floor) lives in a private corpus, so nothing in CI holds that 0.032 margin.

Non-blocking, because the gap bites only if a later edit loosens the floor rather than in this diff. One caution if you add the fixture: it has to be built against the real tokenizer to land in the band where only jaccard rejects (>=6 shared bigrams, >=0.35 overlap, <0.2 jaccard). A candidate that merely reads as a different defect in the same domain gets rejected on bigrams instead and pins nothing — I measured one such sketch at jaccard 0.065 / overlap 0.167 / 1 shared bigram against adjudicatedThread().

A sketch, not a committable replacement:

    it("keeps a cross-file hard negative: vocabulary overlap alone does not clear the jaccard guard", () => {
        // The precision half of the widening. This candidate is a DIFFERENT
        // defect that shares the file's vocabulary (expired/memories/
        // deletion/TestExpiration) heavily enough to clear the bigram and
        // overlap floors; only jaccard rejects it, which is the guard the
        // frozen 41290 corpus measured at 0.168 against the 0.2 floor.
        const negative = crossFile({
            subject: "The expiration sweep holds the memories lock while deleting.",
            discussion:
                "Expiration takes the write lock for the whole deletion pass over expired memories, so readers of the memory map block for the length of the sweep instead of per key.",
            failure_scenario:
                "A large batch of expired memories stalls every concurrent reader of the memory store until the deletion pass finishes.",
        });
        const {kept, suppressed} = suppressAdjudicatedDuplicates(
            [negative],
            [adjudicatedThread()],
        );
        expect(kept).toEqual([negative]);
        expect(suppressed).toEqual([]);
    });

_{review details}

_{found by test-adequacy}

[github-actions]

note (non-blocking): The description doesn't mention the dedup.ts module split that makes up most of the diff. I checked the move rather than assuming: dedup.ts was 1021 lines before this PR against the repo's 1000-line max-lines budget, so the split follows the established dedup-cluster.ts/dedup-text.ts precedent, and grepping from "./dedup" shows no file still importing a moved symbol — the refactor looks clean and justified. It's the disclosure that's missing: the behavior change is one line, and everything else in the diff is a structural move the reader is not told to expect. No Jira/Confluence access was available here, so this is judged against the PR description alone.

_{review details}

_{found by completeness}

[github-actions]

suggestion (non-blocking): OTHER_LINE_FLOOR's definition-site doc still describes only its same-file calibration, while this PR quietly makes it load-bearing for a cross-file corpus calibrated elsewhere. This change moves the floors into the shared dedup-text.ts and, in the same diff, adds a third consumer whose calibration basis is different (a private frozen corpus: cross-file negative at 0.168 jaccard, weakest true match at 7 bigrams — documented only in bestOpenThreadMatch's doc in dedup-threads.ts). PR_LEVEL_FLOOR, right below it, does name its own calibration test; OTHER_LINE_FLOOR now has two calibration bases and names one, so the "re-derive, don't nudge" instruction at the definition site is incomplete in the direction that loses findings. Verified by grepping every OTHER_LINE_FLOOR reference: dedup.ts:199 (merge tier), dedup-threads.ts:221 (same-file suppression) and the new cross-file use, with only the first cited at the definition.

A sketch, not a committable replacement:

 * Every floor is a minimum over real trial claims, so the margins are thin
 * by construction: the exact-anchor tier separates on bigrams 4 vs 3, the
 * other-line tier on 6 vs 5. Re-derive them from `dedup.test.ts`'s fixtures
 * rather than nudging them by feel.
 *
 * OTHER_LINE_FLOOR carries a SECOND calibration since the adjudicated pass
 * dropped its path key: it is also the cross-file floor for
 * `bestOpenThreadMatch({ignorePath: true})`, calibrated on the frozen
 * webapp#41290 family corpus (strongest cross-file negative at jaccard
 * 0.168 against the 0.2 floor; weakest true match at exactly 7 bigrams).
 * `dedup-adjudicated.test.ts` pins only the bigram edge, so a jaccard or
 * overlap change here must be re-derived against that corpus too — see
 * `bestOpenThreadMatch`'s doc.

_{review details}

_{found by holistic}

[github-actions]

suggestion (non-blocking): Trailing options bag is options?: where the lib defaults it (options: {...} = {}). Every other trailing options bag in workflows/review/lib is a defaulted parameter read without optional chaining — options: {dryRun?: boolean} = {}, (run-thumbs-sweep.ts:73), options: ProvenanceGateOptions = {}, (provenance.ts:427, read as options.anchorSnap ?? true), options: CheckOptions = {}, (check-consumer-config.ts:362, read as options.repoRoot ?? ".") — while this adds options?: {ignorePath?: boolean}, and then has to read it as options?.ignorePath !== true at line 290.

Suggested change

	options?: {ignorePath?: boolean},
	options: {ignorePath?: boolean} = {},
	// and at the guard:
	// (!options.ignorePath
	// ? claim.path !== undefined && thread.path !== claim.path

_{review details}

_{found by conventions}