refactor(builtin-skills): state comment rationale in present tense

[bolichen97]

Problem / Motivation

Comments in the builtin_skills scripts told the story of how the code got
here instead of what it does now. fleet_probe.py opened a paragraph with
"This comment used to say ... That premise is wrong"; claim_preflight.py
said "it used to be CLOSE" and "The first version applied any withdrawal to
every earlier claim"; ledger_entry.py cited a PR number; three comments
carried an incident date.

A reader has to work out which half is current before they can trust any of
it. The repo already says so twice: code-style.md requires present tense,
and scripts/check_comment_history.py is a shrink-only CI ratchet that
counts these markers per file.

Why it matters

These are the scripts the pipeline, security and goal conductors run to
decide whether to dispatch work, close somebody's item, or suppress a queued
one. The comments are the only place the reasons live. A reason written as
history makes the next reader guess, and guessing here withholds work or
acts on somebody else's item.

The ratchet also only moves when someone moves it.

What changed (motivation → approach → change)

Every comment in these five scripts that narrated a past revision now states
what is true. Nothing was deleted for being long: each rewrite keeps the
failure the comment guards against, the measurement behind it, the count, and
the identifiers — and drops only the account of which version had the bug,
which git already holds.

The ratchet is the criterion, so the file set is not a matter of taste. It
counts 16 markers in these five files, and --write-baseline records the drop
to 4 — twelve fewer, four entries pruned outright. Those 4 stay on purpose:
they are the English phrases claim_preflight.py's own closure regexes match
("no longer an issue"), quoted by the comment that explains them, so rewriting
them would make the comment wrong.

--write-baseline also recomputes _total, which the base carried one higher
than its own files sum. So the header moves by 13 while this diff accounts
for 12; the per-entry diff is the honest number.

One code line changes. security-conductor/scripts/ledger.py computed
active = (1 if state[0] else 0) if state is not None else 0 in the
propose-golden-path report path. _read_golden_path_state already returns
bool(int(row["active"])), so int(state[0]) is the same value and the same
int type, and the emitted JSON is byte-identical.

The prepare-pr/scripts/ bundle keeps its 19 markers for its own pass: PR
#9308 is rewriting preflight.py, and pr_status.py already carries 10
markers against a baseline of 9 on main, so touching it would red the gate
for a reason this PR did not cause.

Tests

No new tests. Comment prose has no test to add, and the one code line is
already pinned: test/test_security_conductor_ledger.py asserts the exact
emitted object ({"id": ..., "created": False, "active": 1} and "active": 0)
for a fresh proposal, a dedupe hit on an approved row, and a hand-retired row.

scripts/check_comment_history.py is the gate that locks the baseline in, and
it fails if any of these five files grows a marker back.

Ran green: test_pipeline_conductor_*, test_security_conductor_*,
test_conductor_ledger_entry, test_ledger_conductor_agent,
test_builtin_skill_{scope,packaging,sync_safety}, test_web_verify_downscale
— 1166 passed. Plus flake8, isort, mypy, black-formatting,
subprocess-encoding, comment-history, brand-name, harness-parity, docs-lint
(both), per-file-coverage, builtin-skill-scope, vendor-manifest and
changelog-history, all exit 0.

Manual verification

N/A — unit coverage sufficient. The one behavioral surface is the
propose-golden-path JSON, and a differential run of the pre- and post-change
_dispatch over eight stored states (absent row, inert, approved, retired,
and four corrupt column values) produced byte-identical stdout.

Review fleet

Before opening, five parallel subagent lenses reviewed the diff (AUTOSDE +
blocking rules, behaviour preservation, import semantics, security keystone,
comment accuracy), then two more verified the enlarged diff. What they found
and what happened:

• Fixed. The comment-history ratchet would have gone red: removing a marker
  from a baselined file requires lowering its entry in the same commit, and the
  gate reads as green until the change is committed. The baseline is now in the
  commit.
• Fixed. Three claims I introduced were false. claim_preflight.py's
  docstring said Rule 3 is "SKIP and not CLOSE" — it is REVIEW (verdict table
  line 109, exit code 13). A tense flip made "returns CLOSE on a live item"
  present tense, which nothing on that path does any more. And "two days behind
  the newest" overstated a 38-hour measurement.
• Fixed (First Principles Review's subtractions, over two rounds): three
  narration clauses the ratchet's patterns miss — claim_preflight.py:188, and
  the plain_prose and rule-table paragraphs where "is now" and a line-wrapped
  "no longer" both slip past the gate. The criterion is now a tokenize+AST scan
  of every comment and docstring in the touched files, not the gate's patterns;
  it returns 11 remaining hits, all runtime state or quoted regex text.
• Refuted. A suggestion to keystone-exclude these scripts inside this PR:
  that list lives in the sweep tooling, not the repo, and it is updated there.

Related Issues

no linked issue: a comment-hygiene and readability pass, not a reported defect.

Why no screenshot: backend only — five skill scripts and one gate baseline;
nothing under website/ and no rendered surface changes.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[bolichen97]

Intent: Make the builtin_skills conductor scripts' comments state what the code does now, so a reader trusts them without reconstructing which half is historical — and move the repo's shrink-only comment-history ratchet down by 12 markers while doing it.

Not a goal: Changing any behaviour. Rewriting the prepare-pr/scripts/ bundle's markers (one file is contested by #9308, another already exceeds its baseline on main). Chasing coverage. Rewriting comments whose matched phrase is a regex the comment is quoting.

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed dfbeb3f75063b30116ef6bd314ed323b227dec6b — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] dfbeb3f

Verdict parsed from the review's SHA-scoped output markers for commit dfbeb3f75063b30116ef6bd314ed323b227dec6b.

False positive or not applicable? A repository writer can comment:
/ai-review override fable dfbeb3f75063b30116ef6bd314ed323b227dec6b: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of dfbeb3f75063b30116ef6bd314ed323b227dec6b — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The int(state[0]) change is provably equivalent — _read_golden_path_state returns bool(int(row["active"])) at ledger.py:574, so int() of it yields the same 0/1. The comment rewrites preserve each guard's rationale while dropping only revision narrative, which matches the code-style rule and the ratchet's purpose. One fidelity gap: the baseline diff also lowers shell_normalizer.py (4→3) and test_config_loader.py (13→12) — files this PR never touches — so the header actually moves by 15, not the 13 the description accounts for.

Design-Verdict: PASS

A hygiene pass with a real, gate-enforced criterion; every rewrite keeps the guard's rationale, and the one code line is provably equivalent and test-pinned.

Watch

The regenerated baseline also lowers two files this PR never touches (shell_normalizer.py 4→3, test_config_loader.py 13→12), so _total moves by 15 while the description accounts for 13 ("the header moves by 13 while this diff accounts for 12"). Almost certainly slack capture from --write-baseline and the blocking gate would red if the numbers were wrong, but a reviewer cannot reconcile the baseline hunk from the description as written.
Clears when: the description names the two extra pruned entries (or they're dropped from the baseline hunk), and the comment-history gate is green on CI for this head.

[DESIGN-REVIEWED] dfbeb3f

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of dfbeb3f75063b30116ef6bd314ed323b227dec6b and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] dfbeb3f

False positive or not applicable? A repository writer can comment:
/ai-review override gpt dfbeb3f75063b30116ef6bd314ed323b227dec6b: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of dfbeb3f75063b30116ef6bd314ed323b227dec6b — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All counts verified: claim_preflight.py retains exactly 4 markers (two "no longer" spans on line 276, plus lines 294 and 827 — the quoted regex phrases the description names), matching the new baseline entry. Everything in the diff checks out against the tree. Final review:

First-Principles-Verdict: PASS

Description says the baseline header "moves by 13"; the diff moves it 15 — two extra entry drops for untouched files rode in, both verified accurate.

Not justified as shipped

• Item 3 — undeclared: shell_normalizer.py 4→3 and test_config_loader.py 13→12 are lowered though this PR touches neither file; I counted both files' current markers (3 and 12) and the recomputes are correct — main's drift, picked up by --write-baseline after the merge, contradicting the description's "header moves by 13 while this diff accounts for 12" (actual: 15 and 14).
• Item 4 — rides along (declared): the ledger.py active one-liner; _read_golden_path_state returns bool(int(row["active"])) (ledger.py:574), so int(state[0]) is provably byte-identical and the pinned test already asserts the exact emitted JSON. It is itself a shrink; nothing to act on.

What this change ships

Inventory (4 items) — 2 justified

Intent: make the conductor scripts' comments state current rationale instead of narrating past revisions — a FIX (of documented-invariant violations, code-style.md's present-tense rule).

1. Comments/docstrings in five builtin-skill scripts now state present-tense rationale, keeping every failure, count and measurement — justified
2. Baseline entries for those five files lowered/pruned in the same commit (16→4) — justified
3. Baseline entries for two files this PR never touches also lowered — undeclared; both recomputes verified accurate against the tree
4. active = int(state[0]) replaces the double conditional in security-conductor ledger.py — rides along; verified behavior-identical

[FIRST-PRINCIPLES-REVIEWED] dfbeb3f

[bolichen97]

• Delete the surviving narration clause at claim_preflight.py:188 — fixed in dc75f8448.

Fixed, not rebutted: the finding holds and the description's claim depended on
it. _ISSUE_OBJECT's comment now reads "A bare pronoun resolves to whatever was
last mentioned, and in a bug report that is usually a socket, a file or a handle,
so accepting one fires on 'The connection leaks. Please close it.'" — same three
facts (pronoun resolution, the bug-report noun classes, the concrete sentence),
no account of which revision accepted them.
Deliberately NOT restated as "closed the item": a prose closure request produces
REVIEW, not CLOSE, so the fix says "fires on" instead. The same trap caught three
other rewrites in this PR before it opened.
Class-level ruling, so this covers the whole criterion rather than one line: for
the five files this PR touches, the pass is now
grep -nE "first version|earlier version|used to |previously |we now |An earlier"
returning zero. That is a stricter criterion than the ratchet's own PATTERNS,
which do not match this line — which is exactly why it survived the first pass.
Findings naming another un-ratcheted narration clause in these five files are
covered by this ruling; there are none left.

self-added: no
mechanism: none added — one comment clause rewritten, no new code, structure or contract.

[bolichen97]

• Inventory item 3, int(state[0]) classified "rides along" under Not justified as shipped — rebutted (disproportional to split), diff unchanged at dc75f8448.

The classification is accurate and the review's own reasoning grants the rest:
byte-identical, declared in the description, and pinned by
test_security_conductor_ledger.py:1812-1817.
It is not loose scope. This PR is one module's pass in a module-at-a-time
simplification campaign, and builtin_skills is the module. The detector's
report for it was exactly one actionable site — this ternary — so splitting it
into its own PR yields a one-line PR whose reviewer pays the same fixed cost as
this one, and the campaign's rule is that comment hygiene rides along with the
module's substantive change rather than becoming its own sweep.
Class-level ruling: any finding asking that a provably-equivalent single
expression in the same module be lifted into a separate PR is covered here.
One consequence worth recording rather than leaving implicit: the sweep's
keystone-exclusion list now covers security-conductor/scripts/{scope_check, ledger,verify_fix}.py, so a later pass will not offer this file's expressions
as ordinary work. That list is in the sweep tooling, outside this repo, so it is
not part of this diff.

self-added: no
mechanism: none added.

[bolichen97]

• Drop the surviving narration in the plain_prose docstring, claim_preflight.py:720-721 — fixed in 691566261.

Fixed, and the criterion widened so this class stops recurring. The finding is
right on both counts: it is narration, and it evades the ratchet because "no
longer" is line-wrapped and "is now" is not a PATTERN at all.
The paragraph now reads "…scanning it raw fires the closure branch on a live
item. That branch's verdict is REVIEW, so a false reading does not close work in
flight — but it still withholds the item from dispatch and spends a human read…"
and "The asymmetry is narrow and does not invert". Every fact survives: the
measured item, the quoted phrases, REVIEW's real cost, the missed-detection cost,
and the direction of the asymmetry that puts citation-stripping before matching.
The invariant, not a fourth sibling patch: the criterion for this PR is no longer
the ratchet's PATTERNS but a tokenize+AST scan of every comment and docstring in
the touched files for is now|are now|no longer|used to|first version|earlier version|previously|formerly|we now|now that|any more|has been|was missing. That
found two genuine sites — :83 ("for the SAME reason now that neither one closes
anything") as well as the :720 one this finding names — and both are fixed in the
same push. The remaining 11 hits are runtime state or quoted regex text, not code
history: a process "has been alive", a ruling "no longer owed", and the closure
patterns' own literal English.
This is a class-level ruling: any further finding naming a change-history phrase
in these five files is covered, because the scan above now returns only those 11
and each is enumerated as not-a-violation.

self-added: no
mechanism: none added — two docstring paragraphs rewritten; no code, structure or contract change, and the ratchet baseline is unchanged because neither site was ever counted.

[bolichen97]

• Re-run --write-baseline and confirm a no-op diff; the gate never validates _total — rebutted (verified, no change needed), diff unchanged at 691566261.

Verified on the pushed HEAD, not asserted. python3 scripts/check_comment_history.py --write-baseline prints pruned 0 entr(y/ies), lowered 0; 1674 remain and
git diff --stat is empty, so the committed file is the writer's own fixed point.
_total also self-checks: 6942 equals sum(files.values()) exactly, where the
base carried 6955 against its own sum of 6954. So the −13 header is 12 markers
this diff removed plus the 1 the base was over by, and both halves are
reproducible from the committed file alone rather than from my tooling run.
The observation that the gate does not validate _total is correct and worth
keeping on the record: test/test_check_comment_history.py::TestCommittedBaseline
asserts the committed file parses, that every count is an int >= 1, that no
_vendor path is listed and that every key is under DEFAULT_TARGETS — it pins
_total only for _write_baseline's output on a tmp file. That is a gap in the
gate, not in this diff, and widening the gate's own tests is out of scope for a
comment pass; this PR leaves it strictly better than it found it by making the
committed _total correct for the first time.
Class-level ruling: findings asking this PR to add coverage for the baseline
file's own header are covered here.

self-added: no
mechanism: none added.

[bolichen97]

Maintainer note (@bolichen97, agent-assisted): pushed a merge commit dfbeb3f75 bringing main into this branch. The only conflict was the generated comment-history-baseline.json; regenerated with scripts/check_comment_history.py --write-baseline (pruned 4, lowered 3) and the gate passes. The pipeline-/security-/goal-conductor and web-verify script tests pass on the merged tree (1037 passed). No source changes.