The thumb is placed and never withdrawn - #40
Merged
Conversation
A product decision, and it removes more than it adds. Withdrawing was the source of the concurrency problem the settle step kept growing conditions for. Nothing serialises invocations, so an older findings run finishing after a newer clean one deleted a thumb that correctly described the current head — and the justification for leaving that delete unguarded, "the current run re-adds it on completion", held only when the current run finished last, which nothing guarantees. Removing the withdrawal removes the class. What it costs, stated plainly: a head that was clean keeps its 👍 after a later push breaks something. The review comment on that push says so and the pinned summary says so, so the mark now means "some head of this PR reviewed clean" rather than "the current one does". The hosted reviewer has made the same trade since it shipped, so the two paths agree about what it means. The head guard stays, and matters more rather than less — a wrong add is now permanent. An unconfirmed head still counts as moved. The existence lookup goes too: creating a reaction that already exists is a no-op at the API, so there is nothing to check first. That also takes the 👍 off the identity resolution entirely, which confines the custom-App gap in issue #39 to clearing 👀. Verified: parses, the settle step passes bash -n, and no withdrawal path remains.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A product decision that deletes more than it adds.
Withdrawing was the source of the concurrency problem the settle step kept growing conditions for. Nothing serialises invocations, so an older findings run finishing after a newer clean one deleted a thumb that correctly described the current head — and the justification for leaving that delete unguarded ("the current run re-adds it on completion") held only when the current run finished last, which nothing guarantees.
Removing the withdrawal removes the class. This closes finding 2 of #39.
What it costs
A head that was clean keeps its 👍 after a later push breaks something. The review comment on that push says so and the pinned summary says so, so the mark means "some head of this PR reviewed clean" rather than "the current one does".
The hosted reviewer has made the same trade since it shipped, so both paths now agree about what it means.
What stays
The head guard, and it matters more rather than less — a wrong add is now permanent. An unconfirmed head still counts as moved.
What else goes
The existence lookup: creating a reaction that already exists is a no-op at the API, so there is nothing to check first. That takes the 👍 off identity resolution entirely, which confines the custom-App gap in #39 to clearing 👀.
🤖 Generated with Claude Code