Files
werkrevisor/docs/prs/2026-10-05-PR#11-mechanical-changes-skip-the-model.md
claude-code d778c113e9
werkator build succeeded after 01:14
werkrevisor 0.13.0: mechanical file changes skip the model (#11)
werkrevisor 0.13.0: mechanical file changes skip the model.

- A file is mechanical only if every changed line is a pure rename, a whitespace change (not in whitespace-significant files) or an import line of a known language; it goes neither into the diff nor into the full files for the model.
- A PR of only mechanical files is GREEN-eligible without a model call; hard rules still apply.
- Additional changes: the example host configs match mih09 (`mode = "combined"`, `token_count_factor = 1.55`); an unknown change kind in a changed `.werkrevisor.toml` gives YELLOW, not RED.

Details: the PR document under `docs/prs/`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Reviewed-on: #11
Co-authored-by: claude-code <claude-code@javagil.de>
2026-10-05 19:28:32 +02:00

271 lines
12 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# PR#11: mechanical file changes skip the model
> **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.12.0 sends the diff and the full content of every changed file to the model.
Many changed files carry no logic change: changed imports, a new indentation, a renamed file.
These files cost tokens, and the model has nothing to judge in them.
A PR of such files alone needs a model call to become GREEN, although deterministic rules can judge it.
## Non-Goals
- No Jev-based risk filter per file; it stays a measured follow-up, see `docs/konzept-jev-classifier.md`.
- No reformatting rule beyond whitespace, e.g. changed line breaks or spaces around operators.
- No change of the hard rules: risky paths, DB schema, open questions and writing rules apply to mechanical files too.
## The Scenarios
### Feature: mechanical file changes skip the model
#### Background
- The config is `config/werkprobe.toml` with `[mechanical] enabled = true`.
- A file is mechanical if the rules `imports`, `whitespace` or `renamed` explain every changed line.
#### Scenario#11.01: a file with only import changes is not sent to the model
So that the model reads only files with something to judge.
- **Given** a PR changes `A.java` with logic and `B.java` with imports only
- **When** werkrevisor reviews the PR
- **Then** the model prompt contains neither the diff nor the full content of `B.java`
- **and** the comment says `1 file changed only mechanically and was not reviewed by the model: B.java (imports).`
##### Verified by
- [test_mechanical.FlowTest.test_mechanical_files_stay_out_of_the_prompt_and_the_coverage_check](../../tests/test_mechanical.py)
- [test_pr_agent_run.PrAgentRunTest.test_excluded_files_never_reach_the_prompt](../../tests/test_pr_agent_run.py)
#### Scenario#11.02: a mechanical file missing in the prompt is no skipped file
So that leaving out a mechanical file does not end the review with ERROR.
- **Given** a PR changes `A.java` with logic and `B.java` with imports only
- **When** the model reviews the PR without `B.java` in its prompt
- **Then** there is no reason `skipped_files`
- **and** the verdict is GREEN
##### Verified by
- [test_mechanical.FlowTest.test_mechanical_files_stay_out_of_the_prompt_and_the_coverage_check](../../tests/test_mechanical.py)
#### Scenario#11.03: the comment lists many mechanical files in one line
So that a large package move does not fill the comment.
- **Given** a PR changes 4 Java files with imports only and `a.c` with whitespace only
- **When** werkrevisor reviews the PR
- **Then** the comment says `5 files changed only mechanically and were not reviewed by the model:`
followed by 3 files with their rules and `and 2 more.`
##### Verified by
- [test_mechanical.FlowTest.test_comment_lists_many_mechanical_files_in_one_line](../../tests/test_mechanical.py)
#### Scenario#11.04: an added or removed import line is mechanical only for the languages with an import rule
So that only a line without effect at runtime skips the model.
- **Given** a file with the extension `<extension>` changes only in the line `<line>`
- **When** werkrevisor checks the file
- **Then** the file is `<result>`
| extension | line | result |
|---|---|---|
| `java`, `kt`, `kts` | `import java.util.Map;`, `package net.example;` | mechanical (imports) |
| `ts`, `tsx`, `js`, `jsx`, `mjs` | `import { a, b } from './x';` | mechanical (imports) |
| `ts` | `import './polyfill';` | not mechanical |
| `ts` | ` b,` inside an `import { … } from './x';` that the hunk shows completely | mechanical (imports) |
| `ts` | ` b,` inside an import whose first line is outside the hunk | not mechanical |
| `go` | `"os"` inside an `import ( … )` block that the hunk shows completely | mechanical (imports) |
| `go` | `_ "embed"` or `. "math"` | not mechanical |
| `rs` | `use std::io::{Read, Write};` | mechanical (imports) |
| `rs` | `mod helpers;` | not mechanical |
| `py` | `import os` | not mechanical |
| `scala` | `import a.b.C` | not mechanical |
##### Verified by
- [test_mechanical.ImportRuleTest](../../tests/test_mechanical.py)
#### Scenario#11.05: one changed line without a rule makes the file not mechanical
So that a logic change never hides between import changes.
- **Given** a Java file changes 3 import lines and adds the line `int x = 1;`
- **When** werkrevisor checks the file
- **Then** the file is not mechanical
- **and** the model gets its diff and its full content
##### Verified by
- [test_mechanical.ImportRuleTest.test_one_non_import_line_makes_the_file_not_mechanical](../../tests/test_mechanical.py)
#### Scenario#11.06: a whitespace change is mechanical except in whitespace-significant files
So that a new indentation skips the model only where indentation has no meaning.
- **Given** the file `<file>` changes only the indentation of one line
- **When** werkrevisor checks the file
- **Then** the file is `<result>`
| file | result |
|---|---|
| `src/a.c`, `src/A.java` | mechanical (whitespace) |
| `app/a.py`, `ci.yml`, `x.yaml`, `Makefile`, `rules.mk`, `README.md` | not mechanical |
| `v.haml`, `v.pug`, `a.coffee`, `a.nim`, `a.fs` | not mechanical |
##### Verified by
- [test_mechanical.WhitespaceRuleTest.test_indentation_trailing_whitespace_and_blank_lines](../../tests/test_mechanical.py)
- [test_mechanical.WhitespaceRuleTest.test_whitespace_significant_files_are_never_whitespace_mechanical](../../tests/test_mechanical.py)
#### Scenario#11.07: a renamed file is mechanical only with identical content
So that a moved file with changes still gets a review.
- **Given** Gitea reports `new/A.java` as renamed from `old/A.java` with `<added>` added and `<removed>` removed lines
- **When** werkrevisor checks the file
- **Then** the file is `<result>`
| added | removed | result |
|---|---|---|
| 0 | 0 | mechanical (renamed) |
| 1 | 1 | not mechanical |
##### Verified by
- [test_mechanical.RenameRuleTest.test_rename_with_identical_content](../../tests/test_mechanical.py)
- [test_mechanical.RenameRuleTest.test_rename_with_changes_is_not_mechanical](../../tests/test_mechanical.py)
#### Scenario#11.08: a PR of mechanical files only is GREEN without a model call
So that such a PR costs no tokens.
- **Given** a PR changes `B.java` with imports only and `a.c` with whitespace only
- **When** werkrevisor reviews the PR
- **Then** the change kind is `mechanical_only`
- **and** the verdict is GREEN
- **and** neither Jev nor the model is called
##### Verified by
- [test_mechanical.FlowTest.test_all_mechanical_pr_is_green_without_model_call](../../tests/test_mechanical.py)
#### Scenario#11.09: a mechanical change of a risky path stays YELLOW
So that the hard rules keep their effect.
- **Given** a PR changes `src/main/java/net/example/auth/Login.java` with imports only
- **When** werkrevisor reviews the PR
- **Then** the verdict is YELLOW with the reason `risky_paths`
- **and** the model is not called
##### Verified by
- [test_mechanical.FlowTest.test_mechanical_change_of_a_risky_path_stays_yellow](../../tests/test_mechanical.py)
#### Scenario#11.10: changed imports in a test file are no weakened test
So that a feature PR that also cleans up test imports can still become GREEN.
- **Given** a PR changes main code and replaces an import in `ATest.java`
- **When** werkrevisor applies the rules
- **Then** there is no reason `tests_weakened`
- **and** the change kind is `refactoring`
##### Verified by
- [test_mechanical.RulesTest.test_import_only_change_in_a_test_file_is_not_test_weakening](../../tests/test_mechanical.py)
#### Scenario#11.11: mechanical files do not count for the size limits
So that a package move across many files does not need a human only because of its size.
- **Given** `[size] max_files = 30`
- **and** a PR changes 31 Java files with import lines only and one file with logic
- **When** werkrevisor applies the rules
- **Then** the size check counts 1 file
- **and** the PR does not become ORANGE because of its size
##### Verified by
- [test_mechanical.RulesTest.test_mechanical_files_do_not_count_for_the_size_limits](../../tests/test_mechanical.py)
#### Scenario#11.12: an invalid import regex is a config error
So that a typo in a regex cannot silently switch a rule off.
- **Given** a config layer sets `[mechanical.imports] java = ["(unclosed"]`
- **When** werkrevisor loads the config
- **Then** the result is ERROR with `[mechanical] imports.java has an invalid regex '(unclosed'`
##### Verified by
- [test_mechanical.ConfigTest.test_invalid_regex_is_a_config_error](../../tests/test_mechanical.py)
#### Scenario#11.13: the repository config replaces the import regexes per extension
So that a repository can add a language without repeating the built-in ones.
- **Given** the host config sets `[mechanical.imports] scala = [...]`
- **and** `.werkrevisor.toml` sets `[mechanical.imports] java = []`
- **When** werkrevisor merges the layers
- **Then** Scala imports are mechanical, Java imports are not, and TypeScript keeps its built-in rule
##### Verified by
- [test_mechanical.ConfigTest.test_layers_merge_the_imports_per_extension](../../tests/test_mechanical.py)
## The Solution
- `werkrevisor/mechanical.py` decides per changed file which rules explain every changed line.
It works on the hunks of the Gitea diff, which `diff.py` now keeps with their context lines.
- A multi-line import counts only if the whole statement is in one hunk, so missing context gives "not mechanical".
- The whitespace rule compares the changed lines like `git diff -b`:
an added space inside a line, e.g. in a string literal, is a change.
- A file whose diff has fewer lines than the counts of Gitea is not mechanical, because the missing lines could hold anything.
- `evaluate_rules` leaves the mechanical files out of the change kind, the test weakening check and the size limits;
a PR of mechanical files only gets the new change kind `mechanical_only`.
- `pr_agent_settings` passes the mechanical paths as exact `ignore.regex` entries,
so PR-Agent leaves them out of the diff it sends; `fullfiles.collect` leaves out their full content.
- `coverage_reasons` does not miss them, and the comment and the usage record (`mechanical_files`) list them.
- The new section `[mechanical]` is required in every merged config: `enabled`, `whitespace_significant` and `imports`.
The repository layer replaces `enabled` and `whitespace_significant`, and the regexes per extension of `imports`.
- `mechanical_only` is in the default `auto_approvable` and in the example configs.
`.werkrevisor.toml` stays unchanged; the host config on mih09 gets `[mechanical]` with the deploy of 0.13.0.
- README, `docs/konzept-jev-classifier.md` and `docs/betrieb-git.javagil.de.md` describe the change; werkrevisor 0.13.0.
## Open Questions
- [x] Is a renamed file with changed imports, e.g. a moved Java class with a new `package` line, mechanical?
No: a renamed file is mechanical only with identical content, so the model still reviews a move with changes.
- [x] Do mechanical files count for the `[size]` limits?
No: they need no AI judgement, as `docs/konzept-jev-classifier.md` intends for a package move across many files.
- [x] Does a changed space inside a line count as whitespace only, like `git diff -w`?
No: the rule follows `git diff -b`, because a space inside a string literal can change the behavior.
- [x] Is an added or deleted file with only import lines mechanical?
No: only a file that exists before and after the PR can be mechanical.
## Additional Changes
- The example host configs follow the configuration on mih09:
`config/werkprobe.toml`, `config/werkrevisor.toml` and `config/hsadmin-ng.toml` use `[engine] mode = "combined"`
and `[model] token_count_factor = 1.55`.
On werkrevisor PR#10 the factor 1.15 gave 49841 tokens, and Anthropic reported 65792 prompt tokens for Sonnet;
so `o200k_base` counts about 35 % too low, and 1.15 × 65792 / 49841 ≈ 1.52 is rounded up to 1.55.
- The test helper `config()` sets `[engine] mode = "llm"`, because most tests check the LLM path;
the classifier tests set their mode explicitly.
- The tests that depend on the factor 1.15 set it explicitly.
- A change kind in `[categories] auto_approvable` of a changed `.werkrevisor.toml` that the installed werkrevisor
does not know gives YELLOW like an unknown key, not RED:
a newer version may know it, e.g. `mechanical_only` while 0.12.0 is installed.
## Follow-up PRs
- After the deploy of 0.13.0, `.werkrevisor.toml` adds `mechanical_only` to `[categories] auto_approvable`, like PR#8.