docs: move design rationale out of src/ into .claude/notes, gate it, and slim CLAUDE.md - #177
Merged
Merged
Conversation
uipreliga
requested review from
CarlesUIPath,
akshaylive,
bai-uipath and
tmatup
as code owners
September 15, 2026 05:46
|
Claude finished @uipreliga's task in 1m 18s —— View job
|
bai-uipath
approved these changes
Sep 15, 2026
bai-uipath
left a comment
bai-uipath
left a comment
Collaborator
There was a problem hiding this comment.
Fix what you agree with, otherwise lgtm. The prose-only invariant holds: docstring-stripped ASTs are identical across all 99 changed Python files, directive comments are unchanged, Typer help text is byte-identical, and the judge's verdict schema is untouched. The pointer convention is real rather than decorative, with 363 pointers in src resolving to 83% of the note headings and none dangling.
Worth stating before the cuts: src prose drops from 77,954 words to 17,752, but the notes go from 9,356 to 41,391, so the new home is 4.4x the old one. The cuts below take roughly a quarter off that.
Cuts
- The CE lint-rule catalogue in the notes README. Around 2,165 words duplicating each rule's own module docstring, disclaimed as non-authoritative in the two sentences directly above it, and targeted by no pointer. Fix: delete the catalogue, keep the sentences that name where the authority lives.
- The per-file abstract lines. Seven single-line summaries, the largest 19,000 characters, restating the sections immediately beneath them: about 6,900 words, 17% of the notes. Fix: delete them, since the contents list and the headings already navigate.
- "Not wired up yet" in the permissions note. 769 words, nearly half that file, designing code that does not exist. Fix: open an issue and delete the section.
- The CLAUDE.md commit. It asserts that an adapter rejects an unsupported run-limit field at load time; the opencode and pi adapters both log a warning and continue, and the parity doc still records those divergences as table rows. Fix: revert that sentence. The communication-style and delete-before-you-guard edits in the same commit aren't about prose relocation either.
Worth fixing
- The gate penalizes usage examples. Args/Returns/Raises blocks are excluded from the word count but Example: blocks are not, so a call example counts against the bar. Two were deleted here, including the keyword contract for the retry helper. Fix: add Example:/Examples: to the excluded sections and restore them.
Minor: the Typer exemption set has no stale-membership test, unlike the other allowlists in this repo; and the abstract-method exemption is carrying one 315-word docstring worth trimming on its own merits.
Collaborator
Author
|
Thanks — three of the four cuts and the gate fix were right. One cut I could not verify, and I explain below why I did not apply it. Changes are staged locally and land in the next push. AppliedThe gate penalizes usage examples. Correct, and the inconsistency was internal: The CE lint-rule catalogue. Deleted. Nothing pointed at it, and the two sentences above it already disclaimed it. "Not wired up yet". Deleted from The CLAUDE.md parity sentence. Reverted. Confirmed: Minor. Added the stale-membership test for Not applied: the per-file abstract linesThe premise does not hold. I checked the isolation abstract's 191 code identifiers against the rest of its own file, every other note, all of
The smaller abstracts behave the same way. The The readability complaint is fair, so I fixed that part without deleting anything:
If the abstracts should still go, the safe order is to re-home those 61 claims into the Not applied: the rest of the CLAUDE.md commitThe communication-style move and
|
Adds `tests/lint/prose_budget.py`: a stdlib-only measurement of the essay-shaped prose in `src/coder_eval` — docstrings over 150 words (Typer commands exempt) plus comment runs of 3+ lines — gated against a module baseline of 79,754 words. It also resolves every `Rationale: <path> § <heading>` pointer, and under `--assert-code-unchanged <ref>` proves a commit moved prose only, by comparing the docstring-stripped AST and the multiset of functional directive comments. The gate is wired in both seams: `make docs-budget` / `make verify`, and the `quality-gate` job in pr-checks.yml — CI restates every step and never invokes `make`, so the Makefile line alone would gate nothing on a PR. Relocates the 12 sections of `.claude/architecture-notes.md` verbatim into `.claude/notes/<subsystem>.md` and deletes it, so rationale has exactly one home and the tree has exactly one index. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JcdMjPKFc2wdg4J6Ezg4E2
Cuts the four densest files in the tree from 6,874 essay words to 533,
leaving the caller-facing contract and the couplings a future editor
would break, and moving the design rationale to
`.claude/notes/{timing,permissions,persistence}.md`.
Kept in the source: the `timing.ts` parity claim and its shared fixture,
`TurnClock`'s not-for-deadlines hazard, the raw-total ordering hazard and
its division guard, the stacked-window contract, and every CE reference
whose text stayed. Deleted: history git already holds, and `TurnClock`'s
per-harness roster, which `docs/agents/HARNESS_PARITY.md` owns.
Also corrects a stale relocated claim: the notes said the task directory
was not shielded, but `Orchestrator._communicate_with_retry` chmods it
alongside the reference.
No executable statement changed — proved per file by
`prose_budget --assert-code-unchanged`, which compares the
docstring-stripped AST and the functional directive comments.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JcdMjPKFc2wdg4J6Ezg4E2
Cuts the five harness adapters and their eight helpers from 17,227 essay words to 4,456, and gives `.claude/notes/agents.md` the shared story the adapters kept telling five times over: the turn lifecycle, first-window seeding, per-harness generation marks, token accounting and cost resolution, why a clean exit can still be a crash, and how each CLI is reaped. Every per-harness parity claim either stays in the adapter or is already a row in docs/agents/HARNESS_PARITY.md, which the taxonomy makes the SSOT for cross-harness facts. Both SDK#24168 FIXMEs stay FIXMEs. No executable statement changed — proved per file by `prose_budget --assert-code-unchanged`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JcdMjPKFc2wdg4J6Ezg4E2
Cuts orchestrator.py and orchestration/ from 17,724 essay words to 3,980, extending the four sections Phase 1 relocated into orchestration.md rather than opening rival headings for the same topics: the terminal-status chain and the four grading sites under execute-vs-run, the fired-only gate and the ceiling/floor bounds under early stop. New sections cover what the orchestrator alone owns — recording the task as authored, the three separately-resolved routes, interrupt-proof teardown, restoring a PATH from an untrusted run directory, the dialog loop, embedded commands and experiment resolution. regrade's container rationale went to isolation.md, where detached grading already lives. `prose_budget` now resolves a pointer against `##` or `###`, so appending to an existing section does not force the pointer up to its parent. No executable statement changed — proved by `prose_budget --assert-code-unchanged`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
13,289 words of essay-shaped prose across 11 files down to 2,960, with no code change (AST + directive multiset verified against the phase-start SHA). docker_runner.py 4,993 -> ~1,100 and sandbox.py 3,259 -> ~590 are the bulk. The repo's largest comment block, 34 lines on --cap-drop, is now the four COUNTERPART lines that state the coupling plus a pointer; the capability argument it carried is rationale and moved. isolation.md gains Capability drops and the anti-cheat window, What crosses into the container, Trusting what the container sends back, and The sandbox the criteria run in. Detached grading from the CLI joins the existing detached-grading section rather than rivalling it, and the in-container driver rewrite stays in orchestration.md, which already owned it. The Phase-1 relocated bullet covered nine of these topics already, so the single-home rule needed excision as well as writing: seven spans moved out of it, each left as a cross-reference to the section that now owns them. The --cap-drop/FOWNER known gap stays in docs/DOCKER_ISOLATION.md; the notes link it and say so. All eight --help outputs are byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…/notes 14,406 words across 34 files down to about 3,300, with no code change and no generated surface touched. Two constraints made this phase different from the others, and both were verified by parsing rather than by trusting a downstream check: the first non-blank line of every criterion class docstring, which CE033 copies into the plugin reference, is byte-identical; and no Field(description=...) string under models/ changed, which is what CE030 ripples into the task guide. The LiveVerdict determinism and monotonicity properties stay at the definition site in criteria/base.py, because they are the contract an author has to satisfy. What moved is the derivation: why CE036 replays trajectories, and what CE025 cannot see. contracts.md gains the live_verdict contract, the checker base class, route resolution, judge context and untrusted text, sub-agent judging, and recording a CLI invocation. persistence.md gains judge persistence; agents.md the sdk_options pass-through; orchestration.md the rates, the early-stop guardrail placement, and the armed gate. Docstrings that restated a Field description, or the task guide's own YAML examples, were deleted rather than moved — the guide is the source of truth. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…de/notes 10,234 words across 30 files down to about 2,900, finishing the plan. The tree goes from 79,754 essay-shaped words to 18,163, and from 126 docstrings over 150 words to one. That one is agent.py::communicate, the plugin SPI contract every third-party agent author reads: the Args/Returns/Raises, the rule that a mid-turn failure sets pending_turn before raising and the caller rolls the counter back, and the streaming event protocol. It is the exemption the plan predicted, and the only one taken, so the 150-word bar held for the whole tree. reporting.md gains the Agent ABC contract, telemetry emission, cost joining, report rollups and the HTML twin, Harbor export, and the ATIF trajectory bridge. Two hazards the plan named had to be ADDED rather than kept: neither the pricing.py/pricing.ts mirror nor the "evalboard's static twin" parity claim was ever stated in its own source file — both lived only in CLAUDE.md. Editing one side of either pair without the other is exactly what they exist to prevent, so they now say so where the editor will be. errors/categories.py is untouched: its per-member retryability notes are the contract errors/categorization.py dispatches on, and they are one-line comments the plan puts out of scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
check_pointers proves a pointer RESOLVES. It cannot see the defect that actually shipped: a block replacement anchored on the wrong line leaves the tail of the replaced prose stranded after the pointer, where the $-anchored regex stops looking. The file parses, the pointer resolves, and the comment carries a severed half-sentence. Three phases of the prose-reduction work shipped that shape to review before this check existed; a fourth ran clean because the check was running from the first edit. It also reports an orphaned docstring terminator, which is worse than it looks: the file stops parsing, and the measurement silently reports it as zero words rather than failing. A docstring may still follow its pointer with Args:/Returns:/Raises: — that is the house shape, and the one case this must not flag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Self-contained (inline CSS/JS, no external fonts or images)" is caller-facing — it is why the file can be opened offline and uploaded as a CI artifact — and the 7/7 rewrite dropped it while adding the static-twin claim. Both belong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fifth was promoted (b8b1dcf). These four are deferred with the reason, so the next person does not rediscover them: the general severed-fragment shape needs an allowlist to be usable, the single-home rule needs a similarity measure, a pointer landing unhelpfully is probably not mechanizable at all, and the generated-surface guard needs a commit-scoped diff the lint harness cannot see. The single-home one is the highest-value unbuilt guard in the notes design — it bit every phase of this run, once against a file the phase never opened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An audit of the finished refactor found nine places where compression either dropped a contract or, worse, replaced it with a plausible wrong reason. The bad one is `_resolve_backend_route`. Its docstring said the function raises "so it must survive -O" — true of a different function, transplanted here — while the actual contract was gone from the source AND from the notes: an explicit backend override that cannot be honored must fail loudly, not degrade to a backend the task author never asked for. A reader told the raise is only about -O can reasonably "improve" it into a fallback, which is exactly the regression the deleted sentence forbade. The rest: - write_text_atomic claimed mode 0o644 as if guaranteed. It is a ceiling; the umask narrows it, and 0o077 yields 0600 — which breaks the docker driver, the failure the directive exists to prevent. - A NOTE in sandbox.py said three methods do not validate path traversal. Two of them go through resolve_files (and so through _within_sandbox and _reject_escaped); only list_files still does not. Someone reasoning about criterion containment from that comment reached the wrong conclusion. - enforces_permission_windows kept the clause making the window SAFE and lost the one making it REAL: without the container's DAC cap drops a mode-000 directory is still readable by root. - capture_to's copytree flags lost the reason one of them is load-bearing. - adopt no longer said criteria can still write to the adopted tree, under a docstring headlined "materializing nothing". - The docker log constants lost why there are two of them, and had no pointer. - armed_criteria_passed said "weighted score", which names a different quantity in the same class; the binarisation that makes gate_threshold=1.0 an exact equivalence was gone. - judge_context said "these tokens" with no list after it. - _build_run_command_env counted eight env facets and sets nine. The baseline rises 18,163 -> 18,549. That is the ratchet working as intended: restoring real contract is a legitimate reason to raise it, and the reason belongs here rather than in a silent constant bump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review B found one shape repeated six times: compression kept a categorical claim and dropped the words that made it true. Each is the only fact its comment now carries, sitting above the code someone would edit. - claude_code_agent: the task-notification guard is checked SECOND, not first. What matters is that it precedes _is_sdk_result_message. - opencode_agent: "the condition is the TELEMETRY, not the event vocabulary" — but one of the two arms IS the vocabulary, and a later comment says so. The dropped word was "alone". Deleting the arm as redundant re-opens scoring SUCCESS 1.0 on zero turns. - batch: the predicate is NOT_GRADED **or** executed. "Executed is a required half" reads as `and`, which would leave zero-iteration execute rows ungraded forever. - reference_comparison: "every failure below raises" — three branches below return a gating 0.0 on purpose, one of them carrying a CE039 noqa. - docker_runner: the stdout limit does not mirror _POST_RUN_STREAM_LIMIT; it is 256x larger, deliberately. Unifying them downward reinstates the mid-stream teardown that lost whole paid tasks. - cli_called: "harness fault, not agent behaviour" contradicted the block 50 lines down explaining that all five paths are agent-reachable. Also restores the judge ignore floor's MECHANISM: it is a copy-time control, the same list passed as copytree's `ignore`, not only an SDK setting. Without that, dropping .claude / .mcp.json from it looks redundant and lets an agent-planted settings file into the judge's own working directory. _resolve_backend_route's contract now has a home in the notes, and the docstring points at it rather than at the LiteLLM section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--tmpfs` is never emitted. `_build_argv` does not construct one, `test_reference_inside_task_dir_needs_no_tmpfs_mask` asserts `not _tmpfs(argv)`, and docker_runner.py's own `_reference_mount_args` docstring opens "No tmpfs mask any more". Three of its neighbours said the opposite, two of them lines this refactor wrote. The claim predates the branch, so this is not a regression — but the refactor read every one of those lines and compressed them instead of questioning them, and it authored two new canonical homes for the falsehood. Re-ratifying it is worse than never having looked. What is actually true: the task dir is a shielded read-write COPY, so a reference embedded in it is covered by the same mode-000 window rather than hidden under a layered filesystem. `orchestration/evaluation.py`'s hard-fail was justified by the mask and by an EROFS on a `:ro` bind — both false now, and the real reason (the fallback resolves to a tree the window was not opened over) is the one that survives. Also restructures `permissions.md`'s Phase-1 blob. Its single 4,450-char line duplicated two of its own sibling sections, restated the FOWNER/CHOWN rationale that isolation.md explicitly delegates to docs/ saying "they are not restated here", and carried the tmpfs claim. All four inbound source pointers still resolve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_ESSAY_BASELINE_WORDS was the weakest part of the gate. It was a single tree-wide number, hand-edited eleven times in this work alone, that a reviewer had to take on trust; it let growth in one file hide behind shrinkage in another; and it said nothing at all about a file that did not exist yet. It is gone. Two rules replace it, and neither has a number anyone maintains: own-line comments per file <= MAX(20, 0.15 * file length) no docstring over 150 words of PROSE The comment budget is proportional, so deleting code takes its budget with it and a new file is governed from its first commit. Own-line only: a trailing `# noqa` is a directive and a per-member annotation on an enum is the contract a dispatcher reads — counting either would push against documenting them. The floor is what protects a constants module at one comment per constant, which is where the tree's natural maximum sits. The essay rule now counts prose, not structure: an Args/Returns/Raises block is interface documentation, and counting it pushed exactly the docstrings that document their contract best over the line. Exemptions are categorical rather than numeric — an @AbstractMethod docstring IS the contract implementers read, so the plugin SPI is covered by kind, and a new abstract method is covered automatically. The old "at most two, by fiat" allowance is unnecessary: the tree now has ZERO essays. Getting there took 44 comment lines out of 7 files. Most came out by reflowing two lines into one or cutting a section divider; the one real compression was a 22-line block in early_stop.py restating the floor bound that orchestration.md already owns, and the distractor-exclusion rule it uniquely held moved there rather than being dropped. One reflow silently merged `# pyright: reportImportCycles=false` into the prose line above it, which would have stopped pyright honouring it. --assert-code-unchanged caught it. That is the second time the directive multiset has earned its place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd clarifications Co-Authored-By: Claude <noreply@anthropic.com>
`_TRAILING_SECTIONS` already accepted an `Example:` block after a pointer, but `_DOCSTRING_SECTIONS` counted one against the 150-word bar — the two lists disagreed about the same shape, so a docstring in the house style could fail the gate. Two call examples had been deleted to get under it, one of them carrying the `on_attempt_error` keyword contract for the retry helper. A call example is code, not narrative, so it joins the structure list. Both examples are restored, with `execute_with_retry`'s Args/Returns/Raises block. Also from review: - `_TYPER_COMMANDS` gains a staleness test. An allowlist entry whose function has moved exempts nothing while still reading as a deliberate exemption — the vacuous-guarantee shape CE057's membership test already guards against. - `Agent.communicate` trimmed from 315 to 273 words. Every obligation kept; the turn-lifecycle rationale becomes a pointer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three review cuts, and a readability fix for a fourth. - The CE lint-rule catalogue in README.md: 2,165 words restating each rule's own module docstring, disclaimed as non-authoritative by the two sentences directly above it, and targeted by no pointer. Deleted; the sentences naming where the authority lives stay. README.md drops 2,745 -> 589 words. - "Not wired up yet" in permissions.md: 769 words designing code that does not exist. Deleted — git holds it. - The wall bullets are reflowed to the notes' 88-column prose width. The largest was a single 19,453-character line, which no diff could show usefully. - reporting.md's pseudo-directory-tree, two entries carrying 2,174- and 506-character trailing comments, becomes `###` subsections of ordinary prose. The review also asked for the per-file abstracts to be deleted as restatements of the sections beneath them. Not done: 61 of the isolation abstract's 191 code identifiers appear nowhere else in notes/, docs/ or CLAUDE.md — among them `--allow-recorded-commands`, `task_config.resolved`, `_seed_from_prior_result` with its `early_stop` carry, and `FinalStatus.is_execution_fact`. Deleting them loses rationale rather than a duplicate. Re-homing those claims first is a follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md is loaded into every session, so a copy kept there costs context on every turn and goes stale with nothing to sense it. Three sections were copies. - **Directory Structure** was wrong. It listed `optimize/`, which exists on neither this branch nor main, and omitted `errors/` and `plugins.py`, both present since the initial public release. The tree was never the value — the annotations were. It becomes a list of what a filename cannot tell you (the CE048 twins, the CE063 seam, the CE057 sidecar, CE053's filename ownership, CE056's env var, the pricing.ts mirror), opening with "run `ls`". 454 -> 176w. - **Success Criteria** was a third copy of one registry, behind the CE033-generated plugin reference and the Task Definition Guide, with no parity test — the shape CE030's own CLAUDE.md test exists for. Accurate today; stale eventually. Now the shared-field paragraph plus three pointers. 259 -> 76w. - **Extension Points** restated docs/EXTENDING.md's checklists step for step while the section 190 lines above it says "Each entry is a pointer". Keeps the three orientation facts — pkgutil discovery, the plugin SPI with no closed enum, pricing on the same register hook — and the obligations (CE036, CE047, parity). 48 -> 21 lines. Also reverts the run-limit parity sentence claiming an adapter rejects an unsupported field at load time. It does not: opencode_agent.py:785 and pi_agent.py:738 both warn and continue. Neither lint-pinned surface is touched — the CE030 model sentence and the six skill names are unchanged — and nothing links into the cut sections. CLAUDE.md: 2,802 -> 2,028 words. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bind-mount rewrite (#175) landed on main while this branch was open, so its new prose in `harbor/packager.py` never met the gate this branch introduces. Two docstrings were over the 150-word bar. - The module's emitted-layout diagram carried an eight-line annotation restating what `_write_environment` and `_write_docker_compose_mounts` already say. It is three lines now: Dockerfile only when `dockerfile_path` is set, and everything bind-mounted rather than COPY'd. - `_plugin_volume_specs` keeps its contract and its one real HAZARD -- a plugin path is carried UNEXPANDED and `docker_runner.py` expands it at launch, so the two sides must move together. The rest is a pointer. The rationale moves to `reporting.md § What the export carries, and what it refuses to carry`: why a prebuilt image needs no Dockerfile (Harbor's own `should_use_prebuilt_docker_image`, and `[environment].workdir` reaching `docker exec` independently of the build), why nothing is COPY'd, why a bind mount cannot self-nest into the export's `-o` directory, and the live `harbor run` that surfaced the agentless `initial_prompt` validation error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
uipreliga
force-pushed
the
docs/slim-source-prose
branch
from
September 16, 2026 02:39
af5e07c to
4941681
Compare
uipreliga
changed the title
uipreliga
added a commit
that referenced
this pull request
Sep 16, 2026
Rebase fallout from #174/#177, which replaced CLAUDE.md's fenced directory tree with `ls` plus selective bullets, split `.claude/architecture-notes.md` into `.claude/notes/`, and added the `make docs-budget` prose gate. - Drop CE067. It asserted that CLAUDE.md's tree names every top-level package member; that tree no longer exists, and the bullets that replaced it are deliberately not exhaustive, so the rule's invariant is now false by design. The surviving half — no bullet may name a path that does not exist — is recorded in .claude/harness-candidates.md rather than rebuilt here. - Move the reports-package rationale into `.claude/notes/reporting.md`, and retarget `reports_stats` / `reports_junit` in notes/ to their new homes. - Bring `result_metrics.py` and `run_record.py` under the prose budget by adopting main's trimmed wording for the comments and docstrings they inherited, including its `.claude/notes/` pointers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
uipreliga
added a commit
that referenced
this pull request
Sep 16, 2026
…layer (#176) * feat(pricing): 1/5 — generate the evalboard rate table from pricing.py The frontend's rate table was a hand-copy of `_PRICING`, kept honest by five layers of bookkeeping: a regex parser that re-read pricing.py at test time, a meta-guard against that regex silently narrowing, a DELIBERATELY_UNMIRRORED exemption set, a staleness guard for the exemption set, and a comment asking the next reader to keep the set honest. It still shipped a real bug — claude-sonnet-5, gpt-5.6-sol, gpt-5.6-terra and gpt-5.6-luna sat in the exemption set under "the evalboard never runs them" while appearing ~32k / ~2k / ~17k / ~2k times in the run corpus, so those runs rendered "—" for cost with nothing failing. If a test can read the table, a generator can emit it. `make pricing-mirror` now renders evalboard/lib/pricing.generated.ts from pricing.builtin_rates(), and CE065 re-renders and diffs it. All five bookkeeping layers delete. The one thing the exemption set encoded that was NOT bookkeeping — that three OpenRouter models must stay unpriced on the frontend so runs.ts apportions the provider's real per-call bill instead of a static estimate — becomes data on the rate itself: ModelPricing.per_request_billing. The generator skips those rows, so nothing has to remember them. The field is defaulted and last, so 4-positional construction (including the out-of-tree coder_eval_uipath rate card) is unaffected; it participates in register_pricing's anti-shadow comparison, which is correct. Generating the table deliberately ADDS gpt-5.4-mini, gpt-5.4-nano, gpt-5.4-pro and gpt-5.5-pro to the frontend — the four exemption entries that were pure drift. Also corrects every surface that told a reader to hand-edit pricing.ts or named the deleted parity test: the Makefile and CI-job comments, litellm/README.md's "register in both tables" step, CLAUDE.md, and two stale evalboard consumer comments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SGRvWucHgYgpcGhmyHrdBg * refactor(reports): 2/5 — two DRY fixes and hoist 18 function-local imports SLOW_PARAMS_PREVIEW_CHARS was defined in reports.py and used by reports_html.py, while the module that DEFINES it truncated with the literal 50 twice. The variant Token Usage card re-derived TokenUsage.total_tokens inline — a fourth home for arithmetic the model already owns. Both now have exactly one definition; neither changes a rendered byte at today's values, which is the point. Of the 19 function-local imports across the three report modules, 15 had their module edge at top level already, so hoisting them is adding a name to an existing line. Three add a genuinely new top-level edge (analysis in reports.py and reports_html.py, reports_html in reports_experiment.py — verified acyclic). ONE stays deferred: reports.py's `from .criteria import ...`. Importing coder_eval.criteria runs pkgutil auto-discovery with registry side effects, which hoisting would put on the path of every `import coder_eval.reports`. It now carries a comment saying so, and a subprocess test asserts coder_eval.criteria stays out of sys.modules — the decision is pinned rather than remembered. Also documents the run.json `input_tokens` seam at its writer: the key carries uncached_input_tokens, NOT the derived TokenUsage.input_tokens total, and evalboard/lib/runs.ts depends on that reading. Same word, two quantities — the name is fixed by the run.json contract and cannot change without breaking archived runs. No golden value is re-baselined: the test diff is additions only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SGRvWucHgYgpcGhmyHrdBg * refactor(reports): 3/5 — split reports_stats.py into stats, result_metrics and helpers reports_stats.py had become three unrelated modules sharing a file: a distribution-free numeric core, a set of EvaluationResult metrics the ORCHESTRATOR consumes mid-run, and the report renderers' own helpers. The orchestrator importing a "reports" module for a number it needs during a run is the layering wart; the file was the reason it had to. stats.py pure statistics. Imports NOTHING from coder_eval — asserted by a test, not by convention, because that is the whole reason the numeric core can be reasoned about in isolation. result_metrics.py turn_time_buckets, visible_turn_count, has_final_reply, expected_turns_overage. Consumed by the orchestrator during a run as well as by the reporters, so it is not a report. Deliberately NOT folded into timing.py, which has no EvaluationResult dependency and is imported by every agent. reports_stats.py what is left: report-shaped helpers over variant and experiment results, plus the display formatters (fmt_mean_sd, fmt_p) — presentation, not computation. Its cycle rationale is restated rather than deleted: it is LIVE, not historical. experiment -> html -> helpers, so folding the helpers into the experiment reporter would close a cycle. Also moves eval_result_to_task_dict to run_record.py. It writes one run.json row — a run-record serializer, not a report — and its placement was the only reason orchestration/batch.py reached into the reports layer at all. Moving it is what lets Phase 4's CE066 allowlist be purely writers instead of carrying a serializer as a permanent exception. Every one of the 21 moved definitions is byte-identical to its pre-move form, verified by AST comparison. A characterization test pins the full run.json row against a snapshot captured before the move; the non-finite sensor is retargeted to coder_eval.stats and still guards the same 7 functions. Two test files that held only tests for moved names are merged into the files named for those names, rather than left behind as misnamed orphans. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SGRvWucHgYgpcGhmyHrdBg * refactor(reports): 4/5 — reports/ package, durations.py, and CE066 The five reports_*.py modules become a package (markdown/html/experiment/junit/ helpers) whose __init__ re-exports the public writer surface. Private names stay private: the 12 that tests reach for are imported from their submodule, so the package's API is not a function of its test suite. `format_ms` moves to durations.py. formatting.py imports claude_agent_sdk for the payload formatters, and the reports package should not reach through an SDK-shaped module for a 14-line duration formatter. NOTE this does NOT make the package SDK-free — models/agent_config.py imports ClaudeAgentOptions and every report module needs models. The docstring and tests say what is actually true rather than what the plan hoped. CE066 pins the layering the split establishes: core may import only the package's public WRITERS. An allowlist, not a denylist, so a new report helper is banned from core by default. It checks BOTH the absolute and the relative spelling — the first draft matched only `node.module`, which for `from ..reports import X` holds "reports" with the dots in `node.level`, so it fired on neither of the two real edges in the tree and its tests passed because they used the absolute form. An unrun assertion is documentation, not enforcement. The core-layer predicate moves to a shared _layers.py that CE004 and CE066 both read, so a package added to one cannot escape the other. It is stated as "every module directly under src/coder_eval/ is core": naming only orchestrator.py left result_metrics.py exempt — the module CE066's own fix message tells you to move your metric into. Also fixes a latent packaging break: .gitignore's bare `reports/` (meant for run output) matched src/coder_eval/reports/, leaving __init__.py untracked and building a wheel with zero files under coder_eval/reports/. Anchored to /reports/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: 5/5 — retarget every reports_* reference and record the rationale The tree, the review-command module lists and a handful of prose references still named modules that no longer exist. CLAUDE.md's `reports_html.py` line also called it "the evalboard's static twin" — a parity promise nobody was keeping: it has ~40 private renderers and nothing tests it against the 31k-line evalboard. That line goes with the entry. Adds `## The reports package` to .claude/architecture-notes.md, which is where CLAUDE.md's preamble sends a reader for rationale. It records the CE066 layering, why stats.py / result_metrics.py / run_record.py each sit outside the package, the relative-import trap CE066's first draft fell into, and — the part a future reader would otherwise re-litigate — the DECISION NOT to build a shared section-data layer, with the measurement behind it: only 1 of the 4 "duplicated" section pairs shares an input shape, and the differences in the rest are per-surface presentation, not drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): CE004 never fired on the relative import spelling CE004 matched `node.module` against `^coder_eval\.cli`, but a relative import keeps its dots in `node.level` and leaves the rest in `node.module` — so `from ..cli import run_command` arrives as `level=2, module="cli"` and matched nothing. The relative form is this codebase's dominant idiom, so the rule has been guarding roughly nothing since it was written, with its tests green because they used the absolute spelling. Found while fixing the identical bug in CE066 during the reports split. Two rules independently falling into the same trap is the definition of a shared helper, so the matching moves to `_layers.imports_package` next to the core-layer predicate both rules already share — the same reasoning that put `is_core_path` there. The helper also fixes a narrower bug the regex had: `^coder_eval\.cli` prefix- matched `coder_eval.client`. Matching is now on a package boundary. Adds the regression tests both rules were missing: every spelling of a banned import, plus the prefix-bleed case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: record two deferred harness candidates from the reports consolidation Both are real gaps with a known shape but neither is ~30 minutes of work: a hand-edit guard for generated surfaces needs a checksum gate rather than a diff, and a prose-path resolver has the same tree-parsing problem the plan already measured and declined for CLAUDE.md alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: code review fixes for the reports consolidation Three findings from the cross-phase review, all whole-diff-only: CE004 and CE066 both missed a THIRD import spelling. `imports_package` handled the absolute and relative forms, and `is_bare_package_import` handled `from . import reports` — but `from coder_eval import reports` is level=0 with module="coder_eval", which matched neither. It binds the package, so every attribute read through it was invisible to both rules. That spelling is a real in-tree idiom (`from coder_eval import __version__`), which the tests now pin as the negative case alongside the positives. reports/__init__ published nine names nothing imports. The export list was measured before packaging as "every name imported from a reports module", which at that point included the five modules importing EACH OTHER; those became intra-package `from .markdown import …` and need no re-export. 25 -> 16, and the docstring no longer claims more than it delivers. Stale cross-repo pointers the phase greps missed by stopping at the src/ boundary: two evalboard tests still cite the deleted pricing-parity test as the authority on rates (it is CE065 now), runs.ts and variants.ts still name reports_experiment.py / reports_junit.py, and a workflow comment does too. The runs.ts one matters most — it is the far half of the `input_tokens` seam Phase 2 deliberately documented at its writer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): 1/4 — is_core_path covers the whole core layer `_layers.is_core_path` was a denylist of ten directory names plus a top-level-module regex, so `isolation/` — the `driver: docker` evaluation path — was invisible to BOTH CE004 and CE066. Its unanchored `[/\\]coder_eval[/\\]` also classified a repo-root file as core, because this project's own checkout directory is named `coder_eval`. Replace it with the anchored allowlist its own docstring already described: everything under `src/coder_eval/` is core except the `cli/` and `reports/` packages, so a new subpackage is core by default rather than exempt until someone notices. Uses the established `(?:^|[/\\])src[/\\]coder_eval[/\\]` spelling rather than a new variant — the defect being fixed was a regex that disagreed with its siblings. Core-set delta: +isolation/__init__.py, +isolation/docker_runner.py, +resources/__init__.py; zero removals; zero new CE004/CE066 violations. Four CE004 fixtures in tests/test_lint_runner.py built paths without a `src/` segment; three stop firing under the anchor and the fourth passes vacuously, so all four are re-anchored. `make lint` does not run that file. `TestCoreLayerMembership` pins the non-core set against the real filesystem and pins the residual `~/src/coder_eval` collision as unreachable rather than asserting it away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): 2/4 — declare CE044 and CE065 to ruff, and pin the id space `[tool.ruff.lint] external` is what stops ruff reporting RUF102 for a `# noqa: CE0xx` it does not own. It was two ids short: CE065, and CE044 — which the review missed and only prototyping the sensor found. Both are `@pytest.mark.lint` classes rather than BaseRules, and that is exactly why they slipped: `TestRuffExternalCoversEveryRule` already guarded this list, but derived the known ids from `ALL_RULES` alone, so it was blind to half the rule space it was meant to cover. Extend that class rather than adding a second one beside it: `_known()` now unions `ALL_RULES` with the `class TestCE\d{3}` ids scraped from this file, and `test_no_dead_entry_survives` asserts the other direction, so a declared id for a deleted rule fails too. Both messages name pyproject.toml and the offending ids. `tests/lint/runner.py`'s id-claiming note carried the same hand-maintained enumeration and had fallen behind CE044 identically; it now points at the grep instead of listing the ids. Nothing was red for want of these two entries — no `# noqa: CE044` or `# noqa: CE065` exists in the tree — so the fix is pre-emptive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(reports): 3/4 — cover the HTML slowest-commands truncation branch `reports/html.py:825` — the `params_preview += "..."` arm — was the one uncovered line in the renderer. Its markdown twin got both a truncation and a boundary test when SLOW_PARAMS_PREVIEW_CHARS was introduced; the HTML side got neither. Adds the twin pair. Both read the constant rather than the literal 50, so they survive a change to it, and both were mutation-checked: flipping `>` to `>=` fails the boundary case, deleting the ellipsis arm fails the truncation case. `str(dict)` emits single quotes that `_esc` renders as `'`, so the raw cell is longer than the preview. The assertion unescapes before measuring; the renderer's escaping is untouched. No source change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: 4/4 — retarget prose that names deleted report modules The consolidation's own greps could not reach these: nine present-tense references in tests/ naming `reports.py`, `reports_html.py`, `reports_experiment.py` or `reports_junit.py` — modules that no longer exist — plus CE053's fixture default, whose `: str = ` spacing hid it from a `filepath="src/…"` grep. Classified rather than sed'd: eight further sites keep their wording because they date a past incident in past tense, name a test file that still exists, or are the deliberate `reports_html` local alias. All eight verified unedited. Also deletes CLAUDE.md's `optimize/` tree entry. The directory is absent from HEAD and from main; it lives only on the unmerged `feat/plugin-optimize-skill` branch, so the tree as documented did not match the tree as shipped. No source file changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: code review fixes for the reports-consolidation review fixes Two Medium findings, each flagged independently by a reviewer: - `_layers.py` spelled the 30-char package prefix twice, in the module whose own docstring argues a second copy of a definition is how the two drift. `_NON_CORE` now derives from `_PKG.pattern`, so widening one cannot leave the other behind — a divergence no test could have seen, because both assertions only ever feed src-layout paths. Compiled pattern unchanged. - The trailing `[/\\]` in `(cli|reports)[/\\]` is what keeps a top-level module whose name merely STARTS with `reports` or `cli` core. Nothing pinned it: deleting the separator left all 738 tests green. Pins added for `reports_legacy.py` and `cli_helpers.py`; both fail under that mutation. Also, from the same reviews: - Pin the reachability argument that made the `~/src/coder_eval` residual safe. It lived only as docstring prose; adding CE004 or CE066 to `_ALSO_SCAN_TESTS` would hand them a whole `tests/` tree that matches `_PKG` on an ordinary clone layout. Now one assertion. - `tests/lint/runner.py`'s new grep told the next author to run `^class Test(CE\d{3})`. GNU and BSD `grep -E` read `\d` as a literal `d` and report zero hits — "no ids taken", the exact miss the note exists to prevent. Respelled `[0-9]{3}`. - CLAUDE.md's tree audit ran one way only. Deleting the phantom `optimize/` entry was right, but `errors/` and `plugins.py` exist and were absent — and `plugins.py` is the SPI the "Adding a New Agent" section points at. - Record `ce048`'s near-variant of the shared path regex in the candidates entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(lint): CE067 — CLAUDE.md's tree must name every top-level package member Harness loop, closed in-session. The directory tree is the map an assistant reads before touching this package, and it drifted in both directions across two consecutive plans while only one direction was ever audited: a phantom `optimize/` row survived for a directory that lives solely on an unmerged branch, and the sweep that removed it walked entries -> filesystem, so it could not see that `errors/` and `plugins.py` were missing — the second being the plugin SPI that "Adding a New Agent" tells you to use. Asserts both directions over the top-level rows only; nested rows stay illustrative, so a new sibling module is not a forced docs edit. Proven to fire each way: re-adding the `optimize/` row and deleting the `plugins.py` row each fail with the offending name. Writing it also found a third omission the hand audit missed — `__init__.py`, here deliberately ignored along with the build and typing markers. Claims id 067; the Phase 2 parity sensor required the pyproject entry immediately, which is the sensor doing its job. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(lint): drop a personal path and de-duplicate the isolation pin No test datum should carry a developer's home directory. The repo-root case now uses `/home/dev/src/exp/coder_eval/conftest.py`, which keeps the shape that mattered — a `src` component that is NOT the package's parent, so it exercises the anchor rather than merely the absence of `src`. `ISOLATION` and a verbatim three-line docstring were copied into both the CE004 and CE066 test classes. Hoisted to one module-level `CORE_ISOLATION` with the rationale stated once, so the two pins cannot drift to different paths. Verified the pins still bite: re-exempting `isolation/` in `_NON_CORE` fails four tests. Also renames `test_ce008_skips_files_outside_scope` to `ce009` — it sits in the CE009 block and exercises `YamlModelsForbidExtras` (CE009), while CE008's own tests cover `ReadTextExplicitEncoding`. Pre-existing mislabel, flagged in review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(lint): drop two function-local json imports that shadow the module one `tests/test_custom_lint.py` imports `json` at module level, and two tests re-imported it inside the function body. CodeQL flagged the one this PR added (in the pricing-mirror test); the other, in the activation-rows test, predates the PR and has the same shape, so both go. Ruff has no rule for a repeated import inside a function body, which is why `make check` never saw either. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): CE004 checks reports/ — stop borrowing CE066's exemption set CE004 and CE066 shared one predicate, `is_core_path`, whose exemption set is `{cli, reports}`. That set is CE066's: the reports package may reach into itself. CE004's is only `{cli}` — the reports package runs without the CLI (the orchestrator writes a task report mid-run), so a `cli` import added inside `reports/` closes a cli -> orchestration -> reports -> cli cycle, and CE004 would have stayed silent. Latent, not live: widening the scope finds 0 violations. The rules still share what must not drift — the anchored package regex and the `cli/` boundary, now `is_package_path` and `is_cli_path` — but each states its own exemptions. CE066 keeps `is_core_path`; CE004's scope is the package minus `cli/`. Tests, each mutation-checked against CE004 going back to the core predicate: - `test_the_reports_package_is_in_scope` — a `cli` import in reports/ violates. - `TestCoreLayerMembership.test_ce004_scope_is_every_module_outside_cli` runs the RULE at every real module path. Its first draft recomputed the scope from the helpers and passed under that mutation, so it now calls the rule. - `test_the_reports_package_itself_stays_exempt` pins that CE066's scope did not widen with CE004's. Closes the deferred harness candidate; architecture notes updated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): resolve a relative import against the importing file CE004 and CE066 matched `node.module` against the bare package name whenever `node.level` was non-zero, so `from .reports import x` inside `orchestration/` — i.e. `coder_eval.orchestration.reports` — read as the reports layer, and `from ...reports import x` from a sub-package read as it too although it escapes `coder_eval` entirely. Latent: nothing in the tree is nested that way today. `_absolute_module` now resolves the dots against the file's own package, so both rules compare one fully-qualified name in either spelling. The CE004 scope probe moves to the absolute spelling: a relative one resolves against the importing file, so `..cli` names the cli layer only from inside a sub-package and that test would measure depth instead of scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: reconcile the reports split with main's docs restructure Rebase fallout from #174/#177, which replaced CLAUDE.md's fenced directory tree with `ls` plus selective bullets, split `.claude/architecture-notes.md` into `.claude/notes/`, and added the `make docs-budget` prose gate. - Drop CE067. It asserted that CLAUDE.md's tree names every top-level package member; that tree no longer exists, and the bullets that replaced it are deliberately not exhaustive, so the rule's invariant is now false by design. The surviving half — no bullet may name a path that does not exist — is recorded in .claude/harness-candidates.md rather than rebuilt here. - Move the reports-package rationale into `.claude/notes/reporting.md`, and retarget `reports_stats` / `reports_junit` in notes/ to their new homes. - Bring `result_metrics.py` and `run_record.py` under the prose budget by adopting main's trimmed wording for the comments and docstrings they inherited, including its `.claude/notes/` pointers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(pricing): carry the exemption set into the generated mirror The generator kept only `per_request_billing` as an exemption axis, so it reproduced three of the hand-copy's seven exclusions and silently priced the other four. Those four were not drift: `gpt-5.4-mini`, `gpt-5.4-nano`, `gpt-5.4-pro` and `gpt-5.5-pro` were enumerated exemptions in the parity test this branch deletes. Pricing them is a behaviour change smuggled in as a refactor — latent only because no experiment config references them today. The exemption set encoded two different claims and they now survive separately. That a routed model MUST NOT be priced is a fact about the rate, and stays on `ModelPricing.per_request_billing`. That a heavy frontier variant is not worth pricing on the board is a fact about the FRONTEND, so it is `DELIBERATELY_UNMIRRORED` beside the generator. What the hand-copy got wrong was not having an exemption set but letting it go stale unnoticed, so the deleted test's staleness guard comes back as `_assert_exemptions_are_live`: an id that has left `pricing.py` fails `make pricing-mirror` and CE065 rather than sitting there silencing nothing. The generated table now reproduces the hand-copy's 52 keys exactly — no additions, no removals. CE065 asserts both axes from their declared sources, and the evalboard's consumption guard pins the four as unpriced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
uipreliga
added a commit
that referenced
this pull request
Sep 16, 2026
`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>
uipreliga
added a commit
that referenced
this pull request
Sep 16, 2026
…s well as the file total (#180) * feat(lint): 1/8 — scan a _ROOTS tuple in the prose budget 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> * docs(tests): 2/8 — move the CE catalogue from notes/README.md to lint-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> * docs(tests): 3/8 — move lint-rule defect stories into notes/lint-rules.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> * docs(tests): 4/8 — move doc-surface rule rationale into notes/lint-rules.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> * docs(tests): 5/8 — move golden-sensor and bracket-clock rationale into 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> * docs(tests): 6/8 — move plain test-module rationale into the subsystem 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> * fix(docs): wrap an over-long line in the CE063 docstring 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> * docs(tests): 7/8 — delete HISTORY prose from tests, fix dangling docstring 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> * feat(lint): 8/8 — turn the prose budget gate on for tests/ 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> * fix(lint): make the prose-only proof see added directives and reject 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> * docs: record two prose-budget guard gaps the final review found 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> * fix(lint): include untracked files in the prose-only proof 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> * fix: code review fixes for tests-slim-prose 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> * fix(lint): point main's exempt-pair test at the repo root `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> * docs(lint): slim the five essays main's reports split brought in 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> * refactor(lint): delete CE023, which guards a package that no longer exists `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> * feat(lint): cap the comment RUN, replacing the per-file comment budget `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> * feat(lint): keep the file-total comment budget as a backstop under the 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> * docs(lint): answer review — cut runs off the cap, drop lint-rules.md 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> * fix(lint): fail the prose gate cleanly on a missing root, and name the 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> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cuts 79,754 words of essay-shaped prose in
src/coder_evalto 17,752, and 126 docstrings over 150 words to zero, by moving design rationale into.claude/notes/and leaving a resolvable pointer at each source site.No code changed.
python -m tests.lint.prose_budget --assert-code-unchanged 9af3db3dcompares a docstring-strippedast.dumpplus a per-file multiset of functional directive comments (# noqa,# type: ignore,# pyright:,# nosec,# pragma:,# fmt:) and reports nothing across all 14 commits. All 116 directives intact.What moved, and what stayed
Every passage was classified: CONTRACT and HAZARD stay in source (compressed); RATIONALE moves to
.claude/notes/<subsystem>.md; HISTORY is deleted (git holds it); a CROSS-MODULE CLAIM is deleted and linked to its owner..claude/architecture-notes.mdis replaced by nine subsystem files under one index.The gate
make docs-budget, wired into bothmake verifyandpr-checks.yml'squality-gate(CI restates every step as.venv/bin/...and never callsmake, so both seams were needed). Four checks:Rationale: <path> § <heading>pointer resolves$-anchored regex cannot see itMAX(20, 0.15 × file length)The last two replaced a hand-maintained tree-wide word count. Nothing in the gate is now a number anyone maintains: delete code and the comment budget shrinks with it, and a new file is governed from its first commit. Exemptions are categorical — an
Args:/Returns:/Raises:block is structure not prose, and an@abstractmethoddocstring IS the interface contract implementers read.Reviewing this
The diff is large but uniform. The highest-value thing to check is whether any compressed docstring now asserts something false — that produced every real defect found here, and the worst case was a contract not dropped but replaced with a plausible wrong reason that invited the regression it originally forbade. Those are fixed in
0fe3e0a7,fd72e9aaandf7b5a193; the commit messages name each one.Three known items, all recorded rather than hidden:
isolation.mdstill carries a 19KB single-line bullet from Phase 1. A reviewer wanted it deleted as duplicated; measurement says later phases paraphrased rather than copied it, so deleting would lose detail. Left as a judgement call.tests/(105,417 lines) is unmeasured — the plan's own open question..claude/harness-candidates.md; the strongest is a single-home detector for the notes tree.Verification
5,733 tests / 2 skipped ·
make lint635 · pyright 0 · coverage 92.60% ·make evalboard-verifygreen ·make plugin-referenceandmake docs-indexesproduce no diff · all 8 public--helpoutputs byte-identical to before the work.🤖 Generated with Claude Code
Added after review
Review fixes, then a second pass over
CLAUDE.mditself.From the review — the prose gate penalized usage examples:
_TRAILING_SECTIONSaccepted anExample:block after a pointer while_DOCSTRING_SECTIONScounted it against the 150-word bar, so a docstring in the house style could fail the gate. Both deleted examples are restored, withexecute_with_retry's keyword contract. The CE catalogue innotes/README.md(2,165 words, disclaimed as non-authoritative, targeted by no pointer) and "Not wired up yet" inpermissions.md(769 words designing code that does not exist) are deleted._TYPER_COMMANDSgains a staleness test.Not done, with evidence — the review also asked for the per-file abstracts to go as restatements of the sections beneath them. 61 of the isolation abstract's 191 code identifiers appear nowhere else in
notes/,docs/orCLAUDE.md, including--allow-recorded-commands,task_config.resolvedand_seed_from_prior_result'searly_stopcarry. Deleting them loses rationale rather than a duplicate. The readability half is fixed instead: every wall bullet is reflowed to 88 columns (the largest was a single 19,453-character line), andreporting.md's pseudo-tree becomes prose. Re-homing those claims is a follow-up.CLAUDE.md: 2,802 → 2,028 words. It is loaded every session, so a copy kept there costs context on every turn and goes stale with nothing to sense it. Three sections were copies:optimize/, which exists on neither this branch nor main, and omittederrors/andplugins.py, both present since the initial public release. Now a list of what a filename cannot tell you, opening with "runls".CLAUDE.mdtest exists for.docs/EXTENDING.mdstep for step, 190 lines below a section that says "Each entry is a pointer".Also reverts a run-limit parity sentence claiming an adapter rejects an unsupported field at load time. It does not:
opencode_agent.py:785andpi_agent.py:738both warn and continue. Neither lint-pinned surface is touched.Rebased onto
7bf89661(the harbor bind-mount rewrite). Five conflicts inharbor/packager.py, all resolved as main's code plus this branch's prose rules; two of its new docstrings were over the bar and are slimmed, with the rationale moved intoreporting.md.Verification after the rebase: 5,737 tests / 2 skipped ·
make lint635 · pyright 0 · coverage 92.57% ·docs-budget0 essays · all 15 CI checks green.