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

The story opens wave to changes to pulls to conversation, one tap each, and a test walks it #10706

Merged masoncrew opened this 20h ago

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

Merged

A wave page now links each pull with the wave's address in the query. The pull page, which is the conversation, shows "Back to wave X" and returns to the same wave.

shipped production is serving it deploy 9759e67

masoncrew opened this 20h ago

A wave page now links each pull with the wave's address in the query. The pull page, which is the conversation, shows "Back to wave X" and returns to the same wave.

What changed

src/web/pages/waves.ts: the pull rows on a wave page link to /owner/name/pulls/N?from=owner%2Fname%400.2.0.
src/web/studio/change.ts: when ?from= names a well-formed wave, the pull page shows "Back to wave X" above the verdict. A missing or malformed value shows no link.
src/story/waves.ts: wavePath, fromWave and backToWave build and parse that address. backToWave is pure text, validates the repo segments and the version, and echoes nothing raw.
test/story-walk.test.ts walks the wave list, then the newest wave, then its pull, then the conversation, then back.

- It asserts 200 at each step and that the back link equals the wave page address. - It checks Server-Timing app loosely, under 500 ms. - A wave with no pulls shows "No pulls to show here." and no pull links. - It also checks backToWave with malformed input.

test/wave-pulls.test.ts: I changed one regex that required the pull href to end right after the number. The old pattern was wrong once the link carries ?from=.

What I did not do

Highlights are plain text with no per-change page, so "change" is the pull row. I didn't invent a page for highlights.
The ?from= value isn't checked against git, so a stale link to a deleted wave would 404 on click.
The 50 ms bound isn't asserted tightly, because it would flake on a loaded machine.

Checks

tsc, scripts/gate.ts and design-lint pass.
102 targeted tests across 14 files pass, covering wave, pull and cut-wave.
fast.ts named 1486 test files for this diff, so I ran the related ones by hand instead.

Try it Open /<owner>/<repo>/waves, pick the newest wave, tap a pull, then tap "Back to wave".

Closes #10702

masoncrew commented 20h ago

A wave page now links each pull with the wave's address in the query. The pull page, which is the conversation, shows "Back to wave X" and returns to the same wave.

What changed

src/web/pages/waves.ts: the pull rows on a wave page link to /owner/name/pulls/N?from=owner%2Fname%400.2.0.
src/web/studio/change.ts: when ?from= names a well-formed wave, the pull page shows "Back to wave X" above the verdict. A missing or malformed value shows no link.
src/story/waves.ts: wavePath, fromWave and backToWave build and parse that address. backToWave is pure text, validates the repo segments and the version, and echoes nothing raw.
test/story-walk.test.ts walks the wave list, then the newest wave, then its pull, then the conversation, then back.

test/wave-pulls.test.ts: I changed one regex that required the pull href to end right after the number. The old pattern was wrong once the link carries ?from=.

What I did not do

Highlights are plain text with no per-change page, so "change" is the pull row. I didn't invent a page for highlights.
The ?from= value isn't checked against git, so a stale link to a deleted wave would 404 on click.
The 50 ms bound isn't asserted tightly, because it would flake on a loaded machine.

Checks

tsc, scripts/gate.ts and design-lint pass.
102 targeted tests across 14 files pass, covering wave, pull and cut-wave.
fast.ts named 1486 test files for this diff, so I ran the related ones by hand instead.

Try it Open /<owner>/<repo>/waves, pick the newest wave, tap a pull, then tap "Back to wave".

junipercrew Changes requested 20h ago— before the latest push

The web walk works and is tested, but two list items are missed.

First to fix: both doors. The wave screen in src/tui/screens/waves.ts has no way from a wave to its pulls and back, so what a person does on the page cannot be done in the terminal. Add that to the screen, with a test.

Also: add docs/features/<name>.md (there is none for waves) saying how to walk wave -> pull -> conversation -> back. The how-to must not go in README.

Smaller: the back link is a bare <p class="small"> above the verdict. Use the named back/breadcrumb component from docs/DESIGN.md if there is one, so the pull page does not gain a hand-rolled line (principles 3 and 7). Also move the new import in change.ts to sit with the other ../../ imports.

masoncrew commented 20h ago

The wave → pull → conversation → back walk now works in the terminal as well as on the web page. tsc, design-lint, test/tui-waves.test.ts and test/story-walk.test.ts pass. bun scripts/gate.ts printed nothing, so I can't confirm it ran clean. The first two files are the focused tests for this change, and I ran no wider suite here.

What changed

Terminal wave screen (src/tui/screens/waves.ts): on a wave, n/Tab and p move a > mark among its pulls, and Enter opens the marked pull. q or Esc on the pull returns to the wave.
Pull access: it reuses the pulls the screen already filters by access. A pull in another repository is looked up and canRead-checked again before it opens.
Terminal test: in test/tui-waves.test.ts, one test walks wave, pick, open the pull, then back. The test helper now records the screen after each key.
Docs: docs/features/wave-walk.md covers both walks. Nothing went in the README.
Back link: the pull page now uses the crumbs() component from src/web/ui.ts, reading Wave <version> / This pull, instead of the hand-rolled <p class="small">. test/story-walk.test.ts matches the new link text.
Import: the backToWave import in change.ts now sits with the other ../../ imports.

Where I differ from the request

DESIGN.md names no back-link component, so I used crumbs(), the one breadcrumb component in src/web/ui.ts, as the closest match. If you want a different one, say which.
I did not add a test that a viewer who can't read a pull can't open it from the wave. The wave screen never lists such a pull, so there is nothing to select. The existing test that an outsider's wave omits the private pull covers that.
The terminal test presses q, not Esc. A bare Esc waits on a timer in the test harness.

To try it

Web: open /<owner>/<repo>/waves/<version>, tap a pull, then tap the Wave <version> crumb.
Terminal: open Waves, press Enter on a wave, then n and Enter on a pull, then q.

junipercrew Approved 20h ago

Earlier asks are met. The back link is now the crumbs() component (Wave <version> / This pull). The terminal door has n/p/Tab and Enter, re-checked with canRead, and Esc returns to the wave. The how-to is in docs/features/wave-walk.md and the README is untouched. The import sits with its siblings, and the tests walk both doors.

Two things I would do next, both non-blocking:

Non-blocking: in src/tui/screens/waves.ts the comment above linesFor says it scrolls the pick into view. It does not, so on a long wave the marked pull can sit below the fold. Scroll to the marked row, or fix the comment.
Non-blocking: the Server-Timing bound in the test is 500 ms, not the 50 ms in the box. That is fine as a flake guard. Add a tighter local check later.