98 lines
4.2 KiB
Markdown
98 lines
4.2 KiB
Markdown
# PR#29: larger full files and a wider diff context instead of YELLOW
|
|
|
|
> **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
|
|
|
|
With `[review] full_files = true`, werkrevisor gives the model each changed text file in full,
|
|
up to `max_full_file_chars = 40000` characters per file, counted with the line numbers that werkrevisor adds.
|
|
A larger file goes to the model only as diff, and the review gives YELLOW.
|
|
|
|
`werkrevisor/cli.py` has about 39300 characters, with line numbers more than 40000,
|
|
so each PR that changes it gets YELLOW; PR#27 and PR#28 got it for `cli.py` and `config.py`.
|
|
The diff that the model gets instead shows only about 8 lines before and 4 after each hunk,
|
|
so the model can neither see the enclosing function nor what the change calls.
|
|
PR-Agent 0.47.0 caps its extra diff context at 10 lines, so its settings cannot widen it enough.
|
|
|
|
## Non-Goals
|
|
|
|
- No selection of code by a second model: a follow-up PR does that, with a measurement against full files.
|
|
- No change for a file that is not UTF-8 text: the model cannot read it in any form, so it stays YELLOW.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: larger files go to the model in full
|
|
|
|
#### Scenario#29.01: a changed file up to 80000 characters goes to the model in full
|
|
|
|
So that the model can check a change of `cli.py` against the rest of the file.
|
|
|
|
- **Given** a PR changes a text file of 60000 characters with line numbers
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** the model gets the file in full
|
|
- **and** no reason says that the file was given only as diff
|
|
|
|
##### Verified by
|
|
|
|
- [test_fullfiles: test_default_limits_take_a_file_of_60000_characters](../../tests/test_fullfiles.py)
|
|
|
|
### Feature: a file too large for the model in full goes as excerpt
|
|
|
|
#### Scenario#29.02: a file beyond the full-file limits goes as excerpt and is a hint, not YELLOW
|
|
|
|
So that a large file alone needs no human, while the model still sees the surroundings of each change.
|
|
|
|
- **Given** a PR changes a text file larger than `<limit>`
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** the model gets an excerpt of the file with 40 lines before and after each changed line, numbered like a full file
|
|
- **and** the result names the file in a hint
|
|
|
|
| limit |
|
|
|---|
|
|
| `[review] max_full_file_chars` |
|
|
| the rest of `[review] max_full_files_chars` |
|
|
|
|
##### Verified by
|
|
|
|
- [test_fullfiles: test_files_beyond_the_limits_go_as_excerpt_and_are_a_hint](../../tests/test_fullfiles.py)
|
|
- [test_fullfiles: test_a_large_file_as_excerpt_can_be_green](../../tests/test_fullfiles.py)
|
|
|
|
#### Scenario#29.03: a file that goes only as diff stays YELLOW
|
|
|
|
So that a file whose surroundings the model cannot see still needs a human.
|
|
|
|
- **Given** a PR changes a file that `<case>`
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** the review gives YELLOW
|
|
- **and** the reason says that the model got the file only as diff
|
|
|
|
| case |
|
|
|---|
|
|
| is not UTF-8 text |
|
|
| is too large in full and as excerpt |
|
|
|
|
##### Verified by
|
|
|
|
- [test_fullfiles: test_non_text_files_stay_yellow](../../tests/test_fullfiles.py)
|
|
- [test_fullfiles: test_an_excerpt_beyond_the_limits_leaves_only_the_diff](../../tests/test_fullfiles.py)
|
|
|
|
## The Solution
|
|
|
|
- `[review] max_full_file_chars` is 80000 and `max_full_files_chars` 160000 by default and in the example host configs;
|
|
`.werkrevisor.toml` no longer sets them, since a repository config may only lower the host value;
|
|
with line numbers, `werkrevisor/cli.py` has 44901 characters and `werkrevisor/config.py` 42771.
|
|
- A file beyond a limit goes as excerpt in the section of the full files:
|
|
the changed lines with `[review] excerpt_context_lines` lines before and after, default 40,
|
|
merged where they overlap, numbered like a full file, and counted against both limits.
|
|
- A file that went as excerpt gives the hint `full_files_partial`;
|
|
a file that goes only as diff keeps the YELLOW reason `full_files_incomplete`.
|
|
- The README diagram shows this context as its own step of the workflow code.
|
|
- The Werkator machine config of werkprobe and werkator sets the new limits at the deployment.
|
|
- werkrevisor 0.19.2.
|
|
|
|
## Prerequisite PRs
|
|
|
|
- PR#27: judged build paths.
|