88 lines
3.8 KiB
Markdown
88 lines
3.8 KiB
Markdown
# PR#51: a high risk and a confirmed finding in the same file give RED; the review times its phases
|
|
|
|
> **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
|
|
|
|
werkrevisor shall separate the PRs that need a human from the others,
|
|
and the coding agent shall remove issues before a human spends time on the PR.
|
|
The agreed verdict rule: ORANGE for a very likely high risk or a concrete issue,
|
|
RED for both in the same file or category, so that only a new commit lifts it;
|
|
this PR implements the same file.
|
|
Since PR#46, Jev rates each changed file by risk categories, and a very likely risk gives ORANGE;
|
|
a finding of the model that Jev confirms gives ORANGE too.
|
|
Both in the same file still give only ORANGE, which a human can approve.
|
|
|
|
On 2026-10-08 a review of PR#50 took 749 s, with one model call and a timeout of 300 s per call.
|
|
The build used 4.5 s of CPU, so werkrevisor waited for some service most of the time.
|
|
The usage record holds only the total duration, so it does not show which call waited.
|
|
|
|
## Non-Goals
|
|
|
|
- No PR-wide classification by Jev; the rule combines the existing results per file.
|
|
- No limit of the correction rounds after RED yet.
|
|
- No RED for a risk and a finding of the same category in different files.
|
|
- No link between a contradiction of the PR document and a file: the contradiction has no file of the code.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: a high risk and a confirmed finding in the same file give RED
|
|
|
|
#### Background
|
|
|
|
- **Given** Jev judges every changed file with the risk categories of a template
|
|
- **and** Jev judges the findings of the model
|
|
|
|
#### Scenario#51.01: a very likely risk and a confirmed finding in the same file give RED
|
|
|
|
So that the author lowers the risk with a new commit before a human spends time on the PR.
|
|
|
|
- **Given** Jev rates a file at or above `orange_min` of a risk category
|
|
- **When** the model reports a finding in the same file, and Jev rates it as probably real and severe
|
|
- **Then** the review is RED
|
|
- **and** the reason names the file, the risk category and the finding
|
|
|
|
##### Verified by
|
|
|
|
- [test_risk_with_issue: test_a_very_likely_risk_and_a_confirmed_finding_in_the_same_file_give_red](../../tests/test_risk_with_issue.py)
|
|
|
|
#### Scenario#51.02: a risk and a finding in different files stay ORANGE
|
|
|
|
So that a human can still approve a PR whose risk and finding do not meet.
|
|
|
|
- **Given** Jev rates one file at or above `orange_min` of a risk category
|
|
- **When** Jev confirms a finding of the model in another file
|
|
- **Then** the review is ORANGE
|
|
|
|
##### Verified by
|
|
|
|
- [test_risk_with_issue: test_a_risk_and_a_finding_in_different_files_stay_orange](../../tests/test_risk_with_issue.py)
|
|
|
|
### Feature: the review times its phases
|
|
|
|
#### Scenario#51.03: the usage record and the comment show the duration of each phase
|
|
|
|
So that a slow review shows which call waited.
|
|
|
|
- **When** werkrevisor writes the usage record and the result comment
|
|
- **Then** both name the seconds of each phase that ran,
|
|
e.g. the model review, the change summary, the excerpt selection and each kind of Jev call
|
|
|
|
##### Verified by
|
|
|
|
- [test_risk_with_issue: test_the_usage_record_and_the_comment_show_the_duration_of_each_phase](../../tests/test_risk_with_issue.py)
|
|
|
|
## The Solution
|
|
|
|
- A new reason `risk_with_issue` with RED: a file that a risk category or `weakens_checks` rates as ORANGE,
|
|
and an ORANGE finding of the model, judged by Jev, in the same file.
|
|
- The usage record gets `phase_seconds`, a table of phase and seconds; the comment shows it in the usage line.
|
|
- The README says where RED comes from now, and names `phase_seconds`.
|
|
- werkrevisor 0.24.0.
|
|
|
|
## Prerequisite PRs
|
|
|
|
- PR#50: werkrevisor 0.23.1, merged.
|