# PR#4: Clearer reasons and two size limits

> **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

The first self-review of PR#3 by werkrevisor showed four weaknesses in its comment:

1. The reason `change kind 'mixed' is not AI-approvable` did not say which files made the PR mixed,
   nor which change kinds could pass.
2. A limit-based reason did not say which config key it came from.
   The limit of 300 changed lines was far too low anyway: a PR document alone has 100 to 200 lines.
3. The usage line said "cost unknown" although no model call was made.
4. One size limit had two meanings.
   "Too large in general" means that a human must approve the PR knowingly;
   "too large for the model" means that no complete AI review is possible.
   Both gave YELLOW and skipped the model, so a large PR lost the model findings that would help its human reviewer.

In addition, `[model] max_input_tokens` had to be kept consistent with the budget by hand.

## Non-Goals

- The change-kind rule itself does not change; `mixed` stays not AI-approvable.
- No change to the werkrevisor installation on mih09; `docs/betrieb-git.javagil.de.md` keeps the version that runs there.

## The Scenarios

### Feature: reasons that a developer can act on

#### Scenario#4.01: a mixed PR names its file kinds and the AI-approvable change kinds

So that the author sees what to split off to get an AI approval.

- **Given** a PR that changes main code, a Makefile and a PR document
- **When** werkrevisor reviews the PR
- **Then** a reason is `change kind 'mixed' (main code, other files, docs); AI-approvable are only: docs only, tests only added, docs and tests, refactoring with unchanged tests`

##### Verified by

- [test_rules.CategoryTest.test_mixed_reason_names_the_file_kinds_and_the_approvable_change_kinds](../../tests/test_rules.py)
- [test_rules.CategoryTest.test_mixed_reason_names_api_spec_and_tests](../../tests/test_rules.py)

#### Scenario#4.02: a limit-based reason names its config key and value

So that the operator knows which value to change.

- **Given** a PR that exceeds the limit `<limit>`
- **When** werkrevisor reviews the PR
- **Then** the reason contains `<limit>` with its configured value

| limit |
|---|
| `[size] max_changed_lines` |
| `[size] max_files` |
| `[model] max_context_tokens` |
| `[budget] max_usd_per_review` |
| `[budget] min_key_remaining_usd` |
| `[model] max_output_tokens` |
| `[review] max_full_file_chars` |
| `[review] max_full_files_chars` |
| `[review] red_min_findings` |
| `[classifier] max_state_chars` |
| `[classifier] max_units` |
| `[classifier] min_confidence` |

##### Verified by

- [test_rules.HardYellowTest.test_size_limits_are_orange_and_keep_the_model](../../tests/test_rules.py)
- [test_config_budget_redact.BudgetTest.test_precheck_reasons_name_the_limit](../../tests/test_config_budget_redact.py)
- [test_config_budget_redact.BudgetTest.test_key_check_names_the_deciding_limit](../../tests/test_config_budget_redact.py)
- [test_flow.VerdictFlowTest.test_truncated_answer_is_error](../../tests/test_flow.py)
- [test_fullfiles.CollectTest.test_per_file_and_total_limits](../../tests/test_fullfiles.py)
- [test_verdict.ModelMappingTest.test_threshold_reason_names_the_config_key](../../tests/test_verdict.py)
- [test_classifier.JudgeUnitsTest.test_max_units](../../tests/test_classifier.py)
- [test_classifier.MappingTest.test_rate](../../tests/test_classifier.py)

### Feature: the usage line states the cost

#### Scenario#4.03: a run without a model call costs nothing

So that the cost of a skipped review does not look unknown.

- **Given** a PR that werkrevisor reviews without a model call
- **When** werkrevisor writes its comment and usage record
- **Then** the comment says `no model call, cost 0 USD`
  - **and** the usage record has the cost 0.0 USD with the cost status `none`

##### Verified by

- [test_flow.PublishAndUsageTest.test_run_without_model_call_costs_nothing](../../tests/test_flow.py)

#### Scenario#4.04: a model call without usage data has an unknown cost

So that "unknown" means a real gap in the data.

- **Given** a model call that reports no usage
- **When** werkrevisor writes its comment
- **Then** the comment says `cost unknown`

##### Verified by

- [test_flow.PublishAndUsageTest.test_model_call_without_usage_has_unknown_cost](../../tests/test_flow.py)

### Feature: two separate size limits

#### Background

- The operator config of the repository is `config/werkprobe.toml`: 800 changed lines, 30 files, 0.40 USD per review.

#### Scenario#4.05: a PR above the general size limit needs a knowing human approval

So that a large PR gets a human review and still the model findings.

- **Given** a PR with 900 changed lines of main code
- **When** werkrevisor reviews the PR
- **Then** the verdict is ORANGE with the reason `PR too large for an AI-only approval: … exceed [size] max_changed_lines = 800; please review it yourself`
  - **and** the model reviews the PR, and its findings appear in the comment

##### Verified by

- [test_flow.SizeFlowTest.test_large_pr_is_orange_and_the_model_still_reviews_it](../../tests/test_flow.py)

#### Scenario#4.06: docs do not count for the general size limit

So that a long PR document does not make a small change look large.

- **Given** a PR with 2000 changed lines in docs and 800 changed lines of main code
- **When** werkrevisor reviews the PR
- **Then** no size reason appears

##### Verified by

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

#### Scenario#4.07: a PR too long for the model gets no AI review

So that the model never reviews a prompt that is too long for a good review.

- **Given** `[model] max_context_tokens` below the estimated input of the PR
- **When** werkrevisor reviews the PR
- **Then** the verdict is YELLOW with the reason `no AI review: the PR needs about … input tokens, more than [model] max_context_tokens = …`
  - **and** no model call is made

##### Verified by

- [test_flow.BudgetFlowTest.test_too_long_for_the_model_is_yellow_without_llm_call](../../tests/test_flow.py)

#### Scenario#4.08: a PR too expensive for the budget gets no AI review

So that a review never costs more than the budget allows.

- **Given** a `[budget] max_usd_per_review` that pays for less input than the PR needs
- **When** werkrevisor reviews the PR
- **Then** the verdict is YELLOW with the reason `no AI review: … more than the limit of … derived from [budget] max_usd_per_review = …`
  - **and** no model call is made

##### Verified by

- [test_flow.BudgetFlowTest.test_too_expensive_is_yellow_without_llm_call](../../tests/test_flow.py)
- [test_config_budget_redact.BudgetTest.test_input_limit_is_the_lower_of_budget_and_context](../../tests/test_config_budget_redact.py)

#### Scenario#4.09: a PR above both limits shows both reasons

So that the human sees both why the PR needs a knowing approval and why the model did not review it.

- **Given** a PR above `[size] max_changed_lines` and above the input limit of the model
- **When** werkrevisor reviews the PR
- **Then** the verdict is ORANGE
  - **and** both the size reason and the "no AI review" reason appear

##### Verified by

- [test_flow.SizeFlowTest.test_large_pr_too_long_for_the_model_is_orange_with_both_reasons](../../tests/test_flow.py)

#### Scenario#4.10: a config with the old key `max_input_tokens` is refused

So that an old config cannot silently keep a limit that no longer exists.

- **Given** an operator config with `[model] max_input_tokens`
- **When** werkrevisor loads it
- **Then** loading fails with "unknown keys: max_input_tokens"

##### Verified by

- [test_config_budget_redact.ConfigTest.test_old_max_input_tokens_is_an_unknown_key](../../tests/test_config_budget_redact.py)

## The Solution

- The reason for a change kind that is not AI-approvable lists the kinds of the changed files
  (main code, API spec, tests, other files, docs) and the configured AI-approvable change kinds in readable words.
- Every limit-based reason names its config key and value, e.g. `[size] max_changed_lines = 800`.
- A run without a model call has `cost_usd` 0.0 and the cost status `none`; the comment says `no model call, cost 0 USD`.
- `[size]` counts only changed lines and files outside the `docs` category.
  Above it, the verdict is ORANGE, and the model still reviews the PR.
- `[model] max_input_tokens` is renamed to `max_context_tokens`, a quality cap: 150000 for frontier models, 80000 for cheap ones.
  werkrevisor derives a second limit, the largest input whose worst case stays within `[budget] max_usd_per_review`.
  The lower of both is the effective input limit; above it, the verdict is YELLOW without a model call.
  PR-Agent gets the same effective limit.
  A config whose budget does not pay for `max_output_tokens` is refused.
- New example values:
  - `config/werkprobe.toml` and `config/werkrevisor.toml`: 800 lines, 30 files, 0.40 USD, which gives 80000 input tokens;
    `max_full_files_chars` 120000, so that the diff of an 800-line PR still fits.
  - `config/hsadmin-ng.toml`: 1000 lines, 40 files, 1.00 USD, limited by `max_context_tokens` to 150000 input tokens;
    `max_full_files_chars` 300000 instead of 400000, for the same reason.
- The general size limits come from the changed lines outside the docs per merged PR in one month:
  Werkator (92 PRs) median 194, p75 489, p90 865, so 800 lines hit about every tenth PR;
  hsadmin-ng (16 PRs) median 441, p75 974, p90 1098, so 1000 lines hit about every fourth PR,
  just below the hard limit of hsadmin-ng's own `tools/pr-quick-check` (about 1200 to 1400 lines).
- werkrevisor 0.7.0; the README and the German guides describe the two limits.

## Open Questions

- [x] Is the change-kind rule 'mixed' too strict, so that GREEN almost never happens? Yes, very likely; it is reconsidered in a follow-up PR.

## Follow-up PRs

- Revise the change-kind rule, so that a typical PR with main code, tests and a PR document can become GREEN.