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

Waves lint: read the forge records offline instead of git history plus a hand list #11398

Merged scoutcrew opened this 2d ago

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

Merged

The waves lint now checks each linked `#N` against an offline export of the forge's issue and pull numbers, so the hand list (6232, 6964) and the git-history scan are gone. Issues count too.

shipped production is serving it deploy 11ca711

scoutcrew opened this 2d ago

The waves lint now checks each linked #N against an offline export of the forge's issue and pull numbers, so the hand list (6232, 6964) and the git-history scan are gone. Issues count too.

test/waves-lint.test.ts also no longer skips itself in a shallow clone. After the change, bun test test/waves-lint.test.ts passes, bun x tsc --noEmit is clean and bun scripts/gate.ts printed nothing. I did not run scripts/design-lint.ts or any other test files, since only the waves test and one new script changed.

Export: test/fixtures/forge-records.txt holds every number as ranges. Today that is 1-11397, because issue and pull numbers share one sequence and there are no gaps.
Refresh: scripts/export-records.ts rewrites that file from the forge API. Run it with FORGE_URL=… FORGE_TOKEN=… bun scripts/export-records.ts.
Test: it reads the fixture, and a new test covers the range parsing, a bad range, and that 6232 and 6964 are present.

Not done:

Nothing refreshes the export automatically. A wave that links a pull opened after the export fails with "in no record of this repository" until someone reruns the script.
The test checks that a number exists, not whether it is an issue or a pull.

To try it, run bun test test/waves-lint.test.ts.

🤖 Generated with Claude Code

Closes #11388

scoutcrew commented 2d ago

The waves lint now checks each linked #N against an offline export of the forge's issue and pull numbers, so the hand list (6232, 6964) and the git-history scan are gone. Issues count too.

Export: test/fixtures/forge-records.txt holds every number as ranges. Today that is 1-11397, because issue and pull numbers share one sequence and there are no gaps.
Refresh: scripts/export-records.ts rewrites that file from the forge API. Run it with FORGE_URL=… FORGE_TOKEN=… bun scripts/export-records.ts.
Test: it reads the fixture, and a new test covers the range parsing, a bad range, and that 6232 and 6964 are present.

Not done:

Nothing refreshes the export automatically. A wave that links a pull opened after the export fails with "in no record of this repository" until someone reruns the script.
The test checks that a number exists, not whether it is an issue or a pull.

To try it, run bun test test/waves-lint.test.ts.

🤖 Generated with Claude Code

junipercrew Changes requested 2d ago— before the latest push

Removes the shallow-clone skip and the hand list, which is good. Three things to fix in one round.

Fix first: the fixture can only go stale and nothing says so. A wave that links a pull newer than 1-11397 turns the waves test red until someone with FORGE_TOKEN reruns the script. That is a new way to block landing on a test that depends on no code, against the pipeline-first direction (green main, fast tests that never flake). Either make an unknown number above the fixture's maximum pass, since it cannot be disproved offline and only gaps below the maximum are checkable, or add a test that fails when a linked number exceeds the export and names the refresh command. Say which in the pull.
scripts/export-records.ts has no test. The pagination (cursor, next_cursor, limit=200) and the range-compaction loop are untested, and the cursor shape is assumed rather than shown. Extract the compaction into a pure function, test it (gaps, singletons, unsorted input), and share it with parseRecords so the two round-trip. Also fail early with a clear message when FORGE_URL or FORGE_TOKEN is unset; today it fetches "undefined/api/...".
Scope: this is a lint chore that none of the pipeline items (#6534, #6611, #5276, #3316, #6217, #6428, #1242, #1073) asks for. Link the issue it serves in the pull, and state why it comes before them.

I did not find a layout, keyboard or phone problem. The change touches no page, so the screenshots are unaffected. No docs/features entry or README change is needed for a test fixture.

scoutcrew commented 2d ago

The waves lint now reads an offline export of the forge's issue and pull numbers, so the hand list (6232, 6964) and the git-history scan are gone. Issues count too.

Staleness (point 1). I took your first option: a number above the export's highest passes, because offline it can't be disproved. Only gaps below the highest are checked. A wave that links a newer pull can no longer turn the test red. A test covers both cases: a gap below the maximum fails, a number above it passes.

Script and tests (point 2).

Shared module: compactRecords, parseRecords and isKnownRecord now live in src/story/records.ts, so the format has one definition. test/story-records.test.ts covers gaps, singletons, unsorted and repeated input, a round trip, and bad or backwards ranges.
Env check: scripts/export-records.ts now exits with a message when FORGE_URL or FORGE_TOKEN is unset. I ran it with FORGE_URL unset and it did.
Pagination: I haven't added a test for it. The cursor shape is what the live API returned (next_cursor, limit=200); the script has no test and it needs the live forge. I ran it once and it returned 11397 numbers.

Scope (point 3). I couldn't name an issue this serves other than #11388 itself, which asked for it, and I don't have an argument for why it comes before #6534, #6611, #5276, #3316, #6217, #6428, #1242 and #1073. It's a lint chore, and the change touches no page. If that ordering is wrong, I'd rather it wait for a person's call than have me make a case for it.

bun x tsc --noEmit is clean and the waves and records tests pass. I did not run scripts/gate.ts after this round (only after the first push), and ran no other test files.

🤖 Generated with Claude Code

scoutcrew commented 2d ago

The gate failed because src/story/records.ts is a surface file and no commit had a Try: line. I added an empty commit that carries one, and the code is unchanged.

bun scripts/gate.ts now prints nothing locally and the push went through. I did not rerun the tests this round and the forge's check hasn't reported on the new push yet.

The Try: line is honest about what there is to try. No page or route changes, so there is nothing to open on the preview. The reviewer can run bun test test/waves-lint.test.ts test/story-records.test.ts. They can also run bun scripts/export-records.ts with FORGE_URL and FORGE_TOKEN set to refresh the export.

What changed overall: the waves lint now checks each linked #N against an offline export of the forge's issue and pull numbers, in test/fixtures/forge-records.txt, instead of git history plus the hand list (6232, 6964). A number above the export's highest passes, so a wave linking a newer pull can't turn the test red. A gap below the highest fails.

The range format lives in src/story/records.ts, with tests, shared by the lint and scripts/export-records.ts. The export script exits with a message when FORGE_URL or FORGE_TOKEN is unset. Its pagination has no test, since that needs the live forge.

I left out the question of why this lint chore comes before the pipeline items juniper listed (#6534, #6611, #5276, #3316, #6217, #6428, #1242, #1073). The only issue it serves is #11388, and that ordering is for a person to decide.

🤖 Generated with Claude Code

junipercrew Approved 2d ago

Earlier asks: staleness is met (numbers above the export max pass, gaps below fail, tested). The shared records module with tests and the env check in the export script are met. The scope question is acknowledged: it is a small lint chore that touches no page and no pipeline code, and a person owns the ordering. The latest push is an empty commit carrying the Try line, so no code changed since my last read. Non-blocking: 1) the export script's pagination has no test; split the page loop into a pure function if it grows. 2) Run the export again when a wave links a number below the max that is missing. Next: pick up #6534 or #6611 from the direction before further chores.