# PR#16: the colors say how certain a finding is, and Jev judges the model findings

> **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.14.0 rates almost every PR one level too critical.
Of the werkrevisor PRs #3 to #15, none was GREEN, and this was hardly ever due to a real defect.
The causes are:

- any minor finding or a cautious recommendation of the model gives YELLOW;
- a changed assertion line counts as a weakened test, even if the test only checks a new value;
- the change kind `mixed` is not AI-approvable;
- a PR above the general size limit gives ORANGE, although nobody found anything;
- a doc line longer than 128 characters gives ORANGE.

The colors get a meaning that a reviewer can act on:

| Color | Meaning |
|---|---|
| 🔴 RED | certain finding: a deterministic rule is violated |
| 🟠 ORANGE | probably real finding |
| 🟡 YELLOW | dangerous change detected, but no finding |
| 🟢 GREEN | nothing of the above; hints may remain |

The model cannot tell well whether its own finding is real.
A measurement on 42 findings of this session, 29 real and 13 false, showed that Jev can:

- the probability of a real defect separates real from false findings with an AUC of 0.80;
- with "real ≥ 0.60 and severity ≥ 2.5", 10 findings get ORANGE:
  9 of the 15 real medium or high findings, no real low one, and 1 false one.

## Non-Goals

- No RED for model findings: a false RED is a hard lock, and on dev.hostsharing.net only a new commit lifts it.
- No change of the DB schema rule: a changed Liquibase changelog stays YELLOW.
- No `[hard_orange]` tier: PR#15 is replaced by this PR; dangerous paths give YELLOW.
- No Jev judgment of the classifier hints for the PR documents; they keep their own thresholds.

## The Scenarios

### Feature: a dangerous change without a finding gives YELLOW

#### Scenario#16.01: a PR above the general size limit gives YELLOW

So that a large PR does not look like a PR with a finding.

- **Given** the config sets `[size] max_changed_lines = 800` and `max_files = 30`
- **When** werkrevisor reviews a PR with `<changed>`
- **Then** the verdict is YELLOW with the reason `<reason>`

| changed | reason |
|---|---|
| 801 changed lines | `size_lines` |
| 31 changed files | `size_files` |

##### Verified by

- [test_rules.HardYellowTest.test_size_limits_give_yellow](../../tests/test_rules.py)

### Feature: Jev judges every model finding

#### Background

- The host config sets `[engine] mode = "combined"` with a `[classifier]` section.
- The host config keeps the defaults `[review] finding_real_min = 0.6` and `finding_severity_min = 2.5`.

#### Scenario#16.02: Jev's two answers decide the color of a finding

So that only a probably real and severe finding needs a developer's check.

- **Given** the model reports one finding of the severity `<model>`
  - **and** Jev rates it as a real defect with `<real>` and with the severity `<severity>` of 3
- **When** werkrevisor reviews the PR
- **Then** the verdict is `<verdict>`
  - **and** the comment shows the finding with Jev's two values

| model | real | severity | verdict |
|---|---|---|---|
| medium | 0.72 | 2.6 | ORANGE |
| high | 0.40 | 2.8 | YELLOW |
| high | 0.90 | 2.0 | GREEN |
| low | 0.95 | 2.9 | GREEN |

##### Verified by

- [test_findings.FindingJudgmentTest.test_jev_answers_decide_the_verdict](../../tests/test_findings.py)
- [test_findings.FindingJudgmentTest.test_comment_shows_the_jev_values](../../tests/test_findings.py)

#### Scenario#16.03: Jev sees the finding and the code around it

So that Jev judges the finding against the reviewed code and not against the model's text alone.

- **Given** the model reports a finding at `app.py:40-41`
- **When** werkrevisor asks Jev
- **Then** Jev gets the finding text and the 25 lines above and below the finding in `app.py` at the head commit

##### Verified by

- [test_findings.FindingStateTest.test_state_holds_the_finding_and_the_code_excerpt](../../tests/test_findings.py)

#### Scenario#16.04: a finding that Jev cannot judge gives YELLOW

So that a failed call never hides a finding and never blocks hard.

- **Given** the model reports a finding of the severity medium or high
- **When** Jev fails for this finding
- **Then** the verdict is YELLOW with the reason `finding_not_judged`

##### Verified by

- [test_findings.FindingJudgmentTest.test_failed_judgment_gives_yellow](../../tests/test_findings.py)

#### Scenario#16.05: without Jev the model's severity decides

So that a host without a classifier keeps a usable review.

- **Given** the host config sets `[engine] mode = "llm"`
  - **and** the config sets `[review] orange_min_medium = 2`
- **When** the model reports the findings `<findings>`
- **Then** the verdict is `<verdict>`

| findings | verdict |
|---|---|
| 1 high | ORANGE |
| 2 medium | ORANGE |
| 1 medium | YELLOW |
| 3 low | GREEN |

##### Verified by

- [test_verdict.SeverityVerdictTest.test_verdict_follows_the_severities](../../tests/test_verdict.py)

#### Scenario#16.06: the recommendation of the model alone is a hint

So that a cautious model does not make every PR YELLOW.

- **Given** the model reports no finding of the severity medium or high
- **When** the model answers `<answer>`
- **Then** the verdict is `<verdict>`

| answer | verdict |
|---|---|
| `merge_with_caution` | GREEN with a hint |
| `changes_required` | GREEN with a hint |
| `risk_level: high` | YELLOW |
| a security concern | YELLOW |

##### Verified by

- [test_verdict.SeverityVerdictTest.test_recommendation_alone_is_a_hint](../../tests/test_verdict.py)

#### Scenario#16.07: the usage record holds Jev's values for every finding

So that the thresholds can be calibrated later against the human approvals.

- **Given** Jev judged two findings
- **When** werkrevisor writes the usage record
- **Then** the record lists both findings with file, lines, model severity, Jev's two values and the color

##### Verified by

- [test_findings.FindingJudgmentTest.test_usage_record_holds_the_jev_values](../../tests/test_findings.py)

### Feature: style and change kind do not block on their own

#### Scenario#16.08: a violated writing rule is a hint

So that a style finding does not cost a review cycle.

- **Given** a PR changes a doc line with two sentences
- **When** werkrevisor reviews the PR and the model reports nothing
- **Then** the verdict is GREEN
  - **and** the comment lists the violation as `hint`

##### Verified by

- [test_writing.WritingFlowTest.test_violation_is_a_hint_and_the_model_still_reviews](../../tests/test_writing.py)

#### Scenario#16.09: only a lost or disabled test gives YELLOW

So that a test that checks a new value does not count as weakened.

- **Given** a PR changes main code and the test file `<test change>`
- **When** werkrevisor reviews the PR
- **Then** the verdict from the test rule is `<verdict>`
  - **and** the model gets the instruction to check the changed test lines for a weakened test

| test change | verdict |
|---|---|
| replaces `assertEquals(1, x)` with `assertEquals(2, x)` | GREEN |
| removes an assertion line without a new one | YELLOW |
| deletes the test file | YELLOW |
| adds `@Disabled` | YELLOW |

##### Verified by

- [test_change_kinds.TestWeakeningTest.test_only_lost_or_disabled_tests_give_yellow](../../tests/test_change_kinds.py)
- [test_change_kinds.TestWeakeningTest.test_changed_test_lines_go_to_the_model](../../tests/test_change_kinds.py)

#### Scenario#16.10: the change kind `mixed` is AI-approvable

So that a PR with files outside the known categories depends on the paths and the review, not on its kind.

- **Given** a PR changes main code and a file of no category, outside the dangerous paths
- **When** werkrevisor reviews the PR and the model reports nothing
- **Then** the verdict is GREEN

##### Verified by

- [test_change_kinds.MixedTest.test_mixed_is_approvable](../../tests/test_change_kinds.py)

### Feature: deferred work in the PR document does not block

#### Scenario#16.11: an unchecked TODO item in the open questions is a hint

So that work deliberately left for a follow-up does not make a PR RED.

- **Given** the section "Open Questions" of a changed PR document holds the item `<item>`
- **When** werkrevisor reviews the PR
- **Then** the result of the item is `<result>`

| item | result |
|---|---|
| `- [ ] TODO.tool: raise the forks once the server has more cores` | hint `pr_doc_deferred_todos` |
| `- [ ] who decides?` | RED `pr_doc_open_questions` |
| `- [ ] TODO: without a topic` | RED `pr_doc_open_questions` |

##### Verified by

- [test_open_questions.OpenQuestionParserTest.test_todo_item_is_deferred](../../tests/test_open_questions.py)
- [test_open_questions.OpenQuestionRuleTest.test_deferred_todos_are_a_hint](../../tests/test_open_questions.py)

## The Solution

- `size_lines` and `size_files` give YELLOW; a violated writing rule is a hint.
- An open item that starts with `TODO.<topic>:` is deferred work, as hsadmin-ng marks it, and only a hint.
  In the dry run, this was the only cause of RED for hsadmin-ng #281 and #282.
- The new module `werkrevisor/findings.py` asks Jev once per model finding of the severity medium or high.
  The state holds the finding and 25 lines of code above and below it at the head commit.
  The questions are fixed in the code, as in the measurement; only the thresholds are configurable.
- `[review] finding_real_min` (default 0.6) and `finding_severity_min` (default 2.5) are host-only.
  A repository must not lower the bar of its own reviews.
- `assess_model` maps the findings:
  with Jev judgments, as in Scenario#16.02; without them, high or `orange_min_medium` medium findings give ORANGE,
  another medium finding YELLOW, and a low finding a hint.
- A hint is a reason with the verdict GREEN; the comment lists it as `hint`.
- `find_test_weakening` counts removed and added assertion lines per test file.
  Only a net loss, a deleted test file or an added skip marker is a weakening.
  Other changed test lines go to the model instruction.
- The default `auto_approvable` and all example configs contain `mixed`.
- `.werkrevisor.toml` of this repository no longer lists `config/**` as a dangerous path;
  the effective config lives on the host.
- `config/hsadmin-ng.toml` lists only `tools/remote` instead of `tools/**`; the other tools do not deploy.
- werkrevisor 0.15.0.

## Open Questions

- [x] Do the deterministic writing rules give RED, as certain findings?
  No: a style finding is certain, but rarely worth a new commit.
- [x] Do the writing rules stay ORANGE, or do they become YELLOW or a hint?
  A hint: on 25 hsadmin-ng PRs they gave ORANGE to 19, mostly for line length;
  as a hint, 7 of the 25 PRs pass the rules instead of 3.
- [x] Does an incomplete full-file context stay YELLOW, or does it become a hint?
  It stays YELLOW: less context means less certainty; it fires on 6 of the 25 PRs.

## Measurement

Dry run of the rules only (`tools/dry-run`, without model and Jev) on 25 merged hsadmin-ng PRs, #265 to #315;
#283 has no base commit and is left out:

| Rules verdict | 0.14.0 | 0.15.0 |
|---|---|---|
| GREEN | 0 | 7 |
| YELLOW | 4 | 18 |
| ORANGE | 19 | 0 |
| RED | 2 | 0 |

`mixed` as AI-approvable lets #290, #292 and #315 pass, the writing rules as hints #286, #299, #300 and #304.
#281 and #282 change from RED to YELLOW, because their only open items are deferred TODOs.
Of the 18 YELLOW PRs, 12 change the DB schema and 13 touch a dangerous path.
The model findings and Jev's judgment come on top; they can only keep or raise these verdicts.


## Additional Changes

- The comments in the example configs and `.werkrevisor.toml` state the new meaning of the colors.

## Prerequisite PRs

- PR#14: the severity of the model findings.

## Follow-up PRs

- Dry run of the new rules on the hsadmin-ng PRs, to measure the share of GREEN.
- Calibrate `finding_real_min` and `finding_severity_min` against the logged Jev values and the human approvals.
- Targeted review requests per dangerous path.