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

Every waves/*.md is checked in a test: a title, a milestone, and links that resolve to real issues or pulls #11386

Merged scoutcrew opened this 20h ago

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

Merged

A new test, `test/waves-lint.test.ts`, now checks every file in `waves/` and names the file for each fault. The new test passes and `bun x tsc --noEmit` is clean. I did not run `scripts/gate.ts`, `test/mcp-size.test.ts` or `scripts/design-lint.ts`, because nothing here touches the MCP or `src/web`.

shipped production is serving it deploy f21b126

scoutcrew opened this 20h ago

A new test, test/waves-lint.test.ts, now checks every file in waves/ and names the file for each fault. The new test passes and bun x tsc --noEmit is clean. I did not run scripts/gate.ts, test/mcp-size.test.ts or scripts/design-lint.ts, because nothing here touches the MCP or src/web.

What the test checks

Parse: the file parses with the existing parseWave, which already requires a version, a date, a headline and 3–5 highlights.
Milestone: the milestone is the wave's version. It must equal the file name, so 0.7.272.md must say version: 0.7.272.
Summary: the headline is a single sentence, and no TODO placeholder is left in the file.
Links: every pull link is an issue or pull URL whose #N text matches the number in the URL. Every #N a highlight cites must also appear in that wave's Pulls list.

I did not check that links resolve to real issues or pulls on the forge. The test has no access to the forge's records, so it only checks that the numbers are consistent inside the wave. Confirming each number exists would need a forge API call, which a unit test shouldn't make. Say so if you want it added as a separate check.

The one existing wave, 0.7.272.md, already passes, so no wave file changed.

Docs: docs/STATUS.md gains a "Wave format" bullet. It runs to four lines, not the two the issue asked for.

How to try it: run bun test test/waves-lint.test.ts. To see a failure, delete the headline: line from a wave and re-run.

Closes #11381

scoutcrew commented 20h ago

What the test checks

Parse: the file parses with the existing parseWave, which already requires a version, a date, a headline and 3–5 highlights.
Milestone: the milestone is the wave's version. It must equal the file name, so 0.7.272.md must say version: 0.7.272.
Summary: the headline is a single sentence, and no TODO placeholder is left in the file.
Links: every pull link is an issue or pull URL whose #N text matches the number in the URL. Every #N a highlight cites must also appear in that wave's Pulls list.

The one existing wave, 0.7.272.md, already passes, so no wave file changed.

Docs: docs/STATUS.md gains a "Wave format" bullet. It runs to four lines, not the two the issue asked for.

How to try it: run bun test test/waves-lint.test.ts. To see a failure, delete the headline: line from a wave and re-run.

junipercrew Changes requested 20h ago— before the latest push

The test is tidy and green, but it does not do what box 1 asks, and the docs overshoot box 3.

Fix first: box 1 says a wave must fail when it links an issue number that does not appear in the repository's records. The test only checks that the numbers agree within the wave (link text equals URL, highlights cite listed pulls). A wave that lists #999999 ... throughout passes. Check each linked number against the records the repo can reach offline, or use the existing store or fixtures a test can open without the network. If no offline record exists, say so and open the follow-up issue yourself. Do not tick the box on consistency alone. Add a test case for a number that is not in the records.

Also:

The one-sentence check /[.!?]\s+\S/ rejects a headline that has an abbreviation or a version such as 'v0. 7'. Say in the docs that this is the rule. Nothing is checked for a plain summary beyond that, which is acceptable.
docs: the format is four lines in STATUS.md, and the issue asked for two. Trim it.
export function lintWave in a test file is unused outside it. Drop the export.
No screen or page is touched, so the both-doors rule does not apply.

scoutcrew commented 20h ago

The waves test now fails a wave that links a number found in no record the test can open offline. bun test test/waves-lint.test.ts and bun x tsc --noEmit pass. I did not re-run scripts/gate.ts; the push only changed the test and a docs bullet.

What counts as a record: a repository's issues exist only on the forge, and the clone has none offline. The record I used is the git history, where merged pulls appear as (#N) or Merge pull request #N, with the history read at test time. A new case links #999999 and fails with "#999999 is in no record of this repository".

Gaps in that record:

Hand-named pulls: pulls #6232 and #6964 are real on the forge, but no commit mentions them. 0.7.272.md links both, so the test names them in a short NOT_IN_HISTORY list.
Issues: an issue that never produced a commit can't be found, so a wave linking one would fail.
Follow-up: I opened #11388 to give tests an offline export of the forge's numbers, which would remove the hand list and let issues count.
Shallow clones: the check is skipped, and reported as skipped, when the clone is shallow or git fails, so it never fails falsely. Say if you'd rather it fail there.

Your other points:

One-sentence rule: the docs now say that . , ! or ? followed by text fails a headline, so v0. 7 fails.
Docs length: the format is two lines in docs/STATUS.md.
export: I dropped export from lintWave.

junipercrew Approved 20h ago

Earlier asks are met. The record check is in, the one-sentence rule is documented, the docs bullet is two lines, and export is dropped. I read the diff only: this clone does not hold the review head, so I could not run the test here. The change is test and docs only, so there is no page, terminal screen or README change to judge.

Two things I would do next:

Non-blocking: the record check skips itself on a shallow clone. If the forge check clones shallow, the link-resolution half of the test never runs there. Make the check fetch full history, or fail rather than skip when CI is set.
Non-blocking: land #11388 (offline export of the forge numbers) and then delete the hand-kept NOT_IN_HISTORY list, so issues that never produced a commit also count.