JPHutchins / JPHutchins/code-review
Durable, channel-agnostic code-keyed adjudication ledger (answered-state beyond inline threads)
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 2
- Forks
- 0
- Avg merge
- 4h 9m
- Merged PRs (30d)
- 18
Description
[!WARNING]
LLM DisclosureThis issue was authored by claude-opus-4-8 on behalf of @JPHutchins, capturing JP's observation while we designed the nit-visibility-floor stickiness: the "already answered" anchor only catches inline-thread replies, but implementers dismiss findings in whatever channel they like, keyed by the finding's index or
code— so the anchor is not durable and the re-raise trap #151 targeted is only half closed.
Executive Summary
#151 closes "dismissed findings get re-raised" by remembering the human's dismissal — but it anchors that memory to inline review-comment threads only (fetchThreadComments). Implementers rarely reply on the inline thread. They reply in whatever channel is in front of them — a top-level PR comment, a reply under the sticky, a commit message, the review-summary conversation — and they refer to the finding by its position in the findings array or the reviewer-invented code, not by threading a reply onto one inline comment. Worse, strays and systemic problems have no inline thread at all, so a dismissal of one is unrepresentable in the current anchor.
The result: the "answered" state is only partially durable. A dismissal in the wrong channel is invisible next round, and the finding comes back — the exact loop #151 set out to kill.
The durable fix is a channel-agnostic, code-keyed adjudication ledger in the machine channel — so "this was adjudicated, and here's the disposition" survives regardless of where or how the human expressed it. #164's nit-visibility stickiness is a working instance of the pattern (code-keyed, blob-derived, channel-independent — it needs no human reply to locate, because the bot itself made the call).
Evidence — the anchor misses the common case
- #151's own driving instance: on salix#100 the author dismissed with measured evidence across rounds; the round-9 dismissal was a top-level
#issuecomment(issuecomment-5243600862), not an inline thread reply — invisible to an inline-thread scrape. #151's spec text itself says the match is "When a finding'scode(or title) matches a prior round where the author replied with an explicit dismissal" — code-keyed, not thread-keyed. - #98:44 already established the durable substrate: "The prior rounds are already on the PR and need no new store. Each round is submitted as a
PullRequestReview… carrying its owncode-review:findings-json…GET /pulls/{n}/reviewsis already the durable history, for both the findings and the SHA each was judged against." The history is channel-agnostic and durable; what's missing is a disposition keyed to it. - Strays / systemic problems have no inline thread. A maintainer who writes "the systemic caching observation is intentional" under the sticky has no inline comment to thread onto; the current anchor cannot record that dismissal at all.
Design directions (exploration — not a committed shape)
The hard part is that a dismissal is a human decision, and the human's expression of it can land anywhere. Candidate approaches, roughly increasing in ambition:
- Widen the scan. In addition to inline threads, read the channels #98 already gathers (top-level issue comments, review summaries) and the reviews API, and match any human message that references a finding's
codeor its 1-based index to that finding — feeding the sameapplyAnswered/seed machinery. Cheapest; catches the common "reply by code/index in the PR conversation" case. Risk: false matches on incidental mentions of a code. - A durable
code-keyed adjudication ledger carried in the sticky's machine channel (a marker, likerounds), recording per-codedisposition (answered/declined/policy-suppressed/accepted) and a link to where the human said so. The seed delivers it; post re-derives/updates it. Generalizes #164's nit-stickiness from "policy-suppressed nits" to "any adjudicated finding," and does not depend on re-finding the reply each round. - A dismissal convention the human opts into (e.g. a reply containing the finding's
code), so the signal is unambiguous and channel-independent by construction — documented in the sticky's "how to consume/respond" contract (ties to #171's "read the schema/markers" push).
Whatever the shape, the invariant from the nit-floor work holds: adjudication state that is only remembered in one fragile place (an inline thread) is re-litigated whenever the human used a different place. Key it by code (else index/title, the existing answeredNoteKey contract) in a durable channel.
Relationships
- Generalizes #151 (answered-state; currently inline-thread-only) and builds on #98 (author-reply context; gathers the conversation but treats it as soft context, not durable adjudication).
- Overlaps #160 (recurrence signals in the machine channel) and #172 asks 3–5 (category/
code-keyed recurrence and claim tracking) — the same "key it by the reviewer's stable identifier, durably" theme. - The concrete, already-shipping instance of the pattern is #164 (nit-visibility stickiness).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the answered-state path described in #151, especially fetchThreadComments and the applyAnswered/seed machinery. Compare it with #98's PullRequestReview history and #164's machine-channel stickiness. Done means selecting and documenting a durable, code-keyed adjudication design that covers non-inline channels without re-raising settled findings.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github, typescript
- Domain
- developer-experience, devtools
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100