ci: enforce comment-encoding rules with a guard

[lmeyerov]

Standalone off master. Root cause of the class you rejected four PRs running: comment discipline was the only rule in this repo with no automated gate. Type hygiene, cypher surface, coverage floors and the polars lane-completeness lock all have one; comments were enforced by human review alone, so they were the only class that reliably reached you.

bin/ci_comment_density_guard.py — stdlib tokenize+ast, modelled on ci_type_hygiene_guard.py: same per-file ratchet, same --report/--list/--update-baseline/--strict surface, same exit codes, ~3s, identical counts on py3.10 and py3.12. Escape hatch # guard-ok: <check> -- <reason>.

Checks: comment-block (2+ adjacent full-line #; tool directives and the file header neither start nor continue a run; a #: Sphinx run is allowed at 2 and flagged at 3+ — the loophole you called out), perf-claim (O(...) plus faster/slower/vectoriz/measurably/cheap/expensive/hot-path), issue-rationale (a standalone comment or docstring citing #NNN; a trailing # #1891 on a code line is fine).

Acceptance test: 16/16 of your flagged comments

Replayed against the exact commits you reviewed (9e16c74, b645af6, d044e3e; the outdated anchors resolved via original_commit_id). Every one is caught — the query_types.py note, all four "eliminate/shorten" sites, the three lowering.py "should this be a test?" sites, both agg_types.py sites (the second via the #: 3+ rule), the polars chain and hop_eager sites, hop.py, and execution.py.

More decisive than flagging: the ratchet would have failed all three PRs — all 9 touched files exceed their master cap (e.g. gfql_unified.py 32/1/15 → 36/1/18, hop.py 6/4/1 → 10/6/6, query_types.py absent from baseline → 1).

Two deviations from my brief, both forced by that acceptance test

1. comment-block is not restricted to function bodies — query_types.py:9-12 and agg_types.py:53-55 are module-scope inside if TYPE_CHECKING:, and you flagged both.
2. perf-claim/issue-rationale also read docstrings — a claim doesn't become admissible by moving into a docstring, and flatten.py's 22-line narration is a docstring.

False positives: measured, removed, and pinned as tests

Raw perf-claim was 344. Two classes were noise: regress* (101 hits) was almost all correctness — "REGRESSION GUARD — DO NOT REMOVE", plus junk from regressive — and A/B (6) matched a/b/c id columns. Both now require performance vocabulary in the same comment. A comment naming pyg-bench is exempt from the benchmark term: pointing at where the measurement lives is the correct encoding. Net 344 → 216, no loss on the acceptance set. These decisions are locked in tests, not prose — 22 tests in test_ci_comment_density_guard.py; neither existing guard has one.

Master's grandfathered debt: comment-block 948/138 files, perf-claim 216/88, issue-rationale 169/50. The guard also cleans its own five 2-line comments, so it obeys itself.

Honest limits — this is ~two-thirds of the class

It closes comment volume, which is the dominant violation and purely structural. It cannot decide whether a one-line comment earns its place (that is "could a test have been written instead?" — undecidable), and the predictable evasion is compressing narration into one dense 120-char line. It catches a docstring citing an issue or claiming perf, not one narrating mechanism — a length rule was considered and rejected as ~90% noise against 254 legitimate long docstrings. And "should this be a symbol / externed DRY?" is not mechanizable at all.

It also stays a ratchet: lowering.py already carries a cap of 50, and within a cap comments can be freely rewritten. --strict would tighten that over time but fails today.

So it stops the specific thing that reached you four times and shrinks the residue to a review conversation — it does not make the review skill unnecessary.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi

[lmeyerov]

Better to figure out a refactor for an encapsulated folder include?

[lmeyerov]

Done in 38162c5b3 — five lines become two.

Two of the four entries I added were dead. The group's first pattern is '\.py$', unanchored, so bin/ci_type_hygiene_guard.py and bin/ci_comment_density_guard.py were already matched. Listing them changed nothing. Only the two .json baselines ever needed covering.

So the encapsulation is one pattern:

'^bin/ci_.*baseline\.json$'

A future guard now needs no ci.yml edit at all — its script is caught by \.py$ and its baseline by the glob. Verified both directions: matches all four guard inputs plus ci_cypher_surface_guard_baseline.json, and does not match docker/, README.md, bin/something.sh or requirements/*.lock.

Pinned rather than described, by test_every_guard_baseline_triggers_the_python_ci_lane — it parses the live workflow and asserts every bin/ci_*baseline*.json on disk is matched by the python group. Mutation-verified: narrowing the pattern back to a single baseline turns it red. That is the part no comment could carry, because the failure mode is a guard added later whose baseline silently stops triggering the lane.

On the literal folder include (bin/ci_guards/): considered, and deferred rather than dismissed. 10 of the 13 open stack PRs modify a guard baseline, so renaming those files right now would conflict with every one of them — and unwinding exactly that kind of cascade is what most of today went to. Filed for post-release with the other structural moves (alongside the .agents/ migration). The pattern above already removes the per-guard edit that prompted your comment, so the deferral costs nothing ongoing.