style(fork): darken the workspace stage to #191919 - #25
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Review:
|
| colour | L* |
|---|---|
#161616 (upstream pre-paint) |
7.25 |
#191919 (new stage) |
8.76 |
#1e1e1e (panel) |
11.26 |
#212121 (old stage) |
12.74 |
#2d2e2e (border) |
18.84 |
1. The design principle in the manifest now contradicts the values it explains — blocking
Before:
the panel sits below the stage rather than lifting off it — the inverse of upstream's "surfaces lift from the base" model, and deliberate: the panel is chrome, the stage is the work.
After:
the panel sits slightly above the stage — deliberate: the panel is chrome, the stage is the work.
The clause after the dash was the reason for the inversion. It has been kept verbatim while the fact it justifies flipped 180°. The fork's surface palette now implements upstream's "surfaces lift from the base" model — the exact model the entry defined itself against — and offers, as its rationale, the sentence that was written to reject it.
.fork/AGENTS.md and #21 make manifest intents load-bearing: they are what a future rebase conflict is resolved against. An intent that asserts a relationship and then justifies it with the argument for the opposite relationship cannot arbitrate anything. Either:
- the principle genuinely changed (chrome now sits in front of the work) — then say that, and drop or rewrite the "panel is chrome" clause so it supports the new arrangement; or
- the principle stands and the stage should have gone lighter, not darker, to widen the panel-below-stage gap.
Right now the entry is unfalsifiable, which is the one thing a Tier-1 intent must not be.
2. The --surface-grain: none rationale is not stale — it is inverted — blocking
Both the CSS comment and the guard comment were edited to preserve sentence shape:
theme.custom.css:101—~#212121 — above the #191919 stage … the panel climbs past the stage and the three-level separation this whole palette is built on disappears.forkSurfacePalette.test.ts:104—from #1e1e1e to ~#212121 — past the #191919 stage
The original argument was precise and checkable: upstream's grain lifts the panel to exactly the stage's colour, so the separation collapses to zero. That is why the line existed. Against a #191919 stage that argument does not weaken, it reverses:
| panel L* | stage L* | ΔL* | contrast | |
|---|---|---|---|---|
| shipped (grain off) | 11.26 | 8.76 | 2.51 | 1.055 |
grain on (panel lifts to ~#212121) |
12.74 | 8.76 | 3.98 | 1.092 |
Upstream's grain would give the panel/stage boundary ~1.6× more separation than this PR ships. "The separation disappears" is now the opposite of true, and a future reader who trusts that comment will reach a wrong conclusion about a line the palette depends on.
To be clear: I would keep --surface-grain: none. The second argument in that comment survives intact — 0.008 opacity leaves no texture, only unspecified drift on every surface, and this palette is explicitly built on flat opaque sRGB rather than compounding alpha. That is now the whole reason. Rewrite the comment to say so and delete the collapse argument rather than rephrasing it, because the rephrasing is what makes it wrong.
Same defect one file over, forkSurfacePalette.test.ts:56–58:
If this block goes, the stage falls back near black and the
#1e1e1epanel reads as a lift off the base rather than as chrome against#191919.
The panel is now a lift off the base (11.26 > 8.76). The comment describes the intended state as the failure mode. A guard comment whose failure description matches the passing state teaches nothing.
3. Every guard stayed green while the palette's central relationship inverted — blocking
This is the finding I care about most, and it generalises past this PR.
The guards assert literal hex strings: toContain("--background: #191919"). So they pin today's constants and are silent on the invariant the intent declares. This PR flipped stage-vs-panel ordering — the one structural claim the entry makes — and the only thing that had to change to keep CI green was the string in the assertion. That is a guard measuring the diff against itself.
This is the same class of gap #13 closed for the mono stack ("editing one and forgetting the other left the safety net silently backing up a stack it no longer matched — with every existing guard still green"). The fix is the same shape: derive the assertion instead of duplicating the constant.
const hex = (block: string, prop: string) =>
/* parse `--prop: #rrggbb` out of block */;
const relLum = (hex: string) => /* sRGB → Y */;
it("keeps the panel and stage on the side of the relationship the intent names", () => {
const stage = hex(blockFor(theme, STAGE), "--background");
const panel = hex(blockFor(theme, PANEL), "--background");
// Intent: fork-surface-palette. Assert the ordering, not the constants —
// a value tweak is fine, silently inverting the hierarchy is not.
expect(relLum(panel)).toBeGreaterThan(relLum(stage));
// And that the step is actually perceptible on a large flat field.
expect(deltaLStar(panel, stage)).toBeGreaterThan(2);
});Whichever direction the answer to §1 is, encode that — then this PR either passes for a real reason or fails loudly and forces the conversation. As written, the guard file is a second copy of the CSS that must be edited in lockstep with it, which is precisely the drift #13 argued is worth a test.
Related, lower stakes: surfaceHexes (line 119) is a hand-maintained duplicate of the dark-block hexes, kept in sync by memory. Same argument — collect it from the parsed dark rules.
4. The v1 A/B got quieter, and the NOTE that worries about it was hex-substituted without re-checking
theme.custom.css:89–92 explicitly flags v1 as the case to watch: v1 keeps upstream's black panel "so the beta toggle stays a real A/B". Upstream's v1 dark panel is --background: #000 with --sidebar-border: rgb(255 255 255 / 8%) (index.css:975, 983). Darkening the stage moves the stage toward that black panel:
| v1 panel vs stage ΔL* | contrast | |
|---|---|---|
against #212121 |
12.74 | 1.304 |
against #191919 |
8.76 | 1.194 |
A 31% loss of the v1 panel's separation from the stage. The border erodes with it: 8% white composites to ~#333 over #212121 but only ~#2b2b2b over #191919. So the surface the NOTE is protecting reads measurably flatter after this change, and the NOTE — the one place in the file that would have caught it — was updated by replacing #212121 with #191919 in a sentence about how fine it is.
Not blocking; v1 is a deliberate A/B and 1.19:1 is still an edge. But it should be an acknowledged cost in the NOTE, not an invisible one.
5. This PR silently fixes a real seam nobody has written down — and leaves the coupling unguarded
apps/web/index.html (upstream-owned, not in the manifest) hardcodes #161616 in four places — both dark theme-color metas, DARK_BACKGROUND in the pre-hydration script, and html, body { background: #161616 }. That colour paints the pre-hydration flash, the overscroll rubber-band, and the mobile browser chrome band.
| stage | vs #161616 ΔL* |
contrast |
|---|---|---|
#212121 |
5.49 | 1.124 |
#191919 |
1.51 | 1.029 |
Darkening the stage cuts that mismatch by ~3.6× — the load flash and the overscroll band effectively stop being visible. That is a better argument for this PR than anything currently in its description, and it is nowhere in the PR, the intent, or the guards.
Which means: nothing ties #191919 to #161616, so the next stage adjustment reopens a seam that is now closed, with no test and no comment to notice. Either record the coupling in the intent (cheap), or add a guard asserting the stage stays within a few L* of the pre-paint colour (better, and it makes the win real rather than accidental).
6. #191919 has no cited source
Every other number in this block is justified: the grain lift is "measured, not assumed", #303030 is derived as rgba(255,255,255,0.08) over the panel, the pill geometry cites drawn values, #24 cites a Figma node id. #191919 arrives as a bare hex in a one-line summary. Given that it inverts the palette's stated hierarchy (§1), the provenance matters — design source, or eyeballed? If it is from Figma, cite the node; if it is a judgement call, say so in the notes file, which is where #21 put narrative.
7. Nit: the stage-level --border was not re-derived
The block's own comment says the frozen composites "are only correct against #1e1e1e anyway" — but --border: #2d2e2e is declared at stage scope too, and that is the one value in this diff now sitting on a different base than the one it was chosen against (ΔL* 6.10 → 10.08, 1.182 → 1.291). Almost certainly fine or better. Worth one sentence confirming it was looked at, since the file's standard is that these numbers are chosen, not inherited.
Verification
All four boxes in the test plan are unchecked, on a change whose entire content is "does this look right". AGENTS.md requires an integrated browser pass via test-t3-app for any user-visible frontend change; this repaints the global --background. A before/after screenshot pair — stage + v2 panel + v1 panel + light mode — is not ceremony here, it is the only test that can actually fail. The unit guard cannot: it asserts the string this PR wrote.
Also worth explicitly confirming, since the diff touches the global stage and nothing else: the terminal drawer derives its background from the computed --background (ThreadTerminalDrawer.tsx:140–151), so it follows automatically — but that is the kind of downstream consumer a colour-only PR should name rather than leave to the reviewer to find.
What I verified locally: re-ran the guard file's own blockFor regex against theme.custom.css standalone — the three assertions this PR touches do hold (#191919 present in the stage block, #1e1e1e in the panel block, --surface-grain: none intact). I could not execute the suite (vite-plus / @tailwindcss/vite unresolved in this environment), and at review time the Test, Check, and Mobile Native Static Analysis runs on 69b7cf4 were still in progress. Repo-wide grep confirms no #212121 references survive outside the two comments discussed in §2. All L* and contrast figures above are computed, not estimated.
Recommendation: hold. §1 needs a decision before the values are right or wrong, and §2 and §3 both need to change whichever way §1 lands. §4–§7 are follow-on once the intent says something checkable.
Generated by Claude Code
|
Addressed the hold items on §1 — Kept §2 — Kept §3 — Guard now asserts the ordering (panel relative luminance > stage, ΔL* > 2), not only the hex strings. §4 — NOTE acknowledges the narrower v1 A/B edge as an accepted cost. §5 — Intent records the §6 / §7 — Judgement-call note + one sentence confirming stage Local: |
|
Browser verification on the follow-up commit (dark + sidebar v2 enabled):
CI on |
Integrated visual verification (the outstanding hold item)Ran the mandated web pass via test-t3-app: isolated dev env (
This closes the Verification section of the hold review. |


Summary
#212121to#191919#1e1e1e(now slightly above the stage)Test plan
#191919#1e1e1evp test run apps/web/src/__fork_guards__/forkSurfacePalette.test.tsMade with Cursor