feat: diff cap -> 1000-line runaway guard excluding tests; comment discipline in worker contract + review directive #467

New issue

Closed

opened 2026-08-09 00:54:34 +00:00 by igor · 0 comments

igor commented

2026-08-09 00:54:34 +00:00

Collaborator

Deliverables

Cap becomes a runaway guard: 1000 net lines, EXCLUDING test files.
- Locate the finalize-time scope gate (the hard block referenced in AGENTS.md "the harness HARD-BLOCKS larger PRs") in bin/tick.sh and change: threshold 400 -> 1000 (hardcoded constant, per house style -- no env knob), and the counted diff excludes files matching a hardcoded test-path classifier.
- Test classifier (deterministic, hardcoded): path matches any of (^|/)test[s]?/, (^|/)bin/test-[^/]+$, \.(test|spec)\.[a-z]+$, _test\.(go|py|rb|ex|exs)$, (^|/)spec/. Keep it a single function with its own unit test so the list is easy to extend.
- Rationale in a comment: excluding tests makes "delete tests to shrink the diff" structurally impossible (igor#411) and stops taxing coverage (igor#465).
AGENTS.md (the harness worker contract): replace the 400-line MANDATORY block.
- New guidance: the diff should be the size the task honestly requires -- no padding, no drive-by refactors (unchanged), and the hard block at 1000 non-test lines is a runaway guard, not a target.
- ADD comment discipline to the contract: comments exist only to state a constraint the code cannot show (a non-obvious why, an invariant). Never narration ("call the helper"), never what-the-next-line-does, never changelog-style "added X" comments. Shorter is better; zero comments on self-explanatory code is correct.
bin/lib/review-directive.md: two new explicit review dimensions.
- Diff-appropriateness: is this diff the size the task honestly requires? Padding, redundant abstraction, and drive-by changes are findings.
- Comment bloat: unnecessary or narrating comments are findings with the same standing as any other -- the reviewer is licensed to REQUEST_CHANGES on comment noise alone if pervasive.
Tests: update whatever bin/test-*.sh covers the scope gate to the new threshold + exclusion semantics; add classifier unit cases (positive and negative paths).

Notes

AGENTS.md and bin/lib/review-directive.md are prompt surfaces -- treat edits like a deploy (they take effect fleet-wide on the next tick after merge).
Keep the checkpoint/finalize interaction unchanged: the gate still runs only at finalize, never on a checkpoint.
The make test contract check (bin/check-sync.sh) greps AGENTS.md/tick.sh agreements -- make sure the reworded AGENTS.md block keeps whatever sentinels it needs.

The 400-line diff cap measures the wrong thing. Evidence: igor#411 (a worker deleted its own passing tests to duck under the cap -- the cap made the PR worse) and igor#465 (overage was pure failure-mode test coverage; the human waived it). Meanwhile the actual concern -- padding, comment bloat, narration -- is invisible to a line counter. Rework the cap into a runaway guard and move sizing judgment to the review, per the operator's 2026-08-08 direction. ## Deliverables 1. **Cap becomes a runaway guard: 1000 net lines, EXCLUDING test files.** - Locate the finalize-time scope gate (the hard block referenced in `AGENTS.md` "the harness HARD-BLOCKS larger PRs") in `bin/tick.sh` and change: threshold 400 -> 1000 (hardcoded constant, per house style -- no env knob), and the counted diff excludes files matching a hardcoded test-path classifier. - Test classifier (deterministic, hardcoded): path matches any of `(^|/)test[s]?/`, `(^|/)bin/test-[^/]+$`, `\.(test|spec)\.[a-z]+$`, `_test\.(go|py|rb|ex|exs)$`, `(^|/)spec/`. Keep it a single function with its own unit test so the list is easy to extend. - Rationale in a comment: excluding tests makes "delete tests to shrink the diff" structurally impossible (igor#411) and stops taxing coverage (igor#465). 2. **`AGENTS.md` (the harness worker contract): replace the 400-line MANDATORY block.** - New guidance: the diff should be the size the task honestly requires -- no padding, no drive-by refactors (unchanged), and the hard block at 1000 non-test lines is a runaway guard, not a target. - ADD comment discipline to the contract: comments exist only to state a constraint the code cannot show (a non-obvious why, an invariant). Never narration ("call the helper"), never what-the-next-line-does, never changelog-style "added X" comments. Shorter is better; zero comments on self-explanatory code is correct. 3. **`bin/lib/review-directive.md`: two new explicit review dimensions.** - Diff-appropriateness: is this diff the size the task honestly requires? Padding, redundant abstraction, and drive-by changes are findings. - Comment bloat: unnecessary or narrating comments are findings with the same standing as any other -- the reviewer is licensed to REQUEST_CHANGES on comment noise alone if pervasive. 4. **Tests:** update whatever `bin/test-*.sh` covers the scope gate to the new threshold + exclusion semantics; add classifier unit cases (positive and negative paths). ## Notes - `AGENTS.md` and `bin/lib/review-directive.md` are prompt surfaces -- treat edits like a deploy (they take effect fleet-wide on the next tick after merge). - Keep the checkpoint/finalize interaction unchanged: the gate still runs only at finalize, never on a checkpoint. - The `make test` contract check (`bin/check-sync.sh`) greps AGENTS.md/tick.sh agreements -- make sure the reworded AGENTS.md block keeps whatever sentinels it needs.

igor added the

Agent

label

2026-08-09 00:54:35 +00:00

igor self-assigned this

2026-08-09 01:02:03 +00:00

igor referenced this issue from a pull request that will close it,

2026-08-09 01:13:32 +00:00

feat: turn the 400-line diff cap into a 1000-line runaway guard excluding tests #470

igor added spent time

2026-08-09 01:13:33 +00:00

11 minutes 1 second