diff --git a/apps/desktop/layout-spec.json b/apps/desktop/layout-spec.json index dbca65cd..4da0b346 100644 --- a/apps/desktop/layout-spec.json +++ b/apps/desktop/layout-spec.json @@ -32,7 +32,7 @@ } }, "shell": { - "titlebarHeight": 48, + "titlebarHeight": 46, "regions": { "navigationRail": { "defaultWidth": 288, @@ -114,9 +114,9 @@ } }, "verticalRhythm": { - "titlebarHeight": 48, - "normalControlHeight": 28, - "fieldControlHeight": 32, + "titlebarHeight": 46, + "normalControlHeight": 32, + "fieldControlHeight": 36, "sectionGap": 16, "pageSectionGap": 32 }, diff --git a/apps/desktop/src/github/PullRequestsPage.tsx b/apps/desktop/src/github/PullRequestsPage.tsx index d4c0bf53..8f2ca0cf 100644 --- a/apps/desktop/src/github/PullRequestsPage.tsx +++ b/apps/desktop/src/github/PullRequestsPage.tsx @@ -7,6 +7,7 @@ import { } from "react"; import type { ReactNode } from "react"; +import { LoadFeedback } from "@/components/business/load-feedback"; import { MasterDetailRow } from "@/components/business/master-detail-row"; import { StatusBadge } from "@/components/business/status-badge"; import type { StatusTone } from "@/components/business/status-badge"; @@ -461,28 +462,17 @@ export function PullRequestsPage({
{loading && items.length === 0 ? ( -
- - {t("pullRequests.loading")} -
+ ) : error != null && error !== "" ? ( -
- -

{error}

- -
+ void reload()} + /> ) : groups.length === 0 ? ( @@ -682,32 +672,21 @@ export function PullRequestsPage({ {selected ? ( detailState?.loading === true && !detail ? ( -
- - {t("pullRequests.loadingDetail")} -
+ ) : detailState?.error != null && detailState.error !== "" ? ( -
- -

{detailState.error}

- -
+ { + const current = selected; + setSelectedId(null); + setTimeout(() => setSelectedId(current.id), 0); + }} + /> ) : detail ? (
diff --git a/apps/desktop/tests/designContract.test.ts b/apps/desktop/tests/designContract.test.ts new file mode 100644 index 00000000..82a02947 --- /dev/null +++ b/apps/desktop/tests/designContract.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; + +const normalizeSource = (source: string) => source.replaceAll(/\r\n?/gu, "\n"); +const readSource = (relativePath: string) => + normalizeSource( + readFileSync(new URL(relativePath, import.meta.url), "utf-8") + ); + +const layoutSpec = JSON.parse(readSource("../layout-spec.json")) as { + spacing: Record; + shell: { + titlebarHeight: number; + regions: Record< + string, + { defaultWidth: number; minWidth?: number; maxWidth?: number } + >; + }; + content: { + primaryColumn: { maxWidth: number }; + settings: { maxWidth: number }; + }; + verticalRhythm: Record; +}; +const tokens = readSource("../src/design/tokens.css"); +const styles = readSource("../src/styles.css"); +const app = readSource("../src/App.tsx"); +const pullRequests = readSource("../src/github/PullRequestsPage.tsx"); + +const declarations = new Map(); +for (const match of tokens.matchAll(/--([\w-]+):\s*([^;]+);/gu)) { + declarations.set(match[1], match[2].trim()); +} + +// Semantic roles alias foundations (`--ds-space-page: var(--ds-foundation-space-24)`), and the +// foundation names embed their size, so walk the chain and read the trailing number. +const resolvePx = (name: string): number | null => { + const value = declarations.get(name); + if (value == null) return null; + const px = /^(\d+)px$/u.exec(value); + if (px != null) return Number(px[1]); + const ref = /^var\(--([\w-]+)\)$/u.exec(value); + if (ref != null) return resolvePx(ref[1]); + const trailing = /-(\d+)$/u.exec(name); + return trailing == null ? null : Number(trailing[1]); +}; + +describe("design contract alignment", () => { + test("normalizes source contracts across platform line endings", () => { + expect(normalizeSource("first\r\nsecond\rthird")).toBe( + "first\nsecond\nthird" + ); + }); + + test("keeps the layout spec on the token vertical rhythm", () => { + expect(resolvePx("ds-titlebar-height")).toBe( + layoutSpec.verticalRhythm.titlebarHeight + ); + expect(resolvePx("ds-titlebar-height")).toBe( + layoutSpec.shell.titlebarHeight + ); + expect(resolvePx("ds-control-normal")).toBe( + layoutSpec.verticalRhythm.normalControlHeight + ); + expect(resolvePx("ds-control-field")).toBe( + layoutSpec.verticalRhythm.fieldControlHeight + ); + expect(styles).toContain("height: var(--ds-titlebar-height);"); + }); + + test("keeps the layout spec on the token spacing scale", () => { + for (const [name, value] of Object.entries(layoutSpec.spacing)) { + const camel = name.replace(/[A-Z]/gu, (c) => `-${c.toLowerCase()}`); + expect(resolvePx(`ds-space-${camel}`)).toBe(value); + } + }); + + test("keeps the shell defaults and clamps on the spec", () => { + const rail = layoutSpec.shell.regions.navigationRail; + const dock = layoutSpec.shell.regions.dock; + expect(app).toContain(`"codetwo.railWidth",\n ${rail.defaultWidth}`); + expect(app).toContain( + `Math.min(${rail.maxWidth}, Math.max(${rail.minWidth}, railWidth))` + ); + expect(app).toContain(`"codetwo.dockWidth",\n ${dock.defaultWidth}`); + }); + + test("keeps the content measure on the spec", () => { + const column = layoutSpec.content.primaryColumn.maxWidth; + expect(column).toBe(768); + expect(layoutSpec.content.settings.maxWidth).toBe(column); + expect(column / 16).toBe(48); + }); + + test("routes the pull-request blocking states through LoadFeedback", () => { + expect(pullRequests).toContain( + 'import { LoadFeedback } from "@/components/business/load-feedback";' + ); + expect(pullRequests.match(/'); + expect(pullRequests).not.toContain( + 'role="status"\n className="text-body text-muted-foreground flex items-center justify-center gap-2 py-12"' + ); + }); +}); diff --git a/docs/design/system.md b/docs/design/system.md index bfcdf2fd..b4be9cdf 100644 --- a/docs/design/system.md +++ b/docs/design/system.md @@ -178,7 +178,7 @@ The shared business set is: | module | owns | current callers | | --- | --- | --- | -| `PageHeader` | page heading hierarchy, description measure, responsive action placement | Automations, Plugin Hub, Scene Studio, Task Board | +| `PageHeader` | page heading hierarchy, description measure, responsive action placement | Automations, Plugin Hub, Scene Studio, Task Board, settings pages | | `SearchField` | labelled search input, icon geometry, optional accessible clear action | Automations, Docker, Task Board, Plugin Manager, Memory, Trajectory | | `Empty` primitive | empty-state hierarchy, media, description, and action composition | Automations, Pull Requests | | `SelectableRow` | compact picker choice, visible selection mark, accessible selected/disabled state, description and metadata layout | Composer mode, memory, collaboration, worktree, provider, and model pickers; Scene picker; Checkout picker | @@ -191,6 +191,15 @@ The shared business set is: | `StatusIndicator` | semantic dot-and-label status with theme-managed tones | Docker, Device connections | | `SettingToggle` | visible label and description association, immediate boolean control, disabled presentation, and row layout | Project actions, Memory, Sync, Project scheduling, Appshots, Pets | +The settings cohort composes those primitives once, in `src/settings/SettingsPrimitives.tsx`: `Page` +wraps `SettingsPanel`/`PageHeader`, `Row` and `ProjectRow` wrap `SettingRow` (the project variant +adding the shared trailing control lane), and `GroupHeading` owns the 14px/600 group label used by +the pages whose sections already own their spacing. Settings pages consume that module instead of +re-deriving the anatomy; the module is cohort-local rather than a business primitive, and it is +subject to the same lint restrictions as every other product file. Current callers: General, +Import, Keybindings, Project, Worktrees, Memory, Providers, Appshots, and the Appearance, Pets and +Sync pages that are wrapped through `SettingsPage`. + `SelectableRow` is deliberately limited to persistent selection inside compact pickers. Radio or checkbox questions use `ChoiceRow`; navigation/current-page rows, disclosure rows, and master-detail list rows keep their own interaction contracts. `StatusBadge` is limited to labelled pills; dot-and-label status diff --git a/docs/sdlc/changes/2026-09-16-align-design-contracts/intent.md b/docs/sdlc/changes/2026-09-16-align-design-contracts/intent.md new file mode 100644 index 00000000..39753258 --- /dev/null +++ b/docs/sdlc/changes/2026-09-16-align-design-contracts/intent.md @@ -0,0 +1,57 @@ +--- +id: 2026-09-16-align-design-contracts +schema: 5 +stage: intent +status: accepted +owner: chenli +created: 2026-09-16 +source: user +risk: medium +approved_by: chenli +approved_at: 2026-09-16 +approval_source: "Direct request: 进行治理, after the audit list of governance findings in this session." +next_trigger: chenli reviews the verified work. +--- + +# Intent: Align the design contracts with the code + +## Intent + +The governance item of the agreed repair order: make the design system's machine-readable contracts +describe what the code actually does, register the shared module the doc omits, and give the loading +contract real callers. + +Findings, re-verified against the live checkout: + +1. `apps/desktop/layout-spec.json` disagrees with the token sheet on three vertical-rhythm numbers + (`titlebarHeight` 48 versus the 46px `--ds-titlebar-height`, `normalControlHeight` 28 versus the + 32px `--ds-control-normal`, `fieldControlHeight` 32 versus the 36px `--ds-control-field`) and the + stale titlebar value also appears under `shell`. Nothing fails when the two drift apart: the file + is read only by tests. +2. `apps/desktop/src/settings/SettingsPrimitives.tsx` is the settings-scoped shared layer (Page, Row, + ProjectRow, GroupHeading) used by six settings pages, but `docs/design/system.md` never registers + it and the `PageHeader` caller list omits every settings page, so the doc's component map and the + code disagree. +3. `LoadFeedback` — the doc's owner for content-blocking loading and recoverable failures — has no + product caller; only the development preview renders it. `github/PullRequestsPage.tsx` hand-rolls + the identical shape four times (list and detail, loading and failure). +4. Correction to this session's earlier audit: `ControlChip` is **not** a dead contract — the + composer and `SceneChip` import it (aliased as `Chip`), with nine call sites. The earlier claim + came from grepping the literal `` call sites and no surviving hand-rolled `role="status"` block in `github/PullRequestsPage.tsx`. Rendered from the UI Lab pull-request scenario: with the lab fixture temporarily rejecting, the failure state rendered through `[data-slot="load-feedback"][data-state="error"]` with `role="alert"`, the message and the Retry button (capture `browser-artifacts/browser-screenshot-localhost-mu3vn3j9-73e9bbe1.png`); with it pending, the loading state rendered as `[data-state="loading"]`/`role="status"` with `Loading pull requests…`; after restoring the fixture byte-identically the ready state renders the rows again with no LoadFeedback node. +- AC-5: PASS — From `apps/desktop`: `bun run lint`, `bunx tsc --noEmit` (inside `build:renderer`), `bun test` (963 pass, 3 skip, 0 fail) and `bun run build:renderer`. + +Verdict: verified. +Residual risk: the loading and failure captures required a temporarily patched lab fixture (restored byte-identical, `git status` clean for that file); the detail pane's own two states need a selected row and are covered structurally. The design doc's enforcement section still describes the lint scope loosely and was left as-is. The audit's false `ControlChip` claim is corrected in this record's Intent instead of the earlier report. + +## Cleanup + +Removed: the temporary lab-fixture patch (restored from the backup and diffed byte-identical), the +task-owned Vite dev server on port 1420 and its log at +`/var/folders/nl/47s4vtc92m74_j8pmm7d0chh0000gn/T/opencode/renderer-1420.log`, and the ignored +`apps/desktop/dist/` build output. +Retained: the failure-state screenshot in `/Users/chenli/.t3/userdata/browser-artifacts/` as AC-4 +evidence. +Retention owner: chenli. +Cleanup trigger: remove the screenshot with the next browser-artifact cleanup after review. +Processes: the task-owned Vite server was stopped; `lsof -nP -iTCP:1420 -sTCP:LISTEN` and +`pgrep -fl "vite --port 1420"` match nothing. The user's dev window was left running. +Evidence: `git status --porcelain` lists only the four intended files plus this record bundle. + +## Review and release + +Approval: governance work was requested directly by the user (进行治理); merge and release are not +authorized. +Rollback: See plan.md. +Release: No release requested; merge and external actions require their own authorization. +Feedback: Link an Incident and regression Eval when a real failure occurs.