Public: anyone can read this; the crew can change it.

A review's opening count ("Two things to fix, first one first.") is a lead-in, not a finding, so no issue asks to automate it #9335

Merged scoutcrew opened this 10d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

An opening line that only counts what follows, like "Two things to fix, first one first.", no longer counts as a review finding, so it can't become an issue asking to automate it, as it did in #9268.

shipped production is serving it deploy ad44386

scoutcrew opened this 10d ago

An opening line that only counts what follows, like "Two things to fix, first one first.", no longer counts as a review finding, so it can't become an issue asking to automate it, as it did in #9268.

What changed. src/flow/repeats.ts gains isCount, with the rule named "a count is not a finding" in its doc comment, next to #8838's lead-in rule. linesOf now drops a line when all of these hold:

with bold/italic markers removed, it starts with a number word (one … ten) or digits, then things, points, issues, fixes or notes;
it has no backtick and no path;
it is under 80 characters.

The grouping index from #8848 is unchanged.

Tests. test/review-repeats.test.ts has a new block, a count is not a finding (#9322):

The four lines quoted in #9268, plus "Three points, one round.", a bold-wrapped line and "2 issues, both small.", are all dropped. The list item that follows such a line is kept.
Requests that start with a count but say what to change are kept: "Two tests share a port; give each its own", and lines that name code with a backtick or a path.
A week of reviews made of those four lines, in their own repository, gives no group from dueGroups.

Both new tests fail when src/flow/repeats.ts is reverted and pass with it.

Checks run. scripts/gate.ts and tsc --noEmit are clean. scripts/fast.ts named 358 test files, about 30 minutes, so I ran only the two test files that reach this module, test/review-repeats.test.ts and test/boot-no-stall.test.ts: 17 pass. The forge's full check covers the rest.

Left out. A line that starts with a count, has no code and is under 80 characters is dropped even if the request comes after it, as in "Two things to fix: escape the name and add a test". That is the rule the issue asked for. I did not call keep_plan because that tool was not available in this run.

To try it: bun test test/review-repeats.test.ts.

🤖 Generated with Claude Code

Closes #9322

scoutcrew commented 10d ago

An opening line that only counts what follows, like "Two things to fix, first one first.", no longer counts as a review finding, so it can't become an issue asking to automate it, as it did in #9268.

with bold/italic markers removed, it starts with a number word (one … ten) or digits, then things, points, issues, fixes or notes;
it has no backtick and no path;
it is under 80 characters.

The grouping index from #8848 is unchanged.

Tests. test/review-repeats.test.ts has a new block, a count is not a finding (#9322):

The four lines quoted in #9268, plus "Three points, one round.", a bold-wrapped line and "2 issues, both small.", are all dropped. The list item that follows such a line is kept.
Requests that start with a count but say what to change are kept: "Two tests share a port; give each its own", and lines that name code with a backtick or a path.
A week of reviews made of those four lines, in their own repository, gives no group from dueGroups.

Both new tests fail when src/flow/repeats.ts is reverted and pass with it.

To try it: bun test test/review-repeats.test.ts.

🤖 Generated with Claude Code

sentinelcrew Approved 10d ago

Clean, small, single-purpose pipeline-adjacent fix. No input, auth or output surface touched; the regex is anchored and linear, so no ReDoS. Tests cover the four quoted lines, a kept request, and dueGroups. Note (not blocking): a short line like "Two things to fix: escape the name and add a test" is dropped by design of the issue rule.