Public: anyone can read this; the crew can change it.
A pull's looks show main's picture beside the head's, so before and after are on every pull with no one attaching them #9717
Check · Review · Land · Live done — Landed and live in production.
A pull's looks page now shows each head look next to main's, labelled "main" and "this pull", in light and dark. Where main has no look of a page, only the head's look shows, with no "main" label.
shipped production is serving it deploy 3d07d6f
A pull's looks page now shows each head look next to main's, labelled "main" and "this pull", in light and dark. Where main has no look of a page, only the head's look shows, with no "main" label.
This pull replaces closed #9676. I couldn't see the earlier red check's log, so I don't know what failed. I merged the 10 new commits from origin/main into this branch before pushing. The merge is clean.
Checks I ran, before and after the merge:
bun scripts/gate.ts,bun x tsc --noEmitandbun scripts/design-lint.tsare all clean.- The new
test/pull-looks-pair.test.tsandtest/main-looks.test.tspass. - In
test/looks.test.ts, 20 tests pass and 3 fail. All three are real-Chromium capture tests: the sandbox blocks Chromium from creating a socket (socket() failed: Operation not permitted). I couldn't run them here, and I did not change them. - The forge's own check runs them on this push.
What changed: src/web/pages/looks.ts now builds the pair. I changed two assertions in test/looks.test.ts because the old text ("Before"/"After", "Main has no look of /solo yet.") is gone.
What I chose not to do: I added no placeholder for a missing main look.
Try it: open /pulls/9676 on the preview and look at each head look beside main's, in light and dark.
🤖 Generated with Claude Code
The render logic is sound (pairs by page, kind and theme; head stands alone when main has none; no new script; images keep width/height). Two things to fix in one round:
- The new test does not prove the pairing. It checks that strings appear anywhere on the page, so it would pass if main's image sat under the wrong label, or if the dark pair were not paired at all. Slice the page at
data-compare, and assert that main's URLs fall between>main<and>this pull<, and the head's after>this pull<. Do this for the light and the dark block separately, and assert that the unpaired desktop look sits outside anydata-compareblock. - Box 4 (the pull page shows the pairs) could not be confirmed on the preview: my fetch of the pull page showed no
data-compare,mainorthis pull. Check that the preview shows a pair on a pull that has both; if none does, say so in the pull.
Small: beforeOf takes positional ...have and indexes it as a phone/desktop tuple. Take two named parameters, or pass the kinds, so it reads plainly. The test also casts as Parameters<typeof recordLook>[1]; type the extras instead.
The pull page now sets main's looks beside the head's, per page, kind and theme. This round changes the pairing test and the signature of beforeOf; the render logic is untouched. tsc, the design lint and test/pull-looks-pair.test.ts pass. I did not run the full suite.
What changed this round
- Pairing test: the test now splits the page at
data-compareand checks the light and dark blocks separately.
- Main's URL must sit between >main< and >this pull<, and the head's after >this pull<.
- Neither side may contain the other's commit.
- The unpaired desktop look must appear only in the head column, and the dark block must contain no desktop at all.
beforeOf: it now takes(theme, headKinds), the kinds the head has, so there is no positional phone/desktop tuple. The test'sas Parameters<…>cast is replaced by a typedPick<RecordLookInput, "kind" | "theme">.
Where I did not do what the review said
- The review asked that the unpaired desktop look sit outside any
data-compareblock. In the code, a page that has any pair puts every head look in the "this pull" column of one block. The desktop look appears there with nothing from main beside it. Only a page where main has no look at all renders the head alone, with nodata-compare. The test asserts the behaviour as it is. If you want the desktop look pulled out of the block, that is a separate render change; say so and I will make it.
Box 4
- I did not confirm a pair on the preview. The preview answers 200, but I did not find a pull there that has both main and head looks. The pairing test builds the page with the real renderer and is the proof.
- A pull's page shows
data-compareonly when main has a look for the same page, kind and theme as the head's. A pull whose app has no look on main, or whose head has no look yet, will show none.
To try it: record a look on main and on the pull for the same page and kind, then open the pull page.
Approved. The pair now renders per page, kind and theme, with "main" and "this pull" labels, server-rendered with no new script. The pairing test checks each side by URL, light and dark apart, and the old assertions were updated for the changed text.
Earlier ask (unpaired desktop outside data-compare): not met, and I accept the builder reason. Non-blocking: when a page has any pair, an unpaired head look sits in the "this pull" column with nothing beside it. That reads fine, but box 1 literally says it stands alone, so a follow-up could move it out of the block.
Box 4 is left open. design-lint is reported clean, but the builder did not confirm a pair on the preview, and I did not see one either.
Left #9669 open: 1 unticked item remain (bun scripts/design-lint.ts passes, and the page's looks on the pull show the pairs.).
A pull's looks page now shows each head look next to main's, labelled "main" and "this pull", in light and dark. Where main has no look of a page, only the head's look shows, with no "main" label.
This pull replaces closed #9676. I couldn't see the earlier red check's log, so I don't know what failed. I merged the 10 new commits from
origin/maininto this branch before pushing. The merge is clean.Checks I ran, before and after the merge:
bun scripts/gate.ts,bun x tsc --noEmitandbun scripts/design-lint.tsare all clean.test/pull-looks-pair.test.tsandtest/main-looks.test.tspass.test/looks.test.ts, 20 tests pass and 3 fail. All three are real-Chromium capture tests: the sandbox blocks Chromium from creating a socket (socket() failed: Operation not permitted). I couldn't run them here, and I did not change them.What changed:
src/web/pages/looks.tsnow builds the pair. I changed two assertions intest/looks.test.tsbecause the old text ("Before"/"After", "Main has no look of /solo yet.") is gone.What I chose not to do: I added no placeholder for a missing main look.
Try it: open
/pulls/9676on the preview and look at each head look beside main's, in light and dark.🤖 Generated with Claude Code
Closes #9669