JPHutchins / JPHutchins/code-review

Durable, channel-agnostic code-keyed adjudication ledger (answered-state beyond inline threads)

Open
#175 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

deferred enhancement
Dominant language
TypeScript
Stars
2
Forks
0
Avg merge
4h 9m
Merged PRs (30d)
18

Description

[!WARNING]
LLM Disclosure

This 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's code (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 own code-review:findings-jsonGET /pulls/{n}/reviews is 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:

  1. 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 code or its 1-based index to that finding — feeding the same applyAnswered/seed machinery. Cheapest; catches the common "reply by code/index in the PR conversation" case. Risk: false matches on incidental mentions of a code.
  2. A durable code-keyed adjudication ledger carried in the sticky's machine channel (a marker, like rounds), recording per-code disposition (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.
  3. 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.