werkator build succeeded after 00:22
werkrevisor 0.14.0: the severity of the model findings decides between ORANGE and YELLOW. - The model marks every finding as `[high]`, `[medium]` or `[low]`; werkrevisor reads and strips the mark. - ORANGE needs one high finding or `[review] orange_min_medium` (default 2) medium ones; other findings give YELLOW. - `[review] orange_min_medium` replaces `red_min_findings`. Details: the PR document under `docs/prs/`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed-on: #14 Co-authored-by: claude-code <claude-code@javagil.de>
184 lines
7.5 KiB
Markdown
184 lines
7.5 KiB
Markdown
# PR#14: each model finding has a severity, and only severe findings give ORANGE
|
|
|
|
> **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 0.13.1 counts the findings of the model, but not their weight.
|
|
With `[review] red_min_findings = 2`, any two findings give ORANGE.
|
|
On werkrevisor PR#13, the model reported a log line and a missing wiring test;
|
|
both were correct, and the model itself said that the impact is small.
|
|
Nevertheless, a human had to check the PR knowingly, only for trivia.
|
|
|
|
## Non-Goals
|
|
|
|
- No severity field in the answer schema of PR-Agent 0.47.0; the model writes the severity into `issue_header`.
|
|
- No new verdict: minor findings give YELLOW, as one finding did before.
|
|
- No automatic migration of configs that still set `red_min_findings`.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: the severity of the findings decides between ORANGE and YELLOW
|
|
|
|
#### Background
|
|
|
|
- The operator instructions tell the model to start every `issue_header` with `[high]`, `[medium]` or `[low]`.
|
|
- The config sets `[review] orange_min_medium = 2`.
|
|
|
|
#### Scenario#14.01: werkrevisor reads the severity from the start of the issue header
|
|
|
|
So that the model can report a severity although the answer schema of PR-Agent has no field for it.
|
|
|
|
- **Given** the model reports a finding with the issue header `<header>`
|
|
- **When** werkrevisor reads the model answer
|
|
- **Then** the finding has the severity `<severity>` and the title `<title>`
|
|
|
|
| header | severity | title |
|
|
|---|---|---|
|
|
| `[high] Possible Bug` | high | `Possible Bug` |
|
|
| `[Medium] Missing Test` | medium | `Missing Test` |
|
|
| ` [ LOW ] Log Noise` | low | `Log Noise` |
|
|
|
|
##### Verified by
|
|
|
|
- [test_verdict.SeverityParsingTest.test_known_prefixes_are_parsed_and_stripped](../../tests/test_verdict.py)
|
|
|
|
#### Scenario#14.02: a finding without a known severity counts as medium
|
|
|
|
So that a model that ignores the instruction cannot make a finding look harmless.
|
|
|
|
- **Given** the model reports a finding with the issue header `<header>`
|
|
- **When** werkrevisor writes the comment
|
|
- **Then** the finding counts as medium
|
|
- **and** the comment shows it with the title `<title>` and the note `severity unknown, counted as medium`
|
|
|
|
| header | title |
|
|
|---|---|
|
|
| `Possible Bug` | `Possible Bug` |
|
|
| `[critical] Possible Bug` | `[critical] Possible Bug` |
|
|
|
|
##### Verified by
|
|
|
|
- [test_verdict.SeverityParsingTest.test_missing_or_unknown_prefix_counts_as_medium](../../tests/test_verdict.py)
|
|
- [test_report.FindingOrderTest.test_unknown_severity_is_shown_as_counted_as_medium](../../tests/test_report.py)
|
|
|
|
#### Scenario#14.03: the severities of the findings decide the verdict
|
|
|
|
So that a human checks a PR knowingly only for severe findings.
|
|
|
|
- **Given** the model recommends `safe_to_merge` and reports the findings `<findings>`
|
|
- **When** werkrevisor assesses the model answer
|
|
- **Then** the verdict is `<verdict>`
|
|
|
|
| findings | verdict |
|
|
|---|---|
|
|
| 1 high | ORANGE |
|
|
| 2 medium | ORANGE |
|
|
| 1 medium, 1 without severity | ORANGE |
|
|
| 1 medium | YELLOW |
|
|
| 1 medium, 3 low | YELLOW |
|
|
| 3 low | YELLOW |
|
|
| none | GREEN |
|
|
|
|
##### Verified by
|
|
|
|
- [test_verdict.SeverityVerdictTest.test_verdict_follows_the_severities](../../tests/test_verdict.py)
|
|
|
|
#### Scenario#14.04: a recommendation to change the PR alone gives YELLOW
|
|
|
|
So that the recommendation of the model does not make minor findings severe.
|
|
|
|
- **Given** the model recommends `changes_required` and reports the findings `<findings>`
|
|
- **When** werkrevisor assesses the model answer
|
|
- **Then** the verdict is `<verdict>`
|
|
|
|
| findings | verdict |
|
|
|---|---|
|
|
| 1 low | YELLOW |
|
|
| none | YELLOW |
|
|
| 1 high | ORANGE |
|
|
|
|
##### Verified by
|
|
|
|
- [test_verdict.SeverityVerdictTest.test_changes_required_gives_orange_only_with_severe_findings](../../tests/test_verdict.py)
|
|
|
|
#### Scenario#14.05: the comment lists the findings from high to low severity
|
|
|
|
So that the reviewer reads the severe findings first.
|
|
|
|
- **Given** the model reports a low, a high, a second low and a medium finding, in this order
|
|
- **When** werkrevisor writes the comment
|
|
- **Then** the comment lists the high, the medium, the first low and the second low finding
|
|
- **and** each finding shows its severity, e.g. `1. **[high] Possible Bug** at …`
|
|
|
|
##### Verified by
|
|
|
|
- [test_report.FindingOrderTest.test_findings_are_listed_from_high_to_low_severity](../../tests/test_report.py)
|
|
|
|
#### Scenario#14.06: `orange_min_medium` must be a non-negative integer
|
|
|
|
So that a wrong value fails at the start and not in the verdict.
|
|
|
|
- **Given** a config with `[review] orange_min_medium = <value>`
|
|
- **When** werkrevisor loads the config
|
|
- **Then** the result is `<result>`
|
|
|
|
| value | result |
|
|
|---|---|
|
|
| `0` | valid; medium findings never give ORANGE |
|
|
| `-1`, `"two"`, `true` | config error |
|
|
|
|
##### Verified by
|
|
|
|
- [test_config_budget_redact.ConfigTest.test_orange_min_medium_must_be_a_non_negative_integer](../../tests/test_config_budget_redact.py)
|
|
- [test_verdict.SeverityVerdictTest.test_orange_min_medium_zero_disables_the_medium_rule](../../tests/test_verdict.py)
|
|
|
|
#### Scenario#14.07: a host config with the old key `red_min_findings` is invalid
|
|
|
|
So that the operator notices the renamed key at the first run.
|
|
|
|
- **Given** a host config with `[review] red_min_findings = 2`
|
|
- **When** werkrevisor loads the config
|
|
- **Then** werkrevisor stops with the config error `unknown keys: red_min_findings`
|
|
|
|
##### Verified by
|
|
|
|
- [test_config_budget_redact.ConfigTest.test_old_red_min_findings_is_an_unknown_key](../../tests/test_config_budget_redact.py)
|
|
|
|
#### Scenario#14.08: a changed repository config with the old key gives YELLOW
|
|
|
|
So that a repository config of the old version does not block a PR with RED.
|
|
|
|
- **Given** a PR changes `.werkrevisor.toml` and keeps `[review] red_min_findings = 2`
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** the verdict is YELLOW with the reason `repository_config_unknown` for `[review] red_min_findings`
|
|
|
|
##### Verified by
|
|
|
|
- [test_config_layers.ChangedRepositoryConfigTest.test_old_red_min_findings_is_yellow](../../tests/test_config_layers.py)
|
|
|
|
## The Solution
|
|
|
|
- The `extra_instructions` of all example configs and of `.werkrevisor.toml` define the three severities:
|
|
high is wrong behavior, data loss, a security problem or a broken contract in a realistic scenario;
|
|
medium is a defect with limited impact or a missing test for changed behavior;
|
|
low is cosmetics, wording, log noise or style.
|
|
- `parse_model_review` reads the prefix of `issue_header`, case-insensitive, and removes it from the title.
|
|
- `assess_model` gives ORANGE for one high finding or for `orange_min_medium` medium findings;
|
|
any other finding gives YELLOW with the reason `model reports minor findings`.
|
|
- `merge_recommendation: changes_required` gives YELLOW; ORANGE comes only from the findings.
|
|
- The comment sorts the findings by severity, stable, before it cuts them at `max_findings`.
|
|
- `[review] orange_min_medium` (default 2) replaces `[review] red_min_findings`; the old key is a config error.
|
|
Operators rename it in their host configs before they install 0.14.0.
|
|
- `.werkrevisor.toml` of this repository renames the key too.
|
|
The installed werkrevisor 0.13 does not know `orange_min_medium`, so this PR gets YELLOW;
|
|
install 0.14.0 right after the merge.
|
|
- werkrevisor 0.14.0.
|
|
|
|
## Additional Changes
|
|
|
|
- Several flow tests used `changes_required` to get ORANGE; they now use a finding of high severity.
|
|
- `review_data` in `tests/fakes.py` takes the issue header as a parameter.
|