Files
werkrevisor/docs/prs/2026-10-10-PR#73-approval-carry-over.md
claude-code 12e7417fbd
werkator build succeeded after 00:17
werkator/werkrevisor-statistics build succeeded after 00:34
werkrevisor 0.32.0: an approval survives a rebase (#73)
Co-authored-by: claude-code <claude-code@javagil.de>
2026-10-10 13:56:15 +02:00

135 lines
5.8 KiB
Markdown

# PR#73: An Approval Survives a Rebase
> **WARNING:** This document describes only the change applied in this PR.
> It may already be outdated once the next PR is merged.
> Historic PR documentation is not maintained along with new PRs; treat it as a snapshot.
## The Problem
Gitea dismisses an approval when new commits arrive (`dismiss_stale_approvals`).
A rebase onto `main`, or a merge of `main` into the branch, thus costs a new human approval,
although the change of the PR stays the same.
werkator PR#137 needed three approvals of the same change in this way.
## Non-Goals
- No carry-over of an approval that a human dismissed by hand or revoked with "request changes".
- No check that the merged `main` still fits the change semantically; the CI (`werkator`) stays required for that.
- No calibration of the Jev thresholds for corrections; this PR uses those of `[path_judge]`.
## The Scenarios
### Feature: an approval survives a rebase
#### Scenario#73.01: a rebase keeps the approval
So that a rebase onto `main` costs no new human approval.
- **Given** a human approved the head A of a PR
- **When** the PR is rebased onto a newer `main` without a conflict
- **Then** werkrevisor approves the new head without Jev and without a model call
- **And** its reason names the approval of A
##### Verified by
- [test_carry_over.CarryOverTest.test_a_rebase_keeps_the_approval](../../tests/test_carry_over.py)
#### Scenario#73.02: a small correction after a merge keeps the approval
So that a resolved merge conflict needs no human when Jev rates it as harmless.
- **Given** a human approved the head A of a PR
- **And** the new head has a changed block that A did not have
- **And** Jev rates that block below the thresholds of `[path_judge]`
- **When** werkrevisor reviews the new head
- **Then** it approves without a model call
##### Verified by
- [test_carry_over.CarryOverTest.test_a_harmless_correction_keeps_the_approval](../../tests/test_carry_over.py)
#### Scenario#73.03: a risky correction gets a review of the correction only
So that approved changes do not block the PR again.
- **Given** a human approved the head A of a PR
- **And** Jev rates a new block as risky
- **When** werkrevisor reviews the new head
- **Then** the model gets only the files with new blocks
- **And** a finding on the lines of an approved block is a hint
##### Verified by
- [test_carry_over.CarryOverTest.test_a_risky_correction_gets_a_review](../../tests/test_carry_over.py)
- [test_carry_over.CarryOverTest.test_a_finding_on_approved_lines_is_a_hint](../../tests/test_carry_over.py)
- [test_carry_over.ChangeKindTest.test_the_kind_comes_from_the_files_outside_the_approval](../../tests/test_carry_over.py)
#### Scenario#73.04: an approval dismissed by hand is not carried over
So that a human can take back an approval.
- **Given** a human dismissed the approval by hand
- **When** werkrevisor reviews the new head
- **Then** the model reviews every changed file
##### Verified by
- [test_carry_over.CarryOverTest.test_an_approval_dismissed_by_hand_is_not_carried_over](../../tests/test_carry_over.py)
#### Scenario#73.05: a dropped approved change is not carried over
So that a check that the PR added cannot disappear unnoticed in a rebase.
- **Given** a human approved the head A of a PR
- **When** a rebase drops a changed block of A
- **Then** the next review gives the model every changed file
- **And** a hint names the number of missing blocks
##### Verified by
- [test_carry_over.CarryOverTest.test_a_dropped_block_ends_the_carry_over](../../tests/test_carry_over.py)
#### Scenario#73.06: blocks approved by werkrevisor do not block again
So that a rebase after a GREEN review brings no new blocking findings on unchanged lines.
- **Given** werkrevisor approved the head A of a PR as GREEN
- **When** werkrevisor reviews the rebased head
- **Then** a finding on a block of A is a hint
##### Verified by
- [test_carry_over.CarryOverTest.test_blocks_of_a_green_review_do_not_block_again](../../tests/test_carry_over.py)
## The Solution
- Every result comment gets a hidden marker with a fingerprint per changed block:
the path, the removed and the added lines, without line numbers and without context.
- A review looks for the newest approval of a head other than the current one:
a human approval that Gitea dismissed automatically, or a GREEN approval of werkrevisor itself.
The fingerprints come from the result comment of werkrevisor for that head.
- After a human approval, files with approved blocks only are left out of Jev and the model, like mechanical files.
Without new blocks, werkrevisor approves; with new blocks, Jev judges their files first.
- A finding of the model on approved lines only is a hint.
- The change kind, which the model checks, comes from the files outside the approval.
- `[gate] carry_over_approvals = false` switches the carry-over of human approvals off.
- The README explains it under "Approval after a rebase"; the decision diagram shows both shortcuts.
- Version 0.32.0.
## Open Questions
- [x] Can werkrevisor compare the approved head with the new one through the Gitea API?
Not directly: the compare route with `.diff` answers 404 for a private repository, even with a token.
So each review keeps the fingerprints of its own head in its result comment.
- [x] How does werkrevisor tell an approval dismissed by a push from one dismissed by hand?
Gitea's timeline has a `dismiss_review` event; after a push its text is
"New commits pushed, approval review dismissed automatically", by hand it is the text of the human.
## Additional Changes
- The Werkbaum gets the plan `#werkregie.pwa`: the Werkregie frontend becomes a PWA; this PR does not implement it.
## Follow-up PRs
- Calibrate the Jev thresholds for corrections on real rebases, e.g. werkator PR#137, see `#gate.carry.calibrate`.