Skip to content

review: a downvoted finding joins the adjudicated corpus; document the feedback contract - #333

Merged
jwbron merged 7 commits into
mainfrom
jwies-review-downvote-adjudication
Aug 13, 2026
Merged

review: a downvoted finding joins the adjudicated corpus; document the feedback contract#333
jwbron merged 7 commits into
mainfrom
jwies-review-downvote-adjudication

Conversation

@jwbron

@jwbron jwbron commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Stacked on #332 (base is its branch; retarget to main after it merges).

What

Two of the remaining items from the review-bot feedback effort:

  1. Downvote adjudication. A 👎 on a bot thread's OPENING comment now adjudicates its defect exactly like resolving the thread. The staging fetches each opener's THUMBS_DOWN totalCount over the same GraphQL threads query and stages any bot-opened thread with a downvoted opener into adjudicated-threads.json, whatever its resolution state; the corpus filter admits an explicit positive openerDownvotes count as an alternative to human resolution.
  2. The feedback signal contract, documented. A "What your feedback does" README section: what reply / resolve / 👎 / hide each mean to the reviewer, plus the skip-ai-review label note. Users have been calibrating against the frontend bot's different contract (Slack: "thumbs-down and resolve and it'll assume it's wrong"), and in webapp the reaction channel dead-ended in counters; this makes the real contract discoverable and, with (1), true.

Why

The "downvoting the comments does nothing" report, verbatim: nothing in the live review loop read reactions. The thumbs sweep collects them on a schedule and asks "why?", but its output never reached suppression, so the one feedback affordance the bot itself advertises had no effect on what posts.

Semantics

  • Reactions on REPLIES are conversation, not adjudication: only the opener's count is read (the opener is the finding).
  • A downvote never launders a thread that fails the bot-opener guard; absent, zero, or malformed counts read as no downvote (fail closed toward posting a duplicate).
  • A blocking candidate is still never suppressed by the adjudicated corpus.
  • A still-open downvoted thread stays in the open corpus too; the composed pass attributes a double match to the OPEN thread, keeping the verdict-floor bookkeeping.

Tests

Corpus admission via downvote across resolution states; the zero/malformed/human-opener rejections; staging a downvoted open thread into BOTH files with no reaction leak into threads.json's exact shape. Full workflows/review suite green; tsc and eslint clean.

Follow-up (not this PR)

Parsing the thumbs sweep's reason-vocabulary replies (incorrect/duplicate) as an additional adjudication source; the 👎 itself is the reliable carrier (the sweep only asks after one), so this PR takes the signal at its source.

…s the adjudicated corpus; document the feedback contract

A 👎 on a bot thread's opening comment is the same judgment as
resolving the thread, delivered through the other feedback channel the
bot advertises (the thumbs sweep asks why? on exactly this signal),
and until now it dead-ended in counters: nothing in the live review
loop read reactions at all, which is the 'downvoting the comments does
nothing' report from the review-bot feedback thread, verbatim.

The staging fetches each opener's THUMBS_DOWN totalCount over the same
GraphQL query and stages any bot-opened thread with a downvoted opener
into adjudicated-threads.json, whatever its resolution state. The
corpus filter admits an explicit positive openerDownvotes count as an
alternative to human resolution; every other guard is unchanged, so a
downvote never launders a thread that fails the bot-opener check, a
blocking candidate is still never suppressed, and a still-open
downvoted thread remains in the open corpus, whose match keeps the
verdict-floor bookkeeping (the composed pass order attributes a double
match to the open thread).

Also adds the 'What your feedback does' README section: what reply,
resolve, 👎, and hide each mean, and the skip-ai-review label note,
so the contract is discoverable rather than tribal (users have been
calibrating against the frontend bot's different behavior).
@changeset-bot

changeset-bot Bot commented Aug 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1794e34

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

@khan-actions-bot
khan-actions-bot requested review from a team, jaredly and kevinb-khan and removed request for a team August 11, 2026 14:45

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes requested — see inline comments.

Comment thread workflows/review/lib/threads.ts Outdated
Comment thread workflows/review/lib/threads.ts Outdated
Comment thread workflows/review/README.md
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review live A/B

Baseline: origin/jwies-review-adjudicated-suppression (review.md 524e646e7e53); candidate: working tree (review.md d6a1e6ff9bea).

Ruler: matcher deterministic+arbiter; corpus cc11988c0918 (9 cases).

Metric Baseline Candidate Delta
Must-catch recall 100% 100% +0%
Verdict agreement 89% 89% +0%
Noise (unmatched posted) 53% 47% -6%
Clean false flags 0 0
Judge mean quality 0.89 0.89 +0.00
Cost $8.83 $9.30
Wall clock 1296s 1250s
Cases run / skipped 9 / 0 9 / 0
Misses found-but-dropped 0 0
Findings anchor-snapped 0 0
Cross-source claims merged (of candidates) 5 / 24 (0 by clusterer at $0.24 / 85s, 4 proposed member(s) rejected) 3 / 20 (0 by clusterer at $0.20 / 61s, 6 proposed member(s) rejected)

Adversarial hard gate: PASSED on the candidate arm.

Single-run-stable rows: recall, verdict agreement, regressions, adversarial gate. Judge quality and noise are not: they jitter run-to-run at this corpus size, and a regressed reviewer can score HIGHER on judge quality (fewer, surer comments each read better). Recall against the labeled specs is the load-bearing metric.

Measured noise floor (identical arms, run 29069228968, 2026-07-10, 6 arm-samples, full corpus x3, pre-arbiter; budget skips left the samples on unequal case sets, so these v1 bands also carry case-mix variance): must-catch recall 54%-86% (sd 10%), verdict agreement 75%-100% (sd 9%), noise (unmatched posted) 50%-60% (sd 3%), judge mean quality 82%-86% (sd 2%). A single-run delta whose arms both sit inside a band is indistinguishable from run-to-run wobble; use --repeats to resolve smaller effects.

…igin/jwies-review-adjudicated-suppression' into jwies-review-downvote-adjudication
@khan-actions-bot
khan-actions-bot requested a review from a team August 11, 2026 15:42
jwbron added 2 commits August 11, 2026 11:44
…ity primitives out of dedup.ts

The merge with #328 pushed dedup.ts past the max-lines budget (1008).
contentTokens, bigrams, and intersectionSize are dependency-free and
shared by every similarity floor, so they move to dedup-text.ts (the
dedup-cluster.ts precedent), with the calibration-coupling warning in
the module doc. No behavior change; the floors and their calibration
notes stay in dedup.ts beside the rules they price.
…igin/jwies-review-adjudicated-suppression' into jwies-review-downvote-adjudication
jwbron added a commit that referenced this pull request Aug 11, 2026
…locking-only re-reviews (#340)

🖍 _This is an audit!_ 🖍

## What

Two coupled changes, one install:

1. **Bumps this repo's installed PR reviewer from `review-v1.12.0` to `review-v1.13.0`.** 3-way merge (base = v1.12.0 release source, ours = installed copy, theirs = v1.13.0 release source) with one trivial conflict at the KHAN/ACTIONS LOCAL OVERRIDE comment that rides the checkout `ref:`, then `gh aw compile review`. `source:` and the lib checkout `ref:` stay in lockstep at `review-v1.13.0`; the lock diff is purely version refs plus recompile hashes.
2. **Opts this repo into the new dial**: the ROUTING re-review line becomes `re-review scoped blocking-only` (scoped was already enabled). Repeat reviews keep the full scoped roster, so blocking recall is unchanged; only blocking findings post inline, and validated non-blocking findings collapse to one line each in the review body. First full reviews, divergence-tripwire re-arms, and guard-degraded full runs still post everything.

## What v1.13.0 brings over v1.12.0

- **#328**: pr-level (pathless) claims respect open-thread suppression (stricter `PR_LEVEL_FLOOR` paying for the missing anchor), and long pr-level body folds collapse past 400 chars.
- **#329**: the `blocking-only` re-review modifier this PR enables, with the cap-overflow neutral wording and the set-membership modifier parse.
- **#336**: the clusterer prompt example no longer promises cross-file grouping the same-path rule forbids.

## Configuration audit

No other consumer configuration is needed: v1.13.0 adds no imports, no safe outputs, and no `.github/aw/` config files (verified against the v1.12.0→v1.13.0 diff; the prompt delta is 9 lines). The modifier is consumer-side ROUTING parse in the same release, so the bump and the opt-in belong in one PR: on v1.12.0 the extra field would not parse, and without the ROUTING edit the bump is behavior-neutral.

Part of the review-bot feedback effort; the adjudication work (#332/#333/#334) lands in the next release and a follow-up bump.

Author: jwbron

Reviewers: jeresig, somewhatabstract

Required Reviewers:

Approved By:

Checks: ⌛ 1 check is pending, ✅ 4 checks were successful

Pull Request URL: #340
jwbron added 2 commits August 11, 2026 10:16
…eads become an adjudicated suppression corpus

Resolving a bot thread previously removed it from the only suppression
corpus (threads.json stages unresolved threads only), so the next run
was free to re-derive the same defect with fresh wording as a brand-new
thread, which every later accountability recap then enumerated as
still unaddressed. Observed on webapp#41290: the author replied to and
resolved six variants of one concern at moderation_helpers.go:135 over
two days, and a seventh rephrasing posted anyway.

The staging now fetches resolution state (isResolved + resolvedBy) and
writes adjudicated-threads.json: bot-opened threads a HUMAN resolved.
The dispatcher runs a second suppression pass over it after open-thread
suppression, with two deliberate asymmetries: a thread the bot itself
resolved (the reconciler marking a defect fixed) never joins the
corpus, and a blocking candidate is never suppressed by it, so a
fixed-then-regressed defect worth stopping the PR for always posts.
Every guard fails closed toward posting a duplicate, never toward
dropping a finding; an older staging without the file degrades to an
empty corpus.

Split per the max-lines convention: the corpus filter, the suppression
pass, and the composed two-pass entry point live in
dedup-adjudicated.ts (dedup.ts exports its matcher internals); the
dispatch-level tests live in dispatch-adjudicated.test.ts.
…origin/main' into jwies-review-adjudicated-suppression
@jwbron
jwbron requested review from jeresig and sxkosone and removed request for jaredly and kevinb-khan August 11, 2026 17:40
…t reactors adjudicate via the opener's 👎

Addresses the re-review feedback on #333:

- openerDownvotes counted the opener's raw THUMBS_DOWN totalCount, so the
  planned post-time nudge seeding (the bot 👎-ing its own comments) would
  have put every seeded finding straight into the adjudicated corpus. The
  GraphQL query now fetches reactor identities and the count filters the
  bot (both login spellings) and unattributable reactors, mirroring the
  thumbs sweep's countDownvotes filter; unattributable is stricter than the
  sweep on purpose, matching resolvedBy's rule that an empty identity never
  reads as human adjudication.
- New tests pin the opener-only semantic (a reply's 👎 adjudicates nothing)
  and the reactor-identity filter.
- The README's adjudication bullet now says explicitly that 😕 triggers the
  sweep's follow-up question but never suppresses, so 👎 is the only
  adjudicating reaction.
@jwbron

jwbron commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

End-to-end observation of the loop this PR builds on, from the first post-deploy window in webapp (2026-08-11T16:00Z through 2026-08-12T19:35Z): sxkosone downvoted the opener r3761098824 on webapp#41290; the thumbs sweep posted its "why?" follow-up at 20:37:59Z and the closed-vocabulary reply duplicate landed at 20:39:31Z, 92 seconds later. That was the only reaction across all 66 bot comments in the window (57 inline, 9 guidance), which supports both the reason-vocabulary follow-up named in this PR and prioritizing nudge seeding on the consumer side.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All 3 prior review threads are resolved.
Note: correctness-reviewer not assessed this run (correctness-reviewer output unavailable).
Note: re-review ran at scoped depth (re-review mode scoped).

return false;
}
const login = str(reaction["user"]["login"]);
return login !== "" && !isReviewBotAuthor(login);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question (non-blocking): Is 'any attributable non-bot reactor' the right authority bar for adjudication, given reactions need only read access while resolving needs write? The PR's core premise is '👎 = the same judgment as resolving, through the other channel', but the two channels have different permission floors: I checked the new filter (threads.ts:356–361) and it admits any non-bot login with no scoping to PR participants, while GitHub only lets users with push access or the PR author resolve threads. The cost is bounded (blocking candidates always post, and removing the reaction un-adjudicates on the next staging), so this may be an acceptable trade for a low-friction feedback channel — but it deserves to be a deliberate choice, and the README's contract section could also state the undo path (remove the 👎) so a mistaken or contested downvote is recoverable by the same low-friction means.

A sketch, not a committable replacement:

If wider-than-resolve authority is intended, say so in the README's 👎 bullet (anyone who can read the PR can adjudicate; remove the reaction to undo). If not, scope the count to reactors who are the PR author, a reviewer, or otherwise hold write access — the GraphQL nodes already carry the login needed to check.

Base automatically changed from jwies-review-adjudicated-suppression to main August 13, 2026 01:21
@jwbron
jwbron merged commit 715dd91 into main Aug 13, 2026
20 of 22 checks passed
@jwbron
jwbron deleted the jwies-review-downvote-adjudication branch August 13, 2026 01:21
jwbron added a commit that referenced this pull request Aug 13, 2026
…s a corrected discussion erases

Merge resolution: main's suppressTrackedDuplicates (open + adjudicated
corpora, #332/#333) replaces the open-only pass inside
suppressThenMergeCrossFile; the cross-file merge still runs after both
suppression passes, and shapeFailure rides the composed result.

Blocking re-review finding: applyVerifications applies a validator
corrected.discussion by wholesale replacement, which erased the merged
survivor's 'Also applies to' line and lost the sibling file's finding
from all posted output. The line is now one renderer
(alsoAppliesLine) and reapplyCrossFileOccurrences re-builds it from the
merge records after validation; unit and dispatch-level tests cover the
corrected, untouched, and dropped-survivor shapes.
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