werkator build succeeded after 00:05
werkrevisor 0.9.0: deterministic writing rules for changed docs, and jev-eval with the config layers of the review. - Changed Markdown lines are checked against the AGENTS.md writing style: one sentence per line, line length, sentences per paragraph. Violations give ORANGE; the model still runs. - The unreliable Jev question about the writing rules is removed. - `tools/jev-eval` reads defaults < host < repository config. Details: the PR document under `docs/prs/`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed-on: #6 Co-authored-by: claude-code <claude-code@javagil.de>
271 lines
11 KiB
Markdown
271 lines
11 KiB
Markdown
# PR#6: Deterministic writing rules and config layers for jev-eval
|
|
|
|
> **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 writing style of `AGENTS.md` has three countable rules:
|
|
one sentence per line, at most 128 characters per line, and at most 5 sentences per paragraph.
|
|
Today only the model and the Jev question `writing_rules_violated` check them.
|
|
In the measurement with `tools/jev-eval`, that question flagged all 34 sections, so it does not tell good from bad text.
|
|
|
|
`tools/jev-eval` reads only the host config.
|
|
The review puts `.werkrevisor.toml` on top of it, so the measurement can use other questions than the review.
|
|
|
|
## Non-Goals
|
|
|
|
- No check of the other style rules, e.g. active voice or simple words; they stay with the model.
|
|
- No RED for a writing rule: a style rule must not block hard.
|
|
- No `.werkrevisor.toml` for hsadmin-ng.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: deterministic writing rules for changed docs
|
|
|
|
#### Background
|
|
|
|
- The config is `config/werkprobe.toml` with `[writing] max_line_length = 128` and `max_sentences_per_paragraph = 5`.
|
|
- The PR changes the doc file `doc/guide.md`.
|
|
|
|
#### Scenario#6.01: two sentences on one changed line give ORANGE
|
|
|
|
So that a human sees the violation, while the model still reviews the PR.
|
|
|
|
- **Given** a changed line `The gate stops. A human decides.`
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** the verdict is ORANGE with the reason
|
|
`writing rules violated in the changed docs: doc/guide.md:1: more than one sentence on a line`
|
|
- **and** the model reviews the PR
|
|
|
|
##### Verified by
|
|
|
|
- [test_writing.WritingFlowTest.test_violation_is_orange_and_the_model_still_reviews](../../tests/test_writing.py)
|
|
- [test_writing.LineRuleTest.test_more_than_one_sentence_on_a_line](../../tests/test_writing.py)
|
|
|
|
#### Scenario#6.02: abbreviations, numbers, file names, code spans and URLs do not end a sentence
|
|
|
|
So that the rule gives no false finding.
|
|
|
|
- **Given** a changed line `<line>`
|
|
- **When** werkrevisor checks the writing rules
|
|
- **Then** the line has one sentence
|
|
|
|
| line |
|
|
|---|
|
|
| `Use a glob, e.g. Docs or Tests, i.e. A pattern.` |
|
|
| `Das gilt z. B. Für Tests, d. h. Nicht für Doku, usw. Und mehr.` |
|
|
| `See Nr. 5 vs. Version 1.5 and werkrevisor 0.9.0 in config.py.` |
|
|
| ``Call `x. Y` or https://example.org/a. B with a mark.`` |
|
|
|
|
##### Verified by
|
|
|
|
- [test_writing.SentenceSplitTest.test_abbreviations_numbers_and_file_names_do_not_split](../../tests/test_writing.py)
|
|
- [test_writing.SentenceSplitTest.test_code_spans_and_urls_do_not_split](../../tests/test_writing.py)
|
|
|
|
#### Scenario#6.03: a changed line longer than 128 characters is a violation
|
|
|
|
So that a reader does not scroll and a diff stays readable.
|
|
|
|
- **Given** a changed prose line of 129 characters
|
|
- **When** werkrevisor checks the writing rules
|
|
- **Then** the violation is `doc/guide.md:1: line longer than 128 characters (129)`
|
|
|
|
##### Verified by
|
|
|
|
- [test_writing.LineRuleTest.test_long_line_is_a_violation](../../tests/test_writing.py)
|
|
|
|
#### Scenario#6.04: some long lines are allowed
|
|
|
|
So that tables, headings, links and code stay as they are.
|
|
|
|
- **Given** a changed line of 200 characters that is `<kind>`
|
|
- **When** werkrevisor checks the writing rules
|
|
- **Then** there is no violation
|
|
|
|
| kind |
|
|
|---|
|
|
| a table row |
|
|
| a heading |
|
|
| a line with only a link or a URL |
|
|
| in a fenced code block |
|
|
| in an HTML comment |
|
|
| in the front matter |
|
|
|
|
##### Verified by
|
|
|
|
- [test_writing.LineRuleTest.test_long_line_exceptions](../../tests/test_writing.py)
|
|
|
|
#### Scenario#6.05: a changed paragraph with more than 5 sentences is a violation
|
|
|
|
So that a paragraph stays short.
|
|
|
|
- **Given** a paragraph of 6 sentences on 6 lines, and one of them changed
|
|
- **When** werkrevisor checks the writing rules
|
|
- **Then** the violation is `doc/guide.md:1: paragraph with 6 sentences, more than 5`
|
|
- **and** the items of a list count as separate paragraphs
|
|
|
|
##### Verified by
|
|
|
|
- [test_writing.ParagraphRuleTest.test_paragraph_with_more_than_five_sentences](../../tests/test_writing.py)
|
|
- [test_writing.ParagraphRuleTest.test_list_items_are_separate_paragraphs](../../tests/test_writing.py)
|
|
|
|
#### Scenario#6.06: only added or changed lines count
|
|
|
|
So that old text does not block a new PR.
|
|
|
|
- **Given** a file with violations in lines that the PR does not change
|
|
- **When** werkrevisor checks the writing rules
|
|
- **Then** there is no violation
|
|
- **and** a paragraph counts only if the PR changes one of its lines
|
|
|
|
##### Verified by
|
|
|
|
- [test_writing.ChangedLinesTest.test_untouched_lines_and_paragraphs_do_not_count](../../tests/test_writing.py)
|
|
- [test_writing.ChangedLinesTest.test_a_changed_line_makes_its_paragraph_count](../../tests/test_writing.py)
|
|
|
|
#### Scenario#6.07: one reason with at most 3 examples
|
|
|
|
So that the reason stays one line.
|
|
|
|
- **Given** 5 violations
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** one reason lists 3 violations as `file:line: rule` and `and 2 more`
|
|
|
|
##### Verified by
|
|
|
|
- [test_writing.ReasonTest.test_one_reason_with_three_examples_and_the_count_of_the_rest](../../tests/test_writing.py)
|
|
|
|
#### Scenario#6.08: the docs category and the PR documents are checked
|
|
|
|
So that every Markdown doc follows the same writing style.
|
|
|
|
- **Given** `[writing] globs` is `<globs>`
|
|
- **When** a PR changes `<file>`
|
|
- **Then** werkrevisor checks it: `<checked>`
|
|
|
|
| globs | file | checked |
|
|
|---|---|---|
|
|
| `[]` | `doc/guide.md` (docs category) | yes |
|
|
| `[]` | `src/main/Foo.java` | no |
|
|
| `["docs/prs/*.md"]` | `doc/guide.md` | no |
|
|
| `["doc/*.txt"]` | `doc/guide.txt` | no, not Markdown |
|
|
| `[]` with `[pr_doc] globs = ["notes/*.md"]` | `notes/pr.md` | yes |
|
|
| `[]` with `enabled = false` | `doc/guide.md` | no |
|
|
|
|
##### Verified by
|
|
|
|
- [test_writing.SelectionTest.test_which_files_are_checked](../../tests/test_writing.py)
|
|
|
|
### Feature: the `[writing]` config section
|
|
|
|
#### Scenario#6.09: the repository config may set `[writing]`
|
|
|
|
So that a repository keeps its own writing rules next to its code.
|
|
|
|
- **Given** the host config sets `[writing] max_line_length = 128`
|
|
- **and** `.werkrevisor.toml` sets `[writing] max_line_length = 100`
|
|
- **When** werkrevisor builds the config
|
|
- **Then** `max_line_length` is 100
|
|
- **and** a config without `[writing]` in any layer is an error
|
|
|
|
##### Verified by
|
|
|
|
- [test_config_layers.LayeringTest.test_repository_sets_the_writing_rules](../../tests/test_config_layers.py)
|
|
- [test_writing.ConfigTest.test_writing_section_is_required_and_validated](../../tests/test_writing.py)
|
|
|
|
### Feature: no Jev question about the writing rules
|
|
|
|
#### Scenario#6.10: the example configs ask Jev no writing-rules question
|
|
|
|
So that Jev asks only questions it can answer.
|
|
|
|
- **Given** the example configs and `.werkrevisor.toml`
|
|
- **When** werkrevisor builds the config
|
|
- **Then** the section questions are `hard_to_understand` and `slop`
|
|
|
|
##### Verified by
|
|
|
|
- [test_classifier.ConfigTest.test_examples_have_default_questions_and_keep_llm_mode](../../tests/test_classifier.py)
|
|
|
|
### Feature: `tools/jev-eval` uses the config layers of the review
|
|
|
|
#### Scenario#6.11: `--repo-config` puts a repository config on top of the host config
|
|
|
|
So that the measurement asks the same questions as the review.
|
|
|
|
- **Given** the host config `config/werkprobe.toml`
|
|
- **and** `--repo-config` names a file with one section question `only_this`
|
|
- **When** `tools/jev-eval` judges a PR document
|
|
- **Then** Jev gets only the question `only_this` for the sections
|
|
|
|
##### Verified by
|
|
|
|
- [test_classifier.JevEvalTest.test_repo_config_is_layered_on_top_of_the_host_config](../../tests/test_classifier.py)
|
|
|
|
#### Scenario#6.12: `.werkrevisor.toml` in the current directory is the default
|
|
|
|
So that a measurement in a repository uses its review rules without an option.
|
|
|
|
- **Given** the current directory has a `.werkrevisor.toml`
|
|
- **When** `tools/jev-eval` runs without `--repo-config`
|
|
- **Then** it uses that file
|
|
- **and** with `--no-repo-config` it uses the host config alone
|
|
|
|
##### Verified by
|
|
|
|
- [test_classifier.JevEvalTest.test_werkrevisor_toml_in_the_current_directory_is_the_default](../../tests/test_classifier.py)
|
|
|
|
#### Scenario#6.13: the repository config of jev-eval follows the rules of the review
|
|
|
|
So that a measurement cannot use a config that the review would refuse.
|
|
|
|
- **Given** `--repo-config` names a file that sets `[classifier] endpoint`
|
|
- **When** `tools/jev-eval` runs
|
|
- **Then** it ends with exit code 30
|
|
- **and** the message is `must not set [classifier] endpoint: only the host config sets it`
|
|
|
|
##### Verified by
|
|
|
|
- [test_classifier.JevEvalTest.test_repo_config_follows_the_rules_of_the_review](../../tests/test_classifier.py)
|
|
|
|
## The Solution
|
|
|
|
- `werkrevisor/writing.py` checks the changed Markdown files of the `docs` category and the PR documents.
|
|
It reads the new version at the head commit and takes the added line numbers from the diff.
|
|
- A line counts if the PR adds or changes it; a paragraph counts if it contains such a line.
|
|
Each list item is a paragraph of its own.
|
|
- Table rows, headings, link-only lines, fenced code blocks, HTML comments and the front matter have no line limit.
|
|
- The sentence split is conservative:
|
|
a sentence ends at `.` or `!` before whitespace and a capital letter.
|
|
Abbreviations, initials, numbers, code spans, URLs and link targets end no sentence.
|
|
A question mark ends no sentence, because the PR documents answer a question on the same line.
|
|
- A violation gives ORANGE with one reason: at most 3 violations as `file:line: rule`, then `and N more`.
|
|
The check runs before the model call and does not skip it.
|
|
- The new section `[writing]` (`enabled`, `max_line_length`, `max_sentences_per_paragraph`, `globs`) is required.
|
|
The host config and `.werkrevisor.toml` may set all of its keys.
|
|
`config/werkprobe.toml`, `config/hsadmin-ng.toml` and `.werkrevisor.toml` set it;
|
|
`config/werkrevisor.toml` keeps only the host part.
|
|
- The example configs and `.werkrevisor.toml` no longer ask Jev the question `writing_rules_violated`.
|
|
- `tools/jev-eval` builds its config like the review: defaults, host config, repository config.
|
|
`--repo-config FILE` names the repository config; the default is `.werkrevisor.toml` in the current directory.
|
|
`--no-repo-config` uses the host config alone, e.g. to measure hsadmin-ng from the werkrevisor checkout.
|
|
- werkrevisor 0.9.0; the README, the classifier concept and the operations guide describe the change.
|
|
|
|
## Open Questions
|
|
|
|
- [x] Does a question mark end a sentence?
|
|
Yes: an answer goes on an indented continuation line of the same list item.
|
|
The open-questions rule counts the whole item as checked, not its source lines.
|
|
- [x] Does a number before a period end a sentence?
|
|
No: it is mostly an ordinal or a list number, e.g. `3. Oktober`.
|
|
- [x] Does `config/werkrevisor.toml` get a `[writing]` section?
|
|
No: it holds only the host part; `.werkrevisor.toml` sets `[writing]` for werkrevisor.
|
|
- [x] Does a long code span make a line exempt?
|
|
No: only the kinds of Scenario#6.04 are exempt.
|
|
|
|
## Additional Changes
|
|
|
|
- Two existing tests no longer depend on the removed question or on a one-line document of 500 characters.
|