Found while running three review rounds on steve/mort#1536. Not a breakage —
the reviews themselves were accurate and caught real bugs — but there is
currently no way to determine which commit a given finding was produced
against, which undermines the one label the release flow depends on.
Evidence
1. Finding records carry no commit.list_findings returns:
No sha, no round, no timestamp. The store is also cumulative, so after fixing
round 1 I still saw all 8 of its findings listed alongside round 2's. To work
out what round 2 had actually found, I had to snapshot list_findings before
triggering it and diff the two by hand. Without that snapshot it would have been
guesswork.
2. The consensus comment is edited in place and accumulates. On #1536:
That single comment now contains both a round-1 finding (archive_url's status:"saved") and a round-2 one (_OPAQUE_VALUE_KEYS missing web_search's
snippet). Round 1's consensus as it stood at the time is gone — there is no
per-round artifact to point at when explaining what was addressed and when.
3. Neither comment names a sha. Scanning both bodies for sha-like tokens
finds only 1b634840 — a run id quoted inside a finding's excerpt of my own
code comments, not a commit reference.
4. Comment-triggered runs report the default-branch sha.@gadfly review
produces an Actions run with head_sha = e581dab6 (main), not the PR head. This
one is platform behaviour, not a gadfly bug — an issue_comment event
necessarily runs against the default branch, and the workflow correctly checks
out refs/pull/N/head regardless. I'm noting it only because it removes the
last place an observer could have recovered the reviewed commit, and so that
nobody chases it as a defect.
Why it matters
steve/mort gates merges on a ready-to-merge label meaning "CI green, gadfly
reviewed and addressed". Nothing records which commit was reviewed, so a
review of a stale head is indistinguishable from one of the current head. A PR
that gets three more commits after its last review still looks reviewed. CI
solves this with a per-commit status; gadfly currently has no equivalent.
Suggested fixes, cheapest first
Print the reviewed sha (the refs/pull/N/head commit the container
checked out) in the consensus comment header and the status board. Cheap, and
fixes the human-facing half outright.
Store it on each finding record so list_findings can filter by sha, and
a consumer can ask "what is new since the commit I last addressed?" without
diffing snapshots.
Post a Gitea commit status on the reviewed sha. This makes "gadfly
reviewed this head" machine-readable, so ready-to-merge automation can
verify the claim rather than trust it.
Optionally, keep prior rounds auditable — either a new consensus comment per
round, or a collapsed <details> history inside the existing one.
(1) and (3) are the ones that carry real weight; (2) is what would have saved me
the manual diffing.
Context
Three rounds on #1536, 15 findings, all addressed. Rounds 1 and 2 each caught
genuine correctness bugs inside the previous round's fix, so the multi-round
flow is working — this issue is only about being able to say, afterwards, which
commit each round was talking about.
Found while running three review rounds on `steve/mort#1536`. Not a breakage —
the reviews themselves were accurate and caught real bugs — but there is
currently **no way to determine which commit a given finding was produced
against**, which undermines the one label the release flow depends on.
## Evidence
**1. Finding records carry no commit.** `list_findings` returns:
```json
{"finding_id":"…","repo":"steve/mort","pr":1536,"lens":"correctness",
"file":"…","line":241,"title":"…","models":["claude-code/opus"],"graded":false}
```
No sha, no round, no timestamp. The store is also cumulative, so after fixing
round 1 I still saw all 8 of its findings listed alongside round 2's. To work
out what round 2 had actually found, I had to snapshot `list_findings` before
triggering it and diff the two by hand. Without that snapshot it would have been
guesswork.
**2. The consensus comment is edited in place and accumulates.** On #1536:
```
status-board: id=17711 created=02:55:42Z updated=03:33:23Z
consensus: id=17716 created=03:05:51Z updated=03:24:45Z
```
That single comment now contains both a round-1 finding (`archive_url`'s
`status:"saved"`) and a round-2 one (`_OPAQUE_VALUE_KEYS` missing `web_search`'s
snippet). Round 1's consensus as it stood at the time is gone — there is no
per-round artifact to point at when explaining what was addressed and when.
**3. Neither comment names a sha.** Scanning both bodies for sha-like tokens
finds only `1b634840` — a *run id* quoted inside a finding's excerpt of my own
code comments, not a commit reference.
**4. Comment-triggered runs report the default-branch sha.** `@gadfly review`
produces an Actions run with `head_sha = e581dab6` (main), not the PR head. This
one is **platform behaviour, not a gadfly bug** — an `issue_comment` event
necessarily runs against the default branch, and the workflow correctly checks
out `refs/pull/N/head` regardless. I'm noting it only because it removes the
last place an observer could have recovered the reviewed commit, and so that
nobody chases it as a defect.
## Why it matters
`steve/mort` gates merges on a `ready-to-merge` label meaning *"CI green, gadfly
reviewed and addressed"*. Nothing records **which commit** was reviewed, so a
review of a stale head is indistinguishable from one of the current head. A PR
that gets three more commits after its last review still looks reviewed. CI
solves this with a per-commit status; gadfly currently has no equivalent.
## Suggested fixes, cheapest first
1. **Print the reviewed sha** (the `refs/pull/N/head` commit the container
checked out) in the consensus comment header and the status board. Cheap, and
fixes the human-facing half outright.
2. **Store it on each finding record** so `list_findings` can filter by sha, and
a consumer can ask "what is new since the commit I last addressed?" without
diffing snapshots.
3. **Post a Gitea commit status on the reviewed sha.** This makes "gadfly
reviewed *this* head" machine-readable, so `ready-to-merge` automation can
verify the claim rather than trust it.
4. Optionally, keep prior rounds auditable — either a new consensus comment per
round, or a collapsed `<details>` history inside the existing one.
(1) and (3) are the ones that carry real weight; (2) is what would have saved me
the manual diffing.
## Context
Three rounds on #1536, 15 findings, all addressed. Rounds 1 and 2 each caught
genuine correctness bugs *inside the previous round's fix*, so the multi-round
flow is working — this issue is only about being able to say, afterwards, which
commit each round was talking about.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Found while running three review rounds on
steve/mort#1536. Not a breakage —the reviews themselves were accurate and caught real bugs — but there is
currently no way to determine which commit a given finding was produced
against, which undermines the one label the release flow depends on.
Evidence
1. Finding records carry no commit.
list_findingsreturns:No sha, no round, no timestamp. The store is also cumulative, so after fixing
round 1 I still saw all 8 of its findings listed alongside round 2's. To work
out what round 2 had actually found, I had to snapshot
list_findingsbeforetriggering it and diff the two by hand. Without that snapshot it would have been
guesswork.
2. The consensus comment is edited in place and accumulates. On #1536:
That single comment now contains both a round-1 finding (
archive_url'sstatus:"saved") and a round-2 one (_OPAQUE_VALUE_KEYSmissingweb_search'ssnippet). Round 1's consensus as it stood at the time is gone — there is no
per-round artifact to point at when explaining what was addressed and when.
3. Neither comment names a sha. Scanning both bodies for sha-like tokens
finds only
1b634840— a run id quoted inside a finding's excerpt of my owncode comments, not a commit reference.
4. Comment-triggered runs report the default-branch sha.
@gadfly reviewproduces an Actions run with
head_sha = e581dab6(main), not the PR head. Thisone is platform behaviour, not a gadfly bug — an
issue_commenteventnecessarily runs against the default branch, and the workflow correctly checks
out
refs/pull/N/headregardless. I'm noting it only because it removes thelast place an observer could have recovered the reviewed commit, and so that
nobody chases it as a defect.
Why it matters
steve/mortgates merges on aready-to-mergelabel meaning "CI green, gadflyreviewed and addressed". Nothing records which commit was reviewed, so a
review of a stale head is indistinguishable from one of the current head. A PR
that gets three more commits after its last review still looks reviewed. CI
solves this with a per-commit status; gadfly currently has no equivalent.
Suggested fixes, cheapest first
refs/pull/N/headcommit the containerchecked out) in the consensus comment header and the status board. Cheap, and
fixes the human-facing half outright.
list_findingscan filter by sha, anda consumer can ask "what is new since the commit I last addressed?" without
diffing snapshots.
reviewed this head" machine-readable, so
ready-to-mergeautomation canverify the claim rather than trust it.
round, or a collapsed
<details>history inside the existing one.(1) and (3) are the ones that carry real weight; (2) is what would have saved me
the manual diffing.
Context
Three rounds on #1536, 15 findings, all addressed. Rounds 1 and 2 each caught
genuine correctness bugs inside the previous round's fix, so the multi-round
flow is working — this issue is only about being able to say, afterwards, which
commit each round was talking about.