docs: rule out refactor narration in engine prose - #718
Open
willhea wants to merge 1 commit into
Open
Conversation
The engine's docstrings had accumulated a kind of prose that describes how the
code got here rather than what it faces: which slice extracted which module, the
order the ADR 0020 conversion landed in, the preservation gates it ran under.
That form never goes stale in a way a test catches -- it was already about the
past when it was written -- so nothing noticed it accumulating.
The rule: describe what the code faced, not how it got here. A corpus shape, a
source-format quirk, a measured cutoff and a labelled rejected alternative cannot
be recovered by reading the code, so they stay. Sequencing is in git log, so it
goes. AGENTS.md carries the two new bullets; docs/comment-style.md carries the
worked good/bad examples for all seven rules, drawn from this repository.
Applied worst-first over src/: 97 prose lines removed, and with them five ungated
measurements (+0.9%, 5.552s->5.600s, 1.62x/2.9x/2.4x, 593 invocations, 14,001
pairings) that no assertion or repro could check, now either dropped or moved
into History: tails where a frozen figure stays true. Two dangling symbols went
with them -- `_emit_pair` and `_pdf_similarity_signals`, named in five passages as
the baseline current behaviour was compared against. Neither exists, so the
comparisons had no reachable baseline.
Why not a gate: a pattern over engine prose was written, run, and rejected. The
rules that matter turn on a sense distinction no regex makes -- `is used to
constrain` and `used to be pre-truncated` differ only in the word before them --
and narrowing one until it stops misfiring inverts the failure rather than fixing
it. What survives the narrowing is the vocabulary of whichever migration prompted
it ("slice 6a", the bare "B3" label), so it passes green while the next migration
writes the identical rot in its own words, and the green reads as coverage of the
rule rather than of one finished instance. It was also on a collision course with
the PDF pipeline's own nouns: the phase-label pattern was /\bB\d[a-z]?\b/ against
a pipeline whose central unit is the block, already cited positionally as
"p.61 L5". docs/comment-style.md records this, so the next attempt starts from the
reasoning rather than repeating the experiment.
bill_tree.py is deliberately untouched. It sits inside an ADR 0019 parser-revision
closure hashed over raw file bytes, so a four-line docstring compression there
moves the revision and reddens 27 stored sentinels. Behaviour was unchanged --
test_canonical_baseline, test_pdf_observation_emission and
test_pdf_canonical_baseline all stayed green -- but the sentinel says to re-derive
and re-review rather than regenerate, which is its own change. The constraint is
now written down in both docs.
Verified against develop at 9d6f00a: ruff check and ruff format --check clean;
fast 2074 passed, browser 41 passed, slow 1752 passed (network gate deselected).
test_round1_pairing_sentinel collects and passes, so the closure is undisturbed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ci3piRPLEwChbZ5UAx331L
Claude-Session: https://claude.ai/code/session_01WjekSFzYLqVnwZdDHHXSPc
willhea
force-pushed
the
worktree-comment-style-rule
branch
from
September 9, 2026 21:15
abfeb94 to
37c3b12
Compare
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
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.
What changed
Engine prose in
src/deltatrack/had accumulated a kind of comment that describes how the code got here rather than what it faces: which slice extracted which module, the order the ADR 0020 conversion landed in, preservation gates that already passed.That form is the one kind of stale comment no test can catch. It was already about the past when it was written, so it never contradicts the code, never gets flagged, and accumulates indefinitely. It was ~97 lines by the time of this audit.
The rule: describe what the code faced, not how it got here. Domain knowledge is load-bearing and stays (corpus shapes, source-format quirks, measured cutoffs, labelled rejected alternatives). Migration narration goes, because
git logalready holds it.AGENTS.md— two bullets in "Comments and rationale", plus a pointer. Deliberately short: that file loads every session.docs/comment-style.md(new) — the seven rules with Bad/Good examples drawn from this repository, a keep/cut table, the parser-closure constraint below, and a section on why these rules are held by review rather than by a pattern.src/deltatrack/files — 97 prose lines removed.Held by review, not by a test
A pattern gate over engine prose (
tests/test_comment_conventions.py) was written, run against the whole engine, and rejected before review. It is not in this diff. The reasoning is recorded in the guide and in the commit message, so the next person to reach for one starts from it rather than repeating the experiment.The gate matched the vocabulary of one finished migration:
slice 6a,this slice, the bareB3phase label. That conversion is over, so those words cannot recur. The next migration writes the identical rot as "phase 2" or "step 3" and the gate stays green through it, while the green reads as coverage of the rule rather than of one instance of it.The narrowing that kept it quiet is precisely what made it retrospective, and the two can't be separated. Widen the pattern and it fires on the engine's own legitimate prose ("the
[start, end)slice", "trimming the slice"); narrow it and it only knows the past.It was also on a collision course with the PDF pipeline's own vocabulary. The phase-label pattern was
\bB\d[a-z]?\b, and that pipeline's central noun is "block", already cited positionally asp.61 L5. The firstB12for block 12 fires on correct prose, and the fix would have been to weaken the pattern: the silenced-rather-than-obeyed failure the guide itself warns about, arriving through the front door.Rule 6 (prose may not name code that does not exist) is the one form that is a fact about the repository rather than a judgement about sense, and the one a reader genuinely cannot catch, since nothing in a sentence announces that its subject was deleted. The guide notes it as the piece to mechanise on its own merits if it recurs, rather than as a rider on a gate that doesn't earn its place.
What the cleanup removed
Beyond the phase vocabulary: five ungated measurements that no assertion or repro could check (
+0.9%,5.552s→5.600s,1.62x/2.9x/2.4x,593 invocations,14,001 pairings), either dropped or moved intoHistory:tails where a frozen figure stays true.And two dangling symbols,
_emit_pairand_pdf_similarity_signals, which five passages named as the baseline current behaviour was compared against. Neither exists; the comparisons had no reachable baseline.The constraint that shaped the scope
bill_tree.pyis deliberately untouched, and this is the most important thing to carry forward.bill_tree.pyand the PDF parser modules sit inside an ADR 0019 parser-revision closure, and that revision is a SHA-256 over the raw file bytes, comments and docstrings included (tests/round1_identity.parser_revision,deltatrack.pdf_observations.pdf_parser_revision). A four-line docstring compression inbill_tree.amount_textmoved the revision and reddened 27 cases intests/test_round1_pairing_sentinel.py, each saying the stored ordinals may now address different nodes.Behaviour was provably unchanged (
test_canonical_baseline,test_pdf_observation_emissionandtest_pdf_canonical_baselineall stayed green), but the sentinel says "re-derive and re-review; do not regenerate to make this green." Regenerating fixtures inside an unrelated prose tidy-up is the wrong move, so that file was reverted and the constraint written into both docs.Follow-up work item: cleaning prose in
bill_tree.py(andpdf_text.py/pdf_anchors.py, also in closures) is worth doing as its own change, batched with a deliberate sentinel re-derive that a maintainer reviews. Please don't fold it into anything else.Where the audit stopped, and why
Remaining long docstrings in
diff_bill.py(assign_group,match_nodes_with_stage_outputs,_match_collision_group) andmatching.pywere judged legitimate and left alone: they describe this code's live constraints ("reads the ordered population and the supplied evidence, and nothing else"), not its history. That is the rule working, not the rule running out.Total
src/prose is still ~47% of non-blank lines, and that is not all removable. Prose density in this repo tracks epistemic difficulty:formatters/diff_html.pysits at 19% and is the most readable file in the package;parsers/pdf_anchors.pysits at 59% because corpus-tuned heuristics cannot be self-evident and the tuning evidence is the specification. No uniform target should be applied.Verification
All five CI gates run locally against current
develop(9d6f00a3), on macOS / Python 3.12. Every gate was re-run against the final state of the branch, after the gate removal:uv run ruff check .uv run ruff format --check .uv run pytest -m "not slow and not browser"uv run pytest -m browser --run-browseruv run pytest -m slow --deselect tests/test_govinfo_corpus_parity.pyThe slow tier is the one that matters most here:
tests/test_round1_pairing_sentinel.pycollected and passed, confirming the parser-revision closure was not disturbed even thoughparsers/pdf_blocks.pyis in the diff.Provenance
The prose audit was authored by Claude Code in a remote session, which could not push (the Claude GitHub App is not installed for the org, so reads succeeded and writes were refused). Transferred as a patch and applied locally onto current
develop, where it applied without conflict. Gates were re-run locally against the moved base rather than carried over.🤖 Generated with Claude Code
https://claude.ai/code/session_01WjekSFzYLqVnwZdDHHXSPc