Skip to content

feat(lint): gate tests/ on the prose rules, and cap the comment run as well as the file total - #180

Merged
uipreliga merged 21 commits into
mainfrom
docs/slim-tests-prose
Sep 16, 2026
Merged

uipreliga merged 21 commits into
mainfrom
docs/slim-tests-prose

Conversation

@uipreliga

@uipreliga uipreliga commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Applies the prose rules this repo enforces on src/coder_eval to tests/, turns the gate on for it, and — with tests/lint/ finally in scope — fixes the one gate rule that turned out to be measuring the wrong thing. Follows #177, which did the same for src/, and is rebased onto #176.

Four things happen here.

1. The gate now scans tests/ as well as src/

tests/lint/prose_budget.py scans a _ROOTS tuple instead of one hard-coded root, reports repo-relative paths, and fails when a root does not exist — in the scan and in --assert-code-unchanged, so a renamed root can never pass on zero files. _python_files is the single traversal; main prints collect_failures(). Phase 8 flips the default to (src/coder_eval, tests).

That put the essay police on their own beat for the first time. 67 docstrings over 150 prose words and 5 files over their comment budget, cleared:

Phase Scope Result
3 30 rule docstrings in tests/lint/rules/ 8,347 → 3,328 prose words
4 11 doc-surface lint tools + 4 TestCExxx classes 4,976 → 1,928 prose words
5 _scrub.py, _bracket_clock.py, opencode_fixtures.py 5 essays cleared
6 14 plain test modules 15 essays + 3 comment budgets
7 all of tests/ ~120 HISTORY lines, 16 long test docstrings

Across the branch, tests/lint/ goes from 18,265 to 11,668 docstring prose words and all of tests/ from 105,305 to 95,722.

Each docstring keeps its contract and hazards; the defect story moves behind a resolving Rationale: <path> § <heading> pointer. Rule rationale now lives in the new .claude/notes/lint-rules.md (one section per CE id, split verbatim out of .claude/notes/README.md); test rationale goes to the subsystem note it belongs to.

Dangling citations fixed: harbor/packager.py and tests/test_harbor_packager.py cited a module docstring that no longer held the claim; tests/test_harbor_agent.py cited one that never did.

Prose-only, proved

Every prose phase passes --assert-code-unchanged, which compares the docstring-stripped AST and the directive comments against the base.

Gate holes the final review found, and closed here

Three ways the tool could pass without checking what it claims (each fixed test-first):

  • assert_code_unchanged compared directive comments one way, so adding a # noqa passed a "prose-only" proof.
  • main() ran the ordinary gate for any argv, so a mistyped --assert-code-unchanged exited 0 having proved nothing.
  • The file list came from git diff --name-only, which never names an untracked file, so a new .py file full of code passed.

Two further gaps needed design work and are deferred in .claude/harness-candidates.md: narrative after an Args: block that follows a pointer, and pointer targets outside .claude/notes/.

2. Rebased onto #176, which brought five new essays

396c22cc (the reports split) landed three new modules and reworded two more, all over the 150-word bar once this branch's gate sees them:

tests/lint/rules/_layers.py                       432w -> 137w
tests/lint/rules/ce066_no_report_imports_in_core   313w -> 121w
tests/lint/pricing_mirror.py                       311w -> 143w
tests/test_custom_lint.py::TestRuffExternal...     246w ->  93w
tests/lint/rules/no_cli_imports_in_core.py         235w -> 117w

New notes sections carry what moved: CE004, CE065, CE066, _layers, TestRuffExternalCoversEveryRule. Three merge conflicts resolved: ce053's docstring (kept the slimmed side), harness-candidates.md (kept both appends) and the Makefile (kept pricing-mirror and the new docs-budget help text).

3. CE023 deleted — it guarded a package that no longer exists

src/coder_eval/proxy/ is gone; CHANGELOG.md:2487 records deleting the coder_eval.proxy.pricing shim. The rule banning imports of it stayed behind, so it could not fire, and its docstring still described the shim as present.

Removed from all five places a rule lives: the module, its runner.py import and ALL_RULES entry, its test class, and its id in [tool.ruff.lint] external. The id stays retired, as runner.py already requires for CE062 — both are now recorded in .claude/notes/lint-rules.md § The CE id space, which replaces the 14-line comment that used to carry it.

46 → 45 registered rules.

4. The comment budget was inverted, so the rule is now the comment RUN

With tests/lint/ in scope I measured what MAX(20, 0.15 × lines) on a file's total comment lines was actually doing. It was backwards:

file shape old rule
isolation/docker_runner.py 229 comments, 62 runs of 1–2 lines, longest run 6 229/229 — blocked
tests/test_regrade.py one 16-line paragraph 29% of budget — fine
src/coder_eval/criteria/base.py one 13-line paragraph 31% of budget — fine

It taxed short annotations pinned to the line they explain, and waved through the paragraphs that .claude/notes/ exists to hold. Four files had settled at exactly 100.0% of budget (docker_runner.py 229/229, criteria/agent_judge.py 65/65, evaluation/sub_agent.py 34/34, config.py 33/33) — nobody lands on 229/229 by accident; the budget had become a target.

So a third rule is added, and it is the primary one:

No own-line comment run may exceed 8 lines.

A run reads through one blank line, so splitting a paragraph does not duck it, and through none at two, which is the separation PEP 8 already puts between a banner and its section. It caps a shape, with no per-file allowance: carry as many one-line notes as earn their place.

Measured before choosing 8: 78.5% of runs in the tree are 1–2 lines, 2.5% are 6 or more, and the longest was 16. The cap is below the current maximum, as a cap rule must be. 24 runs across 19 files were shortened or moved to .claude/notes/ behind a pointer — 6 in src/, 18 in tests/.

The file total is kept as the backstop underneath, unchanged at MAX(20, 0.15 × lines): a file can pass the run cap with every block short and still be mostly commentary. test_it_catches_what_the_run_cap_cannot pins that seam — thirty three-line blocks pass the run cap and fail the total — so neither rule can later be deleted as redundant without a red test.

make docs-budget now enforces three rules on both roots: comment run ≤ 8 lines, file total ≤ MAX(20, 0.15 × lines), docstring ≤ 150 prose words.

Worth a human's eye

  • The four files still at 100% of their total budget are frozen: a comment added to one has to be paid for by deleting another, or by raising the ratio. Recorded in .claude/notes/README.md with the measurements, so the next person to hit it sees the trade rather than bumping the number blind.
  • src/ is touched, in 6 files — all of them one over-long comment run each, moved to notes behind a pointer. No behaviour change.
  • .claude/notes/README.mddocs: move design rationale out of src/ into .claude/notes, gate it, and slim CLAUDE.md #177 had rewritten that section to say no prose summary is kept there; this branch points it at lint-rules.md. The rule file owns invariant/scope/blind spots, lint-rules.md owns the motivating defect, and the rule file wins on disagreement.
  • Prose moved rather than shrank. tests/ loses ~12,700 docstring words and .claude/notes/ gains ~12,400. .claude/ has no prose budget, so the repo total is roughly flat; that is the design (rationale has one home) but it is worth stating plainly rather than claiming a reduction.

Verification

make verify: 5,816 passed, 2 skipped, 92.63% coverage, 671 custom lint rules — ruff format, ruff check, pyright, lint suite, prose gate, full test run. The gate reports 0 essays, 0 over-long runs and 0 files over budget with both roots in scope.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown

Claude finished @uipreliga's task in 1m 31s —— View job


Code Review In Progress

Todo List:

  • Read code review guidelines from .github/code_review.md
  • Read project conventions from CLAUDE.md
  • Get full PR diff with git diff origin/main...HEAD
  • Review key changed files:
    • tests/lint/prose_budget.py (the gate implementation)
    • .claude/notes/lint-rules.md (new rule rationale file)
    • Sample rule files to verify prose cleanup
  • Perform cross-file consistency checks
  • Check "what's missing" analysis
  • Verify prose-only proof claim
  • Write comprehensive review feedback

Starting review now...

uipreliga and others added 16 commits September 15, 2026 21:39
prose_budget.py scans a module-level _ROOTS tuple instead of one hard-coded
src/coder_eval root. _python_files is the single rglob loop; a root that is
not a directory raises FileNotFoundError, in the scan and in
assert_code_unchanged, so a renamed root fails the gate instead of passing it
on zero files. Reported paths and _TYPER_COMMANDS keys are repo-relative.
main prints collect_failures(); its output and exit codes are unchanged.

Gate scope is unchanged: _ROOTS is (src/coder_eval,), TOTAL still 17752.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-rules.md

The 2,165-word catalogue paragraph in .claude/notes/README.md is split
verbatim into 19 "## CExxx" sections of the new .claude/notes/lint-rules.md
(2,138 words; the 27-word difference is the **CExxx** ( markers and the
lead-in). README lists the file and says the rule file owns invariant, scope
and blind spots; lint-rules.md owns the motivating defect. CLAUDE.md now says
the same.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s.md

The 30 rule module docstrings in tests/lint/rules/ over 150 prose words now
keep CONTRACT (invariant, forms, scope) and HAZARD (blind spots, noqa
guidance) and point at .claude/notes/lint-rules.md § <id> for the defect
story. Module docstring prose: 8,347 -> 3,328 words.
yaml_models_forbid_extras.py own-line comments: 27 -> 7 (budget 22).

New sections: CE009 CE013 CE014 CE020 CE021 CE022 CE024 CE032 CE043 CE046
CE061 _model_ctor. Appended to the Phase-2 sections where new facts existed;
facts repeated between a Phase-2 paragraph and the appended text were merged.

No pointer (nothing moved, compression only): ce019.
Line 1 of every docstring is unchanged.

"blind spot" counts, HEAD -> now: _model_ctor 1->1, ce020 0->1,
ce058 1->1, ce059 1->1, ce060 2->2, ce061 2->2, ce063 1->1, ce064 1->1,
yaml_models_forbid_extras 0->2, all others 0->0.

Disagreements resolved (the more precise version kept):
- § CE058 said "five syntactic forms"; the rule has six. The notes now defer
  the list to the docstring. Forms 2-4 flag any number, 1/5/6 zero only.
- § CE059 "four suppressions" kept (four generation_duration_ms=None sites).
- § CE063 said inverting _imports_the_helper flags four of five reducers; all
  five import close_window by name, so it is corrected.
- ce019: the helpers that do file I/O run only inside init_telemetry's guard;
  the rest reach side effects through track_event.

Review also corrected three misstated contracts (ce053 match scope, ce063
bookkeeping sentence cited by _scrub.py, ce058 field scope) and one wrong
comment in ce054 (_written_keys counts only subscript assignment).

No executable statement changed — proved by assert_code_unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…les.md

The 11 lint tool modules under tests/lint/ and four TestCExxx classes in
tests/test_custom_lint.py keep their invariant, scope, opt-out markers and
blind spots (these rules have no rule module, so the docstring is their only
home) and point at .claude/notes/lint-rules.md § <id>. The 17 essay
docstrings: 4,976 -> 1,928 prose words. doc_env_parity.py own-line comments:
31 -> 12 (budget 22).

New sections: CE026 CE027 CE028 CE029 CE030 CE031 CE033 CE034 CE035 CE036
CE044 CE045 TestRunRecordFieldVocabulary. Appended: CE047, CE055. The
"not a BaseRule in the AST runner" reason stays in each module docstring;
the notes keep it only where the docstring gives no reason (CE026, CE033,
CE036).

Disagreements resolved:
- CE044: the old docstring said "Six fields" and listed eight; SHARED_KEYS
  has eight.
- CE047: the section's "four of those seven" count kept over "most".
- TestRunRecordFieldVocabulary: _known_fields joins three models, not two.

Review also removed facts repeated between docstrings and notes (CE045, CE055)
and put fixed defects in the past tense (CE028, CE034).

No executable statement changed — proved by assert_code_unchanged.
`make docs-indexes plugin-reference` leaves README.md, docs/ and plugins/
unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…o notes

The five essay docstrings in the test helpers keep their contract and hazards
and point at the notes:
- tests/_bracket_clock.py (module 181 -> 85, assert_overhead_is_measured
  175 -> 59) -> lint-rules.md § CE064
- _scrub.py::_tool_union_ms (153 -> 73) and ::assert_timing_captured
  (720 -> ~120) -> timing.md § The golden-stream timing sensor (new ###
  under § decompose_turn; nothing covered the sensor before)
- opencode_fixtures.py (165 -> 100) -> agents.md § Why a clean exit can
  still be a crash

Kept: "run on the UNSCRUBBED dump", the do-not-simplify hazard on
_tool_union_ms (its CE063 citation still resolves), the strict > 0.0 tail
hazard, the do-not-correct-toward-session.next.* hazard, and that the
identity check is one-sided on purpose.

No executable statement changed — proved by assert_code_unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…m notes

Fifteen essay docstrings in plain test modules now state what they pin plus at
most one hazard, and point at the subsystem note that holds the defect story:
- timing.md: test_timing_identity_contract (module, _claude_turn,
  _claude_slow_result_turn), test_timing_close_window, test_agent_telemetry
  (three new ### subsections)
- reporting.md § Plugin and GitHub Action layout: test_action_inputs,
  test_action_version_pin, test_pr_review_workflow,
  test_verify_published_workflow (four new ### subsections)
- agents.md: test_antigravity_agent (§ The receive_steps re-entrancy window),
  test_opencode_agent (§ Per-harness generation marks)
- isolation.md: test_sandbox_venv_live
- test_agent.py: HISTORY only, deleted
Comment density: test_agent_golden_master 107 -> 50, test_early_stop
516 -> ~440, test_new_criteria 41 -> 27.

Review corrected a false comment added in test_early_stop.py (NoOpAgent does
not support the cooperative interrupt), restored the class-level summaries
the deleted banners carried as class docstrings, and removed text repeated
between timing.md subsections.

The 14 files carry 13 Rationale pointers (0 before).
No executable statement changed — proved by assert_code_unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 3/8 slimming joined two docstring lines into one of 124 characters, which
ruff E501 flags. Prose only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tring citations

HISTORY sweep over every docstring and comment under tests/: about 120 lines
that said "used to", "no longer", "previously", "pre-fix" or "an earlier
version" now state the current contract in the present tense, or are deleted.
No intentional hit is left; both listings from the plan print nothing:
- HISTORY regex over prose lines: 0 hits (was 124)
- test-function docstrings over 100 prose words: 0 (was 16)

CE006 and CE007 first lines are present-tense contracts. CE007 drops the word
"fields" from the planned text, which is 122 characters and fails ruff E501.
CE006 now says "see CE007" instead of claiming the fields are top-level.

Dangling "see ... docstring" citations fixed:
- src/coder_eval/harbor/packager.py `_write_verifier_task_yaml` docstring and
  comment, and tests/test_harbor_packager.py (x2) now point at
  reporting.md § The non-obvious constraint in the emitted task.yaml
- tests/test_harbor_agent.py cites the ImportError in harbor/agent.py, which
  holds the claim; the module docstring never did
The shape is recorded in .claude/harness-candidates.md (not mechanised).

Notes: reporting.md said the Action's env passthrough exports into the step
shell; action.yml collects it and hands it to `env --`. Corrected in place.

Review corrected rewrites that were false: a hazard in test_regrade.py named
a symbol that does not exist; test_task_definition_unknown_fields.py listed
`skip` as undeclared although TaskDefinition declares it; test_agent.py said
the rollback keys on one signal where the code ORs two; and agents.md said the
four-bucket identity is only an upper bound, which a test now checks exactly.

No executable statement changed — proved by assert_code_unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tests/lint/prose_budget.py scans _ROOTS = (src/coder_eval, tests), so
`make docs-budget` (and `make verify`) now fail on a docstring over 150 prose
words, an own-line comment budget breach, or an unresolved/misplaced
Rationale pointer anywhere under tests/.

- test_prose_budget.py: test_default_roots_include_tests replaces the
  "not yet on" pin; every synthetic tree now contains every default root,
  because a missing root raises by design; the follow-roots tests patch
  _ROOTS to src-only for their "before" half.
- prose_budget.py module docstring describes the two per-file rules; it
  still described the removed single-number ratchet.
- .claude/notes/README.md § "The prose budget is not a lint rule" (renamed;
  no pointer named the old heading) describes the two rules, the pointer
  checks and both roots. Makefile help text and the CLAUDE.md Design
  Principles bullet name src/ and tests/.

Verified: gate exits 0 with tests in scope (0 essays, no comment-budget
failures); a temporary 200-word test docstring makes it exit 1; make verify
passes (5744 passed, coverage 92.60%).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…typos

Two ways the prose budget tool could pass without checking what it claims:
- assert_code_unchanged compared directive comments one way only, so adding a
  `# noqa` or `# pragma` to a "prose-only" commit passed the proof. It now
  reports added directives as well as dropped ones, as its docstring says.
- main() ran the ordinary gate for any argv, so a misspelled
  `--assert-code-unchanged` exited 0 without proving anything. Unknown
  arguments are now a usage error (exit 2).

Both caught by the final review of the tests prose slimming run; tests added
first and seen failing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both are deferred with the reason: the Args-then-narrative placement case
needs an indentation-aware walk plus false-positive triage, and restricting
pointer targets to .claude/notes/ is a design decision, not a mechanical
guard. The two cheap gaps from the same review were closed in e71a16f0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assert_code_unchanged listed files with `git diff --name-only <ref>`, which
never names an untracked file, and the proof runs on uncommitted edits. A new
.py file holding code therefore passed a "prose only" check. The listing now
adds `git ls-files --others --exclude-standard` over the same roots; a new file
has no "before", so any statement in it is reported.

test_prose_budget.py also drops ratchet-era wording ("drifting baseline",
"replaced the hand-maintained baseline", "promoted after three phases") from
three docstrings.

Caught by the final review of the tests prose slimming run; test added first
and seen failing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Final review (two models plus an Opus pass over the whole 946ca96 range):
- timing.md § The golden-stream timing sensor said two harnesses take the
  duration from a monotonic clock and the bounds from the wall clock. Every
  clocked harness takes both from one TurnClock; the paragraph now gives the
  real reason the bounds check is not redundant.
- Six TestCExxx class docstrings in test_custom_lint.py (CE026, CE029, CE035,
  CE036, CE044, CE056) still told the defect story that lint-rules.md holds;
  they now keep invariant and wiring and point at the section.
- One home per fact: the Pi one-sided-sensor argument stays in timing.md
  (removed from lint-rules.md § CE061 and agents.md); the OpenCode/Pi
  overstatement examples stay in agents.md (removed from § CE063).
- lint-rules.md § CE064 gave the antigravity tail as 0.035 ms and as
  0.007-0.03 ms; it is one range, 0.007-0.035 ms.
- test_harbor_export_golden.py cited the packager's module docstring for the
  placeholder-agent design, which it never held; it points at reporting.md.
- reporting.md said CLAUDE.md lists "five" agent lifecycle requirements; it
  lists six, so the count is dropped.

No executable statement changed — proved by assert_code_unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`test_every_exempt_pair_still_exists` (added on main, #177) joined each
_TYPER_COMMANDS entry onto src/coder_eval. 1/8 made those keys repo-relative
because the gate now reports repo-relative paths for two roots, so the join
produced src/coder_eval/src/coder_eval/... and the test failed after the
rebase. It resolves against the repo root instead; what it guards is
unchanged.

A silent conflict: both sides merged textually, and only the test run showed it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The prose budget now covers `tests/`, and `396c22cc` landed three new modules
plus two reworded ones whose docstrings are over the 150-word bar. Their
narrative moves to `.claude/notes/lint-rules.md` behind a `Rationale:` pointer,
the shape the rest of this branch uses.

New notes sections: CE004, CE065, CE066, `_layers`,
`TestRuffExternalCoversEveryRule`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…xists

`src/coder_eval/proxy/` is gone — CHANGELOG 2487 records deleting the
`coder_eval.proxy.pricing` shim. The rule that banned importing it stayed, so
it cannot fire, and its docstring still described the shim as present.

Removed from all five places a rule lives: the module, its `runner.py` import
and `ALL_RULES` entry, its test class, and its id in `[tool.ruff.lint] external`.
The id stays retired, as `runner.py` already requires for CE062.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@uipreliga
uipreliga force-pushed the docs/slim-tests-prose branch from eec2c4e to 1ad75ef Compare September 16, 2026 04:48
uipreliga and others added 2 commits September 15, 2026 22:05
`MAX(20, 0.15 x lines)` on a file's total own-line comments was inverted. It
blocked `isolation/docker_runner.py` at 229/229 for carrying 62 short
annotations pinned to the lines they explain, while a 16-line essay in
`tests/test_regrade.py` sat at 29% of its budget. Four files had settled at
exactly 100% of the cap: the budget had stopped being a ceiling and become a
target.

The failure mode the rule exists to stop is a PARAGRAPH in the code, which is a
long run, not a high total. So the bar is now the run: no own-line comment run
over 8 lines, reading through one blank line so splitting a paragraph does not
duck it, and through none at two, which is the separation PEP 8 already puts
between a banner and its section. There is no per-file allowance, so a file may
carry any number of one-line notes and none of them may grow into an essay.

Measured before choosing 8: 78.5% of runs in the tree are 1-2 lines, 2.5% are
6 or more, and the longest was 16. The cap is below the current maximum, as a
cap rule must be.

24 runs in 25 files are moved to `.claude/notes/` behind a `Rationale:` pointer
or shortened. New notes sections: reporting § Ungraded rows in a rollup,
permissions § Why the chmod tests are Linux-only, lint-rules § The CE id space,
§ CE058 field families, § Keeping DELIBERATELY_UNMIRRORED honest.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e run cap

The run cap governs the SHAPE of any one comment and has no allowance, so it
cannot see a file that passes with every block short and is still mostly
commentary. `MAX(20, 0.15 x lines)` on the file total is restored verbatim to
catch that, and `check_comment_density` now says so in its own docstring rather
than standing alone.

Three rules now, in order of what they do: the run cap is primary, the file
total is the backstop, the 150-word essay bar governs docstrings.

`test_it_catches_what_the_run_cap_cannot` pins the seam — thirty three-line
blocks pass the run cap and fail the total — so neither rule can be deleted as
redundant without that test going red.

Four files sit at exactly 100% of their total budget (`docker_runner.py`
229/229, `criteria/agent_judge.py` 65/65, `evaluation/sub_agent.py` 34/34,
`config.py` 33/33). A comment added to one of them has to be paid for by
deleting another; that is recorded in `.claude/notes/README.md`, with the
measurements behind the run cap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@uipreliga uipreliga changed the title docs(tests): apply the src/ prose rules to tests/, and gate them feat(lint): gate tests/ on the prose rules, and cap the comment run as well as the file total Sep 16, 2026

@bai-uipath bai-uipath left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve — execution is clean: the three holes in the prose-only proof were real and are fixed test-first, the CE023 deletion is complete in all five places a rule lives, and I verified the prose-only claim independently against main (the six src/ files are AST-identical). The notes below are about the disposal mechanism, not the gate.

  • The bin isn't free once CI pins it. Nearly 500 Rationale: pointers now fail make docs-budget when a target file or heading goes missing, so .claude/notes/ can no longer be emptied, re-headed or pruned without going red. That's the opposite of what a scratch directory is for. Fix: pick one — drop pointer resolution from the gate and let the notes be an actual bin, or delete the narrative outright instead of relocating it. Moving ~12,400 words into a directory CI won't let you clean out is the worst of both.
  • lint-rules.md is a second catalogue with a tie-break clause. It reverses the "no prose summary is kept here" rule written one merge earlier, and "the rule file wins on disagreement" concedes they'll diverge. The one-sentence defect summary the slimmed rule docstrings already carry is the part anyone editing a rule actually needs. Not a blocker, but decide this one separately from the rest of the diff.
  • The run cap is already saturated. 21 runs sit at exactly 8 lines and none above, so the "a budget with an allowance becomes a target" argument used to replace the file total applies to its replacement on day one. Each of those is one line from a gate failure on an unrelated edit.

Minor, all optional: two blank lines still split a run at module scope, so the paragraph split the docstring says can't duck the cap partly can (ruff format closes it inside functions only); one new pointer in the harbor packager resolves to a notes section that doesn't discuss the thing it cites; a renamed root raises a traceback rather than a gate failure line.

Worth splitting the CE023 deletion and the run-cap rewrite out of prose PRs in future — both are small, independently reviewable, and get lost in 139 files.

uipreliga and others added 2 commits September 16, 2026 13:00
…overlap

- Shorten the 21 own-line comment runs that sat at exactly 8 lines to 7 or
  fewer, so no run starts at the cap.
- Remove from lint-rules.md every invariant, scope and blind-spot sentence
  its rule file already states, and delete the README tie-break clause.
- Drop a harbor packager pointer whose target section does not discuss
  pre_run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e blank-line blind spot

A renamed root now prints one line and exits 1 instead of a traceback. The
own_comment_runs docstring and CLAUDE.md now state that two blank lines at
module scope split a run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@uipreliga

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. I checked each point against the tree. Fixes are in ed1c01c9 (prose only) and d20d40e6 (gate).

1. Pointer resolution makes .claude/notes/ impossible to prune — not changed.
.claude/notes/ is not a scratch directory. CLAUDE.md makes it the home for design rationale, and the code points into it. The resolution check stops dangling citations, which were a real defect in this PR (479f1a3f had to repair them). When you rename a heading, the gate names each pointer that breaks, and one sed fixes them. I agree with the other half: a paragraph that only records history should be deleted, not moved. That is a per-paragraph decision, not a reason to drop the check.

2. lint-rules.md as a second catalogue — fixed.
The file holds the defect stories moved out of the rule docstrings, not a summary of them. But you were right that the content split was not clean: many sections also restated the scope, exemptions and blind spots. I removed each such sentence where the rule file already states it (about 2,000 words over most sections). Where the rule file does not state it, I kept it, so nothing is lost. I deleted the tie-break clause from README.md. No heading changed, and every Rationale: pointer still resolves.

3. The run cap is already full — fixed.
Confirmed: 21 runs sat at exactly 8 lines. I cut all 21 to 7 lines or fewer, and the cap stays at 8. The longest run is now 7 lines. The notes record the change. The file total still has four files at 100% (config.py, criteria/agent_judge.py, evaluation/sub_agent.py, isolation/docker_runner.py). The notes already say this, and I did not change it here.

Minor notes — all confirmed and fixed:

  • Two blank lines at module scope do split a run: 4 lines, two blanks, then 5 lines measure as [(1,4),(7,5)]. It is now a stated blind spot in the own_comment_runs docstring, and CLAUDE.md no longer claims more than the gate does.
  • The pre_run pointer in harbor/packager.py pointed to § Harbor export, which does not mention pre_run. No section covers it, so I removed the pointer. The comment explains itself.
  • A renamed root now prints one line and exits 1, with no traceback. There is a new test for it.

Splitting PRs: agreed for the future.

make verify passes, and --assert-code-unchanged reports code changes only in tests/lint/prose_budget.py and tests/test_prose_budget.py.

🤖 Generated with Claude Code

@uipreliga
uipreliga merged commit a249877 into main Sep 16, 2026
15 checks passed
@uipreliga
uipreliga deleted the docs/slim-tests-prose branch September 16, 2026 20:22
uipreliga added a commit that referenced this pull request Sep 16, 2026
Rebased onto a249877, which turned the prose gate on for tests/ (#180) and
hoisted function-local imports out of src/ (#176). Brings this branch's own
additions in line:

- Three test docstrings stated history ("Previously ...", "used to hide
  behind", "Accepting it recorded"); they now state the contract.
- `report_command` and `evaluate_command` import `run_summary_rebuild` at module
  level. `evaluate_command` keeps the module import because its tests patch
  functions on that module.
- `tests/test_container_context.py` (new in this branch) imports at module level.
- Reflowed one note paragraph to the notes' 88-column width and rejoined a
  split docstring line in `regrade._should_grade_in_container`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
uipreliga added a commit that referenced this pull request Sep 16, 2026
…into report --rebuild (#178)

* feat(isolation): 1/6 — parse context.json as a ContainerContext contract

Replace the json.loads + dict.get + isinstance ladder at the host→container
boundary with a Pydantic model: every field required, extra="forbid",
StrictBool for grade/regrade, StrictInt for replicate_index. The host builds
the model in _stage_inputs and keeps it on DockerRunner._staged_context.

BREAKING CHANGE: a container image and host that disagree about the
context.json keys now fail at parse time (exit 2, naming the field) instead
of falling back to defaults (grade=True, host_task_file=None, the staged
source_yaml).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(isolation): 2/6 — refuse image skew and assert the container echoed its contract

The image version preflight now refuses instead of warning, for dockerfile_path
images too, and is the single reader of the org.coder-eval.version label. The
container echoes the ContainerContext it parsed into
environment_info["container_contract"], written after the regrade seed so the
prior row cannot overwrite it. One host guard, _assert_contract_echoed,
replaces _assert_grade_honored, _assert_regrade_honored and
_warn_on_version_mismatch, and quarantines a refused record to
task.json.unhonored. A host grade drops a prior row's stale echo, and the echo
is kept out of the rendered Environment table.

BREAKING CHANGE: a driver: docker run is refused before the container starts
when the image's org.coder-eval.version label is missing or differs from the
host's installed coder-eval. Set ALLOW_IMAGE_SKEW=1 (not an empty string) to
run a deliberately different image; a missing label is never excused. An image
that predates the host→container contract is refused after the run, and its
record is moved to task.json.unhonored instead of task.json.graded /
task.json.rerun.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(isolation): 3/6 — move the docker→tempdir driver rewrite host-side

DockerRunner._stage_inputs now stages task.yaml as the execution copy with
driver: tempdir and sends the authored sandbox block in the contract as
ContainerContext.authored_sandbox. The in-container entry point no longer
rewrites the driver; it records authored_task from ctx.authored_sandbox, so a
docker run's task.json still says driver: docker. The CE051 exemption moves
with the rewrite: the two sites are now host staging and opt-in host grading.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(cli): 4/6 — replace `aggregate` with `report --rebuild`

Extract the run-summary rebuild into a printer-free
orchestration/run_summary_rebuild.py (rebuild_run_summary, find_run_root) and
expose it as `coder-eval report <run_dir> --rebuild`, which prints the same
counts line and note. --rebuild refuses --format, --output, a task directory
and a directory inside another run. The rebuilt run id now comes from the
resolved directory, so `runs/latest --rebuild` records the real run id.

BREAKING CHANGE: `coder-eval aggregate` is removed; use
`coder-eval report <run_dir> --rebuild`. Its `-o/--output <dir>` option has no
replacement: --rebuild writes in place, so gather task directories into the
target directory first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(evaluate): 5/6 — refresh the run-level run.json after a detached grade

`coder-eval evaluate <task_run_dir>` now rebuilds the owning run's run.json
and run.md itself after writing the verdict back, instead of telling the user
to run a second command. The refresh is best-effort: any failure only warns,
and the exit code is always the verdict's.

find_run_root accepts only a run.json that is a coder-eval run summary (a JSON
object with run_id and task_results), so a row copied into another tool's tree
never overwrites that tool's run.json. rebuild_run_summary refuses to write
through a symlinked run.json or run.md; both evaluate and report --rebuild
inherit that, since it is their one shared write path. The refresh is skipped
when the grading --run-dir sits inside the owning run, where its own task.json
would count as a second row. write_run_summary now writes atomically, and the
new console lines escape Rich markup in paths.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test: guard the contract echo over a maximal sandbox and the Typer-command exemption list

Two review findings from the container-contract work, closed as tests:
- the host compares the container's echo with its own dump, so a validator
  under authored_sandbox that is not idempotent across a JSON round trip would
  refuse every docker run; the round trip now runs over a maximal SandboxConfig;
- prose_budget's _TYPER_COMMANDS kept a dead entry for the deleted aggregate
  command; it must now equal the commands the CLI registers.

Two more candidates that need type information or are conventions are recorded
in .claude/harness-candidates.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(cli): 6/6 — pin every shared run/execute flag as declared identically

Phase 6 planned to declare the flags `run` and `execute` share once, through
option factories. The factory module was built and produced byte-identical
--help output, but review found it saved no lines and turned each new shared
flag into a five-place edit, so it was dropped and cli/ is unchanged.

What remains is the drift guard: a test that compares every click attribute a
shared flag can show or validate between the two commands, and asserts that
--resume and --format stay command-specific.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: code review fixes for container-contract-and-command-surface

- Strip the prior row's container_contract from the staged prior.json. An image
  that honors regrade but predates the echo keeps the prior environment_info,
  so a matching echo from an earlier identical dispatch passed as its own.
- A result refused by the contract echo now leaves a synthetic ERROR task.json
  in place of the quarantined record, like every other container failure, so
  the row no longer vanishes from the next run.json rebuild.
- On a detached grade, the refused task.json.unhonored is folded back from the
  scratch directory beside the graded row, and the error no longer suggests
  --allow-host-grading when the image itself was refused.
- Docstrings, the stale "version checks only warn" wording, the dead None branch
  in _quarantine_record, the run.json environment note, and a cwd-independent
  task path for the evaluate refresh tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: apply main's prose rules to the container-contract branch

Rebased onto a249877, which turned the prose gate on for tests/ (#180) and
hoisted function-local imports out of src/ (#176). Brings this branch's own
additions in line:

- Three test docstrings stated history ("Previously ...", "used to hide
  behind", "Accepting it recorded"); they now state the contract.
- `report_command` and `evaluate_command` import `run_summary_rebuild` at module
  level. `evaluate_command` keeps the module import because its tests patch
  functions on that module.
- `tests/test_container_context.py` (new in this branch) imports at module level.
- Reflowed one note paragraph to the notes' 88-column width and rejoined a
  split docstring line in `regrade._should_grade_in_container`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants