From 7205cc31d48adc9301bd729b4b66289af6697f42 Mon Sep 17 00:00:00 2001 From: Leeeon233 Date: Thu, 3 Sep 2026 09:42:54 +0000 Subject: [PATCH 1/4] feat(components): gate the geometry ledger baseline in CI A ledger baseline was a number the report PRINTED and nothing executed: `pnpm test:geometry` only checked that the two promoted contracts still compiled and held, so an `accepted-debt` finding could drift arbitrarily far from its reviewed offset, and a brand new finding could appear, with every check green. The ledger's 74 debt entries constrained nothing. Extract the capture walk the report already performs into `tests/e2e/support/geometry-capture-plan.ts`: `GEOMETRY_CAPTURE_PLAN` is now the ONE list of captures, `showGeometryCaptureStory` the one way to open and settle a story, and `buildGeometryCapture` the one way to turn discovery into a capture record. The report walks it to shoot its screenshots; the gate walks it to measure and nothing else. Neither can name a story, a viewport, a device scale or an aggregate scope the other does not. Add the ratchet to the gate: rerun `capture -> observation -> findings` over that plan with no screenshots, then fail when - a finding's |offset| passes |baseline| plus one device pixel (1/DPR of the coarsest capture it merged), or - a finding has no ledger entry at all, pointing at `geometry:triage` the way a lockfile check points at an install. `ignored` opts out and `promoted` is left to the contract check that already gates it exactly. `checkGeometryLedgerRatchet` and `geometryFindingDevicePixel` are pure and unit-tested; the failure message carries key, label, status, baseline and current offset. A finding offset is a mean over the whole plan, so a baseline belongs to the platform that recorded it. These were recorded on macOS and the gate runs on Linux, so 12 of the 27 findings this plan reproduces here are re-baselined to the Linux rendering in this commit; the 49 entries that no longer reproduce keep their reviews untouched. Verified: `tsgo --noEmit`, unit tests (106 passed), `pnpm test:geometry` (10 passed, 7.2m; the ratchet adds ~4.6m over the previous 2.6m gate). Model: claude-opus-5[1m] --- packages/components/geometry-ledger.json | 24 +- .../src/lib/geometry-constraint-system.ts | 110 +++ packages/components/tests/e2e/AGENTS.md | 49 +- .../chat-workspace-geometry-report.spec.ts | 828 ++++-------------- .../tests/e2e/chat-workspace-geometry.spec.ts | 68 +- .../components/tests/e2e/support/AGENTS.md | 43 +- .../e2e/support/geometry-capture-plan.ts | 585 +++++++++++++ .../tests/geometry-constraint-system.test.ts | 173 ++++ 8 files changed, 1167 insertions(+), 713 deletions(-) create mode 100644 packages/components/tests/e2e/support/geometry-capture-plan.ts diff --git a/packages/components/geometry-ledger.json b/packages/components/geometry-ledger.json index 35e2e3214..3c668169d 100644 --- a/packages/components/geometry-ledger.json +++ b/packages/components/geometry-ledger.json @@ -47,7 +47,7 @@ "geometry/right-sidebar/1e5wvsl": { "status": "accepted-debt", "baseline": { - "offset": 4 + "offset": 6.5 }, "identity": { "label": "Files", @@ -71,7 +71,7 @@ "geometry/right-sidebar/1mxwyl5": { "status": "accepted-debt", "baseline": { - "offset": -1 + "offset": -1.5 }, "identity": { "label": "app.tsx", @@ -131,7 +131,7 @@ "geometry/right-sidebar/6ijj9r": { "status": "accepted-debt", "baseline": { - "offset": 0.75 + "offset": 1 }, "identity": { "label": "Docs 2 + 100 − 20", @@ -167,7 +167,7 @@ "geometry/right-sidebar/b0m4s": { "status": "accepted-debt", "baseline": { - "offset": 6.5 + "offset": 4 }, "identity": { "label": "Files", @@ -215,7 +215,7 @@ "geometry/right-sidebar/daf70p": { "status": "accepted-debt", "baseline": { - "offset": -1 + "offset": -1.5 }, "identity": { "label": "Conversation Diff", @@ -383,7 +383,7 @@ "geometry/session/1t257v2": { "status": "accepted-debt", "baseline": { - "offset": -1.25 + "offset": -1 }, "identity": { "label": "Private to you: lody is not shared with the team.", @@ -575,7 +575,7 @@ "geometry/workspace/1fccih": { "status": "accepted-debt", "baseline": { - "offset": 1.75 + "offset": -4 }, "identity": { "label": "Machine", @@ -587,7 +587,7 @@ "geometry/workspace/1jrw0q7": { "status": "accepted-debt", "baseline": { - "offset": -0.75 + "offset": -1 }, "identity": { "label": "More actions", @@ -647,7 +647,7 @@ "geometry/workspace/1t2nsoi": { "status": "accepted-debt", "baseline": { - "offset": -0.8181818181818182 + "offset": -1 }, "identity": { "label": "More actions", @@ -767,7 +767,7 @@ "geometry/workspace/cqs9ix": { "status": "accepted-debt", "baseline": { - "offset": -0.8181818181818182 + "offset": -1 }, "identity": { "label": "More actions", @@ -827,7 +827,7 @@ "geometry/workspace/ky1uql": { "status": "accepted-debt", "baseline": { - "offset": 5.269230769230769 + "offset": 12.5 }, "identity": { "label": "Message", @@ -960,7 +960,7 @@ "geometry/workspace/w5fbub": { "status": "accepted-debt", "baseline": { - "offset": -4.076923076923077 + "offset": -4 }, "identity": { "label": "Machine", diff --git a/packages/components/src/lib/geometry-constraint-system.ts b/packages/components/src/lib/geometry-constraint-system.ts index c08e5e02e..1c301e688 100644 --- a/packages/components/src/lib/geometry-constraint-system.ts +++ b/packages/components/src/lib/geometry-constraint-system.ts @@ -380,6 +380,16 @@ export type GeometryFindingArtifact = Readonly<{ export type GeometryLedgerStatus = 'new' | 'accepted-debt' | 'ignored' | 'promoted'; +/** + * Statuses the ratchet holds to their reviewed baseline. `ignored` opts out — + * that is what ignoring one means — and `promoted` is left to the contract + * check that already gates it exactly, rather than gated twice and loosely. + */ +export const GEOMETRY_RATCHETED_LEDGER_STATUSES: readonly GeometryLedgerStatus[] = [ + 'new', + 'accepted-debt', +]; + /** * The stylesheet is the single source of truth for a named geometry token. A * ledger entry only says WHICH custom property carries it; `expected` is @@ -2826,6 +2836,106 @@ export function summarizeGeometryInkCenters( }; } +export type GeometryRatchetViolation = Readonly<{ + kind: 'offset-regression' | 'unreviewed-finding'; + key: string; + label: string; + surfaceFamily: GeometrySurfaceFamily; + axis: SemanticAlignmentAxis; + anchor: SemanticAlignmentAnchor; + status?: GeometryLedgerStatus; + baseline?: number; + current: number; + /** The slack allowed above `|baseline|`, in CSS pixels. */ + tolerance?: number; +}>; + +/** + * The coarsest device pixel this finding was measured with. A finding merged + * across a 2× and a 1× capture is only as precise as the 1× one, so holding it + * to half a pixel would fail on rounding the measurement cannot avoid. + */ +export function geometryFindingDevicePixel( + finding: GeometryFinding, + captures: GeometryCaptureArtifact +): number { + const scaleByCapture = new Map( + captures.captures.map((capture) => [capture.captureId, capture.deviceScaleFactor]) + ); + const devicePixels = finding.evidence.map((evidence) => { + const scale = scaleByCapture.get(evidence.captureId); + return scale && scale > 0 ? 1 / scale : 1; + }); + return devicePixels.length > 0 ? Math.max(...devicePixels) : 1; +} + +/** + * The ratchet. A ledger baseline used to be a number the report PRINTED; this + * is what makes it a number CI enforces. + * + * Two rules, both about the ledger being complete and monotonic: + * every measured finding must be reviewed, and no reviewed finding may drift + * further from its line than the review recorded, allowing one device pixel for + * the rounding the measurement itself cannot avoid. `ignored` opts out — that + * is what ignoring one means — and `promoted` is left to the contract check + * that already gates it exactly, rather than being gated twice and loosely. + */ +export function checkGeometryLedgerRatchet( + artifact: GeometryFindingArtifact, + ledger: GeometryLedger, + captures: GeometryCaptureArtifact +): readonly GeometryRatchetViolation[] { + const ratcheted = new Set(GEOMETRY_RATCHETED_LEDGER_STATUSES); + return artifact.findings.flatMap((finding): GeometryRatchetViolation[] => { + const identity = { + key: finding.key, + label: finding.label, + surfaceFamily: finding.surfaceFamily, + axis: finding.axis, + anchor: finding.anchor, + current: finding.offset, + }; + const entry = ledger.findings[finding.key]; + if (!entry) return [{ kind: 'unreviewed-finding', ...identity }]; + if (!ratcheted.has(entry.status)) return []; + const baseline = entry.baseline?.offset; + if (baseline === undefined) return []; + const tolerance = geometryFindingDevicePixel(finding, captures); + if (Math.abs(finding.offset) <= Math.abs(baseline) + tolerance) return []; + return [ + { + kind: 'offset-regression', + ...identity, + status: entry.status, + baseline, + tolerance, + }, + ]; + }); +} + +export function formatGeometryRatchetViolations( + violations: readonly GeometryRatchetViolation[] +): string { + return violations + .map((violation) => + violation.kind === 'unreviewed-finding' + ? [ + `unreviewed finding ${violation.key}`, + ` ${violation.surfaceFamily} · ${violation.label} · ${violation.axis}/${violation.anchor}`, + ` measured ${violation.current.toFixed(3)}px and no ledger entry reviews it.`, + ' Run `pnpm geometry:report ` then `pnpm geometry:triage ` and commit the ledger.', + ].join('\n') + : [ + `regressed finding ${violation.key}`, + ` ${violation.surfaceFamily} · ${violation.label} · ${violation.axis}/${violation.anchor} (${violation.status})`, + ` baseline ${(violation.baseline ?? 0).toFixed(3)}px → current ${violation.current.toFixed(3)}px`, + ` allowed |offset| ≤ ${(Math.abs(violation.baseline ?? 0) + (violation.tolerance ?? 0)).toFixed(3)}px (baseline + ${(violation.tolerance ?? 0).toFixed(3)}px device pixel)`, + ].join('\n') + ) + .join('\n\n'); +} + export function compileGeometryContracts(ledger: GeometryLedger): GeometryContractArtifact { const contracts = Object.entries(ledger.findings).flatMap(([findingKey, entry]) => { if (entry.status !== 'promoted') return []; diff --git a/packages/components/tests/e2e/AGENTS.md b/packages/components/tests/e2e/AGENTS.md index f04e26221..20f78cd0b 100644 --- a/packages/components/tests/e2e/AGENTS.md +++ b/packages/components/tests/e2e/AGENTS.md @@ -6,10 +6,10 @@ Package `AGENTS.md` and the repository root also apply. Measures rendered geometry, turns it into findings, gates what a human promoted. `src/lib/chat-workspace-geometry.ts` (spec, grid, discovery) and `geometry-constraint-system.ts` (pipeline, ledger, contracts, tokens, metrics); grid and -classification: [src/lib](../../src/lib/AGENTS.md); what a primitive, a row and a name ARE: -[support](support/AGENTS.md); `*-geometry-report.spec.ts` (report); `*-geometry.spec.ts` -(gate). Neither `geometry:report [dir]` nor root `pnpm geometry:triage ` moves a -baseline. +classification: [src/lib](../../src/lib/AGENTS.md); what a primitive, a row and a name ARE, +and the shared capture plan: [support](support/AGENTS.md); `*-geometry-report.spec.ts` +(report), `*-geometry.spec.ts` (gate). Neither `geometry:report` nor `geometry:triage` +moves a baseline. ## X rails @@ -48,9 +48,9 @@ Marker-free, same pipeline, over the anchors [support](support/AGENTS.md) lists. - Exactly two members is one `row-spread` naming both, never two outliers at half the gap: their median is their midpoint, so a signed offset would invent a direction. Three or more have a majority, so their median is a line. -- `y-axis-parity.json` and `marker-removal-readiness.json` ask whether discovery has - replaced the markers ([support](support/AGENTS.md)). The gate proves outlier reporting - with its OWN injected `translateY`, diffing before and after, asserting no product row. +- `marker-removal-readiness.json` asks whether discovery has replaced the markers + ([support](support/AGENTS.md)). The gate proves outlier reporting with its OWN injected + `translateY`, diffing before and after, asserting no product row. ## Pipeline and finding identity @@ -86,13 +86,18 @@ beside the code ([src/lib](../../src/lib/AGENTS.md)). Review lives in checked-in `geometry-ledger.json`; `geometry-contracts.json` compiles only `promoted` entries, each declaring `ink` or `layout-box`. +- A baseline is EXECUTED, not printed: the gate reruns the pipeline over the whole capture + plan with no screenshots, and fails when a finding's |offset| passes |baseline| plus one + device pixel (1/DPR of its COARSEST capture) or when a finding has no ledger entry + (`geometry:triage`, like a lockfile). `ignored` opts out; `promoted` belongs to the + contract check. Offsets are means over the WHOLE plan, so a baseline belongs to the + platform that recorded it: re-baseline where CI runs, never trim the plan for speed. - Two contract members never cover one element twice; member resolution, the ink witness and named tokens (the ledger records only the `--spacing-*` property): see - [support](support/AGENTS.md). -- Relations are a small deterministic algebra ([support](support/AGENTS.md)). Widen the - relation before loosening a tolerance. -- Ledger labels give discovery precision, promoted locators geometry coverage, PNG edge - sampling only confidence. + [support](support/AGENTS.md). Relations are a small deterministic algebra there; widen + the relation before loosening a tolerance. +- Ledger labels give discovery precision, promoted locators coverage, PNG edges only + confidence. ## Report @@ -101,18 +106,14 @@ Discovery or proposal presence is never a report assertion; coverage: - Each detail persists the capture id owning its Story, viewport and scale; `--after` replays that capture and clip and appends only the repair image, never rediscovering - findings or replacing evidence. Replay and Y cards: [support](support/AGENTS.md). -- Steady state, not delta: every finding gets a card grouped by ledger status (`new`, - `changed`, `accepted-debt`, `promoted`, `ignored`) and classification, with baseline vs - current offset, capture count, dimension sensitivity and repair text. Chips filter both, - default new + changed + css-defect + promoted; the meta line prints the total beside - new/changed/resolved. One embedded JSON payload, one renderer, images as files. + findings or replacing evidence. Replay: [support](support/AGENTS.md). +- Steady state, not delta: every finding gets a card grouped by ledger status and + classification, with baseline vs current offset, capture count, dimension sensitivity and + repair text. Chips filter both, default new + changed + css-defect + promoted; the meta + line totals new/changed/resolved. One JSON payload, one renderer, images as files. - Violation images label each deviating member in place with role, physical direction, measured offset, actual anchor and a leader to it. A Y card comes from the FINISHED findings, never a second pipeline printing another number: each annotation IS that - finding's evidence for that member, asserted before the shot; its clip holds the whole - row plus a margin; it draws the row median and the verdict anchor only, and names the row. - Zoomed Y cards are the largest-|offset| findings over every surface. Discovery cards use - product-region names, count unique elements not anchor votes, fold one element's - start/center/end offsets into one annotation, and keep candidate rails under emphasised - outliers. Cards are picked by deviation, inside the generator's budget. + finding's evidence for that member, asserted before the shot. Card clips, zoomed Y cards + and discovery cards: [support](support/AGENTS.md). Cards are picked by deviation, inside + the generator's budget. diff --git a/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts b/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts index 8550fb3be..e18dba286 100644 --- a/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts +++ b/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts @@ -38,10 +38,7 @@ import { type GeometryRepairProposal, } from '../../src/lib/geometry-constraint-system'; import { - auditChatWorkspaceSemanticAlignments, - auditChatWorkspaceSemanticBaselines, auditChatWorkspaceSpacing, - discoverChatWorkspaceAlignmentRails, measureGeometryContractOpticalInsets, type BrowserAlignmentRailDiscoveryScope, type BrowserSemanticAlignmentEntry, @@ -49,6 +46,23 @@ import { measureSettledChatWorkspace, requireGeometryRect, } from './support/chat-workspace-geometry'; +import { + DEFAULT_CAPTURE_DIMENSIONS, + GEOMETRY_REPRESENTATIVE_CAPTURE, + GEOMETRY_RIGHT_SIDEBAR_STATE_CAPTURES, + GEOMETRY_SESSION_STATE_CAPTURES, + GEOMETRY_WORKSPACE_DIMENSION_CAPTURES, + GEOMETRY_WORKSPACE_MATRIX_CAPTURES, + GEOMETRY_WORKSPACE_STATE_CAPTURES, + buildGeometryCapture, + enableGeometryCaptureMode, + geometryReplayContextKey, + observeGeometryPlanEntry, + openGeometryReplayContext, + showGeometryCaptureStory, + type GeometryCapturePlanEntry, + type GeometryPlanObservation, +} from './support/geometry-capture-plan'; const outputDirectory = process.env.GEOMETRY_REPORT_OUTPUT_DIR; const storybookOrigin = process.env.PLAYWRIGHT_BASE_URL ?? 'http://127.0.0.1:6006'; @@ -191,7 +205,7 @@ async function collectGeometryPixelWitnesses( const storyUrl = `${storybookOrigin}/iframe.html?id=${contract.story}&viewMode=story`; const storyResponse = await witnessPage.goto(storyUrl); if (!storyResponse?.ok()) throw new Error(`Witness story failed: ${contract.story}`); - await enableReportCaptureMode(witnessPage); + await enableGeometryCaptureMode(witnessPage); if (contract.story.startsWith('geometry-chatworkspace--')) { await measureSettledChatWorkspace(witnessPage); } @@ -394,115 +408,6 @@ const DISCOVERY_SURFACE_LABELS: Readonly> = { 'Workspace / Chat Landing / Submitting': 'Workspace / Chat Landing 提交中', }; -type GeometryCaptureDimensions = Readonly<{ theme: string; locale: string; density: string }>; - -/** Storybook's own globals; there is no density global, so it stays constant. */ -const DEFAULT_CAPTURE_DIMENSIONS: GeometryCaptureDimensions = { - theme: 'light', - locale: 'en', - density: 'default', -}; - -/** - * Dimensions vary ONE capture family rather than every story: they exist to show - * whether a finding survives a theme or a locale, and a full matrix would - * multiply the report runtime for evidence nobody reads. - */ -const WORKSPACE_DIMENSION_CAPTURES = [ - { - id: 'wide-expanded-dark', - surface: 'Workspace / Chat Landing / Dark', - globals: 'theme:dark', - colorScheme: 'dark' as const, - dimensions: { ...DEFAULT_CAPTURE_DIMENSIONS, theme: 'dark' }, - }, - { - id: 'wide-expanded-zh', - surface: 'Workspace / Chat Landing / 中文', - globals: 'locale:zh_CN', - colorScheme: 'light' as const, - dimensions: { ...DEFAULT_CAPTURE_DIMENSIONS, locale: 'zh_CN' }, - // A zh_CN capture that still rendered English strings would silently claim a - // locale axis it never varied, so the capture asserts a translated label. - // The Sidebar's Chats section header is the one this fixture translates. - expectedText: '对话', - }, -] as const; - -const WORKSPACE_STATE_CAPTURES = [ - { - id: 'landing-submitting', - surface: 'Workspace / Chat Landing / Submitting', - storyId: 'geometry-chatworkspace--submission-pending', - }, - { - id: 'landing-no-machine-download', - surface: 'Workspace / Chat Landing / No Machine Download', - storyId: 'geometry-chatworkspace--no-machine-download', - }, - { - id: 'landing-no-machine-starting', - surface: 'Workspace / Chat Landing / No Machine Starting', - storyId: 'geometry-chatworkspace--no-machine-starting', - }, - { - id: 'landing-no-agent', - surface: 'Workspace / Chat Landing / No Agent', - storyId: 'geometry-chatworkspace--no-agent-config', - }, - { - id: 'landing-long-model', - surface: 'Workspace / Chat Landing / Long Model', - storyId: 'geometry-chatworkspace--long-model', - }, - { - id: 'landing-pasted-text', - surface: 'Workspace / Chat Landing / Pasted Text', - storyId: 'geometry-chatworkspace--pasted-text', - }, -] as const; - -const SESSION_STATE_CAPTURES = [ - { - id: 'session-idle', - surface: 'Chat Session / Idle', - storyId: 'sessions-sessionconversationpage--desktop-idle', - }, - { - id: 'session-working', - surface: 'Chat Session / Working', - storyId: 'sessions-sessionconversationpage--desktop-working-settled', - }, - { - id: 'session-permission', - surface: 'Chat Session / Permission', - storyId: 'sessions-sessionconversationpage--desktop-permission-approval', - }, - { - id: 'session-question', - surface: 'Chat Session / Agent Question', - storyId: 'sessions-sessionconversationpage--desktop-agent-question', - }, -] as const; - -const RIGHT_SIDEBAR_STATE_CAPTURES = [ - { - id: 'session-right-sidebar-changes', - surface: 'Chat Session / Right Sidebar / Changes', - storyId: 'sessions-sessionsidepaneltabbar--geometry-report', - }, - { - id: 'session-right-sidebar-tabs', - surface: 'Chat Session / Right Sidebar / Tabs', - storyId: 'sessions-sessionsidepaneltabbar--unified-tabs', - }, - { - id: 'session-right-sidebar-empty', - surface: 'Chat Session / Right Sidebar / Empty', - storyId: 'sessions-sessionsidepaneltabbar--empty-state', - }, -] as const; - const GEOMETRY_COVERAGE_EXCLUSIONS = [ { surface: 'Chat Session / Mention Drop', @@ -1525,173 +1430,6 @@ function createDiscoveryOverviewDetail({ }; } -async function waitForSessionConversationStory(page: Page): Promise { - await page.locator('[data-testid="session-conversation-story"]').waitFor({ - state: 'visible', - timeout: 90_000, - }); - await page.evaluate(async () => { - await document.fonts.ready; - await new Promise((resolve) => { - requestAnimationFrame(() => requestAnimationFrame(() => resolve())); - }); - }); -} - -async function enableReportCaptureMode(page: Page): Promise { - // Generous on purpose: the FIRST story in a cold browser context compiles and - // parses the whole Storybook bundle, which is a minute on a loaded machine - // and 4 seconds once warm. This is an explicit readiness signal, never a - // sleep — the deadline only decides how loaded a machine may be. - await page - .locator('[data-geometry-fixture-ready="true"], [data-testid="session-conversation-story"]') - .first() - .waitFor({ state: 'attached', timeout: 180_000 }); - await page.addStyleTag({ - content: ` - [data-geometry-report-capture="true"] * { - pointer-events: none !important; - animation: none !important; - transition: none !important; - } - [data-geometry-actions-visible="true"] [data-geometry-hover-action] { - opacity: 1 !important; - pointer-events: none !important; - } - [data-geometry-actions-visible="true"] [data-geometry-hover-rest] { - opacity: 0 !important; - pointer-events: none !important; - } - [data-geometry-report-capture="true"] [data-geometry-capture-reveal="true"] { - opacity: 1 !important; - pointer-events: none !important; - } - `, - }); - const workspace = page.locator( - `[${CHAT_WORKSPACE_GEOMETRY_ATTRIBUTE}="${CHAT_WORKSPACE_GEOMETRY_ANCHORS.workspaceShell}"]` - ); - await page.locator('body').evaluate((element) => { - element.setAttribute('data-geometry-report-capture', 'true'); - element.setAttribute('data-geometry-actions-visible', 'true'); - - const interactiveSelector = - 'button, [role="button"], a[href], input, select, textarea, summary, [tabindex]:not([tabindex="-1"])'; - for (const candidate of element.querySelectorAll('*')) { - if (candidate.closest('[data-geometry-hover-rest]')) continue; - const style = getComputedStyle(candidate); - const rect = candidate.getBoundingClientRect(); - const occupiesViewport = - rect.width > 0 && - rect.height > 0 && - rect.right >= 0 && - rect.bottom >= 0 && - rect.left <= innerWidth && - rect.top <= innerHeight; - const ownsInteraction = - candidate.matches(interactiveSelector) || candidate.querySelector(interactiveSelector); - if ( - Number.parseFloat(style.opacity) <= 0.01 && - style.display !== 'none' && - style.visibility !== 'hidden' && - occupiesViewport && - ownsInteraction - ) { - candidate.setAttribute('data-geometry-capture-reveal', 'true'); - } - } - }); - if ((await workspace.count()) > 0) { - await workspace.evaluate((element) => { - element.setAttribute('data-geometry-report-capture', 'true'); - element.setAttribute('data-geometry-actions-visible', 'true'); - }); - } - - const revealedSurfaces = page.locator('[data-geometry-capture-reveal="true"]'); - await revealedSurfaces.count(); -} - -type GeometryReplayCapture = Readonly<{ - storyId: string; - storyGlobals?: string; - viewport: Readonly<{ width: number; height: number }>; - deviceScaleFactor: number; - dimensions?: Readonly<{ theme?: string }>; -}>; - -/** - * A context is only worth opening once per SCALE and THEME. Device scale and - * colour scheme are fixed when a context is created, but a viewport is not, and - * a fresh context starts with a cold HTTP cache — so one context per capture - * re-downloads and re-parses the whole Storybook bundle every time, which is - * minutes of wall clock and the reason a story can miss its readiness deadline. - */ -function geometryReplayContextKey(capture: GeometryReplayCapture): string { - return `${capture.deviceScaleFactor}|${capture.dimensions?.theme === 'dark' ? 'dark' : 'light'}`; -} - -async function openGeometryReplayContext( - browser: Browser, - capture: GeometryReplayCapture, - blockedRequests: string[] -): Promise { - const context = await browser.newContext({ - viewport: capture.viewport, - deviceScaleFactor: capture.deviceScaleFactor, - reducedMotion: 'reduce', - colorScheme: capture.dimensions?.theme === 'dark' ? 'dark' : 'light', - }); - await context.route(/https?:\/\//, async (route) => { - const url = new URL(route.request().url()); - if (url.origin === storybookOrigin) { - await route.continue(); - return; - } - blockedRequests.push(url.href); - await route.abort('blockedbyclient'); - }); - return context; -} - -/** - * Show the story a capture came from, at that capture's viewport and settled - * the same way the original pass settled it. Shared by the `--after` replay and - * by the Y cards, so a card and a repair image are never shot against a - * differently composed page. - */ -async function showGeometryCaptureStory( - context: BrowserContext, - capture: GeometryReplayCapture -): Promise { - // A fresh PAGE per capture, inside the shared context: the context keeps the - // HTTP cache warm, and a page that has loaded a dozen stories in a row runs - // its renderer out of memory and crashes mid-navigation. - const page = await context.newPage(); - await page.setViewportSize(capture.viewport); - const response = await page.goto( - `${storybookOrigin}/iframe.html?id=${capture.storyId}&viewMode=story${ - capture.storyGlobals ? `&globals=${capture.storyGlobals}` : '' - }` - ); - if (!response?.ok()) throw new Error(`Story capture failed: ${capture.storyId}`); - if (capture.storyId.includes('sessionconversationpage')) { - await waitForSessionConversationStory(page); - } - await enableReportCaptureMode(page); - if (capture.storyId.startsWith('geometry-chatworkspace--')) { - await measureSettledChatWorkspace(page); - } else { - await page.evaluate(async () => { - await document.fonts.ready; - await new Promise((resolve) => { - requestAnimationFrame(() => requestAnimationFrame(() => resolve())); - }); - }); - } - return page; -} - async function captureAfterReport(browser: Browser, reportOutputDirectory: string): Promise { const dataPath = path.join(reportOutputDirectory, 'report-data.json'); const reportData = JSON.parse(await readFile(dataPath, 'utf8')) as PersistedReportData; @@ -1758,30 +1496,20 @@ test('captures the visual geometry report', async ({ browser }) => { return; } - const viewport = { width: 1440, height: 900 }; - const context = await browser.newContext({ - viewport, - deviceScaleFactor: 2, - reducedMotion: 'reduce', - colorScheme: 'light', - }); + const viewport = GEOMETRY_REPRESENTATIVE_CAPTURE.viewport; const unexpectedNetworkRequests: string[] = []; - await context.route(/https?:\/\//, async (route) => { - const url = new URL(route.request().url()); - if (url.origin === storybookOrigin) { - await route.continue(); - return; - } - unexpectedNetworkRequests.push(url.href); - await route.abort('blockedbyclient'); - }); + const context = await openGeometryReplayContext( + browser, + GEOMETRY_REPRESENTATIVE_CAPTURE, + unexpectedNetworkRequests + ); const page = await context.newPage(); const geometryObservationCache: GeometryObservationCache = new Map(); - const cleanUrl = `${storybookOrigin}/iframe.html?id=geometry-chatworkspace--expanded-sidebar&viewMode=story`; + const cleanUrl = `${storybookOrigin}/iframe.html?id=${GEOMETRY_REPRESENTATIVE_CAPTURE.storyId}&viewMode=story`; const cleanResponse = await page.goto(cleanUrl); if (!cleanResponse?.ok()) throw new Error(`Story capture failed: ${cleanUrl}`); - await enableReportCaptureMode(page); + await enableGeometryCaptureMode(page); const hoverActions = page.locator('[data-geometry-hover-action]'); await hoverActions.count(); @@ -1792,14 +1520,16 @@ test('captures the visual geometry report', async ({ browser }) => { }); const spacingAudit = await auditChatWorkspaceSpacing(page); - const semanticAlignments = await auditChatWorkspaceSemanticAlignments(page); - const semanticBaselines = await auditChatWorkspaceSemanticBaselines(page); - const railDiscovery = await discoverChatWorkspaceAlignmentRails(page, { - aggregateScopes: ['sidebar.shell'], - captureId: 'workspace:wide-expanded', - surfaceFamily: 'workspace', - observationCache: geometryObservationCache, - }); + const planObservations: GeometryPlanObservation[] = []; + const representativeObservation = await observeGeometryPlanEntry( + page, + GEOMETRY_REPRESENTATIVE_CAPTURE, + geometryObservationCache + ); + planObservations.push(representativeObservation); + const semanticAlignments = representativeObservation.semanticAlignments ?? []; + const semanticBaselines = representativeObservation.semanticBaselines ?? []; + const railDiscovery = representativeObservation.railDiscovery; const mainPane = requireGeometryRect( measurement.snapshot, CHAT_WORKSPACE_GEOMETRY_ANCHORS.mainPane @@ -1862,7 +1592,7 @@ test('captures the visual geometry report', async ({ browser }) => { const annotatedUrl = `${storybookOrigin}/iframe.html?id=geometry-chatworkspace--geometry-audit&viewMode=story`; const annotatedResponse = await page.goto(annotatedUrl); if (!annotatedResponse?.ok()) throw new Error(`Story capture failed: ${annotatedUrl}`); - await enableReportCaptureMode(page); + await enableGeometryCaptureMode(page); await measureSettledChatWorkspace(page); const spacingOverlay = page.locator('[data-geometry-devtool="spacing-audit"]'); await spacingOverlay.waitFor({ state: 'attached' }); @@ -1892,87 +1622,36 @@ test('captures the visual geometry report', async ({ browser }) => { }); } - const discoverySurfaces: Array< - Readonly<{ - captureId: string; - contractDomain: 'workspace' | 'session' | 'right-sidebar'; - surface: string; - viewport: Readonly<{ width: number; height: number }>; - railDiscovery: readonly BrowserAlignmentRailDiscoveryScope[]; - }> - > = [ - { - captureId: 'workspace:wide-expanded', - contractDomain: 'workspace', - surface: 'Workspace / Chat Landing', - viewport, - railDiscovery, - }, - ]; - const coverageCaptures: Array< - Readonly<{ - captureId: string; - area: 'workspace' | 'session' | 'right-sidebar'; - surface: string; - storyId: string; - storyGlobals?: string; - viewport: Readonly<{ width: number; height: number }>; - deviceScaleFactor: number; - dimensions: GeometryCaptureDimensions; - }> - > = [ - { - captureId: 'workspace:wide-expanded', - area: 'workspace', - surface: 'Workspace / Chat Landing', - storyId: 'geometry-chatworkspace--expanded-sidebar', - viewport, - deviceScaleFactor: 2, - dimensions: DEFAULT_CAPTURE_DIMENSIONS, - }, - ]; + // One context per device scale and theme, exactly as the gate walk opens + // them: a viewport can be set on an open context, a device scale cannot, and + // a fresh context re-parses the whole Storybook bundle. + const planContexts = new Map([ + [geometryReplayContextKey(GEOMETRY_REPRESENTATIVE_CAPTURE), context], + ]); + const planContextFor = async (entry: GeometryCapturePlanEntry) => { + const key = geometryReplayContextKey(entry); + const existing = planContexts.get(key); + if (existing) return existing; + const opened = await openGeometryReplayContext(browser, entry, unexpectedNetworkRequests); + planContexts.set(key, opened); + return opened; + }; - for (const verificationCase of CHAT_WORKSPACE_GEOMETRY_SPEC.verificationCases) { - if (verificationCase.name === 'wide-expanded') continue; - const matrixContext = await browser.newContext({ - viewport: verificationCase.viewport, - deviceScaleFactor: 1, - reducedMotion: 'reduce', - colorScheme: 'light', - }); - await matrixContext.route(/https?:\/\//, async (route) => { - const url = new URL(route.request().url()); - if (url.origin === storybookOrigin) { - await route.continue(); - return; - } - unexpectedNetworkRequests.push(url.href); - await route.abort('blockedbyclient'); - }); - const matrixPage = await matrixContext.newPage(); - const storyId = - verificationCase.sidebar === 'expanded' - ? 'geometry-chatworkspace--expanded-sidebar' - : 'geometry-chatworkspace--collapsed-sidebar'; - const response = await matrixPage.goto( - `${storybookOrigin}/iframe.html?id=${storyId}&viewMode=story` + for (const entry of GEOMETRY_WORKSPACE_MATRIX_CAPTURES) { + const matrixPage = await showGeometryCaptureStory(await planContextFor(entry), entry); + const matrixObservation = await observeGeometryPlanEntry( + matrixPage, + entry, + geometryObservationCache ); - if (!response?.ok()) throw new Error(`Story capture failed: ${storyId}`); - await enableReportCaptureMode(matrixPage); - await measureSettledChatWorkspace(matrixPage); - const matrixDiscovery = await discoverChatWorkspaceAlignmentRails(matrixPage, { - aggregateScopes: ['sidebar.shell'], - captureId: `workspace:${verificationCase.name}`, - surfaceFamily: 'workspace', - observationCache: geometryObservationCache, - }); + planObservations.push(matrixObservation); const matrixOverview = createDiscoveryOverviewDetail({ - surface: `Workspace / ${verificationCase.name}`, - idPrefix: `workspace-${verificationCase.name}`, - viewport: verificationCase.viewport, - railDiscovery: matrixDiscovery, + surface: entry.surface, + idPrefix: entry.detailId, + viewport: entry.viewport, + railDiscovery: matrixObservation.railDiscovery, }); - assignDetailsToCapture([matrixOverview], `workspace:${verificationCase.name}`); + assignDetailsToCapture([matrixOverview], entry.captureId); await matrixPage.screenshot({ path: path.join(outputDirectory, matrixOverview.images.clean), clip: matrixOverview.clip, @@ -1989,75 +1668,24 @@ test('captures the visual geometry report', async ({ browser }) => { scale: 'device', }); workspaceDetails.push(matrixOverview); - discoverySurfaces.push({ - captureId: `workspace:${verificationCase.name}`, - contractDomain: 'workspace', - surface: `Workspace / ${verificationCase.name}`, - viewport: verificationCase.viewport, - railDiscovery: matrixDiscovery, - }); - coverageCaptures.push({ - captureId: `workspace:${verificationCase.name}`, - area: 'workspace', - surface: `Workspace / ${verificationCase.name}`, - storyId, - viewport: verificationCase.viewport, - deviceScaleFactor: 1, - dimensions: DEFAULT_CAPTURE_DIMENSIONS, - }); - await matrixContext.close(); + await matrixPage.close(); } - for (const dimension of WORKSPACE_DIMENSION_CAPTURES) { - const dimensionContext = await browser.newContext({ - viewport, - deviceScaleFactor: 2, - reducedMotion: 'reduce', - colorScheme: dimension.colorScheme, - }); - await dimensionContext.route(/https?:\/\//, async (route) => { - const url = new URL(route.request().url()); - if (url.origin === storybookOrigin) { - await route.continue(); - return; - } - unexpectedNetworkRequests.push(url.href); - await route.abort('blockedbyclient'); - }); - const dimensionPage = await dimensionContext.newPage(); - const dimensionStoryId = 'geometry-chatworkspace--expanded-sidebar'; - const dimensionUrl = `${storybookOrigin}/iframe.html?id=${dimensionStoryId}&viewMode=story&globals=${dimension.globals}`; - const dimensionResponse = await dimensionPage.goto(dimensionUrl); - if (!dimensionResponse?.ok()) throw new Error(`Story capture failed: ${dimensionUrl}`); - await enableReportCaptureMode(dimensionPage); - await measureSettledChatWorkspace(dimensionPage); - const expectedText = 'expectedText' in dimension ? dimension.expectedText : undefined; - if (expectedText) { - await dimensionPage - .getByText(expectedText, { exact: false }) - .first() - .waitFor({ state: 'visible', timeout: 30_000 }); - } - if (dimension.dimensions.theme === 'dark') { - const isDark = await dimensionPage.evaluate(() => - document.documentElement.classList.contains('dark') - ); - if (!isDark) throw new Error(`${dimension.id} did not apply the dark theme global`); - } - const dimensionCaptureId = `workspace:${dimension.id}`; - const dimensionDiscovery = await discoverChatWorkspaceAlignmentRails(dimensionPage, { - aggregateScopes: ['sidebar.shell'], - captureId: dimensionCaptureId, - surfaceFamily: 'workspace', - observationCache: geometryObservationCache, - }); + for (const entry of GEOMETRY_WORKSPACE_DIMENSION_CAPTURES) { + const dimensionPage = await showGeometryCaptureStory(await planContextFor(entry), entry); + const dimensionObservation = await observeGeometryPlanEntry( + dimensionPage, + entry, + geometryObservationCache + ); + planObservations.push(dimensionObservation); const dimensionOverview = createDiscoveryOverviewDetail({ - surface: dimension.surface, - idPrefix: dimension.id, - viewport, - railDiscovery: dimensionDiscovery, + surface: entry.surface, + idPrefix: entry.detailId, + viewport: entry.viewport, + railDiscovery: dimensionObservation.railDiscovery, }); - assignDetailsToCapture([dimensionOverview], dimensionCaptureId); + assignDetailsToCapture([dimensionOverview], entry.captureId); await dimensionPage.screenshot({ path: path.join(outputDirectory, dimensionOverview.images.clean), clip: dimensionOverview.clip, @@ -2074,39 +1702,19 @@ test('captures the visual geometry report', async ({ browser }) => { scale: 'device', }); workspaceDetails.push(dimensionOverview); - discoverySurfaces.push({ - captureId: dimensionCaptureId, - contractDomain: 'workspace', - surface: dimension.surface, - viewport, - railDiscovery: dimensionDiscovery, - }); - coverageCaptures.push({ - captureId: dimensionCaptureId, - area: 'workspace', - surface: dimension.surface, - storyId: dimensionStoryId, - storyGlobals: dimension.globals, - viewport, - deviceScaleFactor: 2, - dimensions: dimension.dimensions, - }); - await dimensionContext.close(); + await dimensionPage.close(); } - for (const story of WORKSPACE_STATE_CAPTURES) { - const response = await page.goto( - `${storybookOrigin}/iframe.html?id=${story.storyId}&viewMode=story` + for (const entry of GEOMETRY_WORKSPACE_STATE_CAPTURES) { + const statePage = await showGeometryCaptureStory(await planContextFor(entry), entry); + const stateMeasurement = await measureSettledChatWorkspace(statePage); + const stateObservation = await observeGeometryPlanEntry( + statePage, + entry, + geometryObservationCache ); - if (!response?.ok()) throw new Error(`Story capture failed: ${story.storyId}`); - await enableReportCaptureMode(page); - const stateMeasurement = await measureSettledChatWorkspace(page); - const stateRailDiscovery = await discoverChatWorkspaceAlignmentRails(page, { - aggregateScopes: ['sidebar.shell'], - captureId: `workspace:${story.id}:1440x900`, - surfaceFamily: 'workspace', - observationCache: geometryObservationCache, - }); + planObservations.push(stateObservation); + const stateRailDiscovery = stateObservation.railDiscovery; const stateMainPane = requireGeometryRect( stateMeasurement.snapshot, CHAT_WORKSPACE_GEOMETRY_ANCHORS.mainPane @@ -2115,22 +1723,21 @@ test('captures the visual geometry report', async ({ browser }) => { (scope) => scope.scope === 'main.chat-landing' || scope.rect.x >= stateMainPane.x - 1 ); const discoveredStateDetails = createDiscoveryDetails({ - surface: story.surface, - idPrefix: story.id, - viewport, + surface: entry.surface, + idPrefix: entry.detailId, + viewport: entry.viewport, railDiscovery: mainDiscovery, }); const stateOverview = createDiscoveryOverviewDetail({ - surface: story.surface, - idPrefix: story.id, - viewport, + surface: entry.surface, + idPrefix: entry.detailId, + viewport: entry.viewport, railDiscovery: stateRailDiscovery, }); const stateDetails = [stateOverview, ...discoveredStateDetails]; - const captureId = `workspace:${story.id}:1440x900`; - assignDetailsToCapture(stateDetails, captureId); + assignDetailsToCapture(stateDetails, entry.captureId); for (const detail of stateDetails) { - await page.screenshot({ + await statePage.screenshot({ path: path.join(outputDirectory, detail.images.clean), clip: detail.clip, animations: 'disabled', @@ -2139,8 +1746,8 @@ test('captures the visual geometry report', async ({ browser }) => { }); } for (const detail of stateDetails) { - await showOnlyDetailSemanticGuides(page, detail); - await page.screenshot({ + await showOnlyDetailSemanticGuides(statePage, detail); + await statePage.screenshot({ path: path.join(outputDirectory, detail.images.annotated), clip: detail.clip, animations: 'disabled', @@ -2149,66 +1756,37 @@ test('captures the visual geometry report', async ({ browser }) => { }); } workspaceDetails.push(...stateDetails); - discoverySurfaces.push({ - captureId, - contractDomain: 'workspace', - surface: story.surface, - viewport, - railDiscovery: stateRailDiscovery, - }); - coverageCaptures.push({ - captureId, - area: 'workspace', - surface: story.surface, - storyId: story.storyId, - viewport, - deviceScaleFactor: 2, - dimensions: DEFAULT_CAPTURE_DIMENSIONS, - }); + await statePage.close(); } const sessionDetails: ReportDetail[] = []; - for (const story of SESSION_STATE_CAPTURES) { - const response = await page.goto( - `${storybookOrigin}/iframe.html?id=${story.storyId}&viewMode=story` + for (const entry of GEOMETRY_SESSION_STATE_CAPTURES) { + const sessionPage = await showGeometryCaptureStory(await planContextFor(entry), entry); + const sessionObservation = await observeGeometryPlanEntry( + sessionPage, + entry, + geometryObservationCache ); - if (!response?.ok()) throw new Error(`Story capture failed: ${story.storyId}`); - await waitForSessionConversationStory(page); - await enableReportCaptureMode(page); - if (story.id === 'session-working') { - await page.locator('[data-stream-phase="indicator-only"]').waitFor({ state: 'attached' }); - } - if (story.id === 'session-permission') { - const responseActionBar = page.locator( - '[data-geometry-capture-reveal="true"]:has(.lucide-info)' - ); - await responseActionBar.first().waitFor({ state: 'attached' }); - } - const sessionRailDiscovery = await discoverChatWorkspaceAlignmentRails(page, { - aggregateScopes: ['session.page'], - captureId: `${story.id}:1440x900`, - surfaceFamily: 'session', - observationCache: geometryObservationCache, - }); + planObservations.push(sessionObservation); + const sessionRailDiscovery = sessionObservation.railDiscovery; const storyDetails = createDiscoveryDetails({ - surface: story.surface, - idPrefix: story.id, - viewport, + surface: entry.surface, + idPrefix: entry.detailId, + viewport: entry.viewport, railDiscovery: sessionRailDiscovery, }); const sessionOverview = createDiscoveryOverviewDetail({ - surface: story.surface, - idPrefix: story.id, - viewport, + surface: entry.surface, + idPrefix: entry.detailId, + viewport: entry.viewport, railDiscovery: sessionRailDiscovery, }); const sessionReportDetails = [sessionOverview, ...storyDetails]; - const captureId = `${story.id}:1440x900`; - assignDetailsToCapture(sessionReportDetails, captureId); + assignDetailsToCapture(sessionReportDetails, entry.captureId); for (const detail of sessionReportDetails) { - await page.screenshot({ + await sessionPage.screenshot({ path: path.join(outputDirectory, detail.images.clean), clip: detail.clip, animations: 'disabled', @@ -2217,8 +1795,8 @@ test('captures the visual geometry report', async ({ browser }) => { }); } for (const detail of sessionReportDetails) { - await showOnlyDetailSemanticGuides(page, detail); - await page.screenshot({ + await showOnlyDetailSemanticGuides(sessionPage, detail); + await sessionPage.screenshot({ path: path.join(outputDirectory, detail.images.annotated), clip: detail.clip, animations: 'disabled', @@ -2228,50 +1806,32 @@ test('captures the visual geometry report', async ({ browser }) => { } sessionDetails.push(...sessionReportDetails); - discoverySurfaces.push({ - captureId, - contractDomain: 'session', - surface: story.surface, - viewport, - railDiscovery: sessionRailDiscovery, - }); - coverageCaptures.push({ - captureId, - area: 'session', - surface: story.surface, - storyId: story.storyId, - viewport, - deviceScaleFactor: 2, - dimensions: DEFAULT_CAPTURE_DIMENSIONS, - }); + await sessionPage.close(); } - for (const story of RIGHT_SIDEBAR_STATE_CAPTURES) { - const response = await page.goto( - `${storybookOrigin}/iframe.html?id=${story.storyId}&viewMode=story` + for (const entry of GEOMETRY_RIGHT_SIDEBAR_STATE_CAPTURES) { + const sidebarPage = await showGeometryCaptureStory(await planContextFor(entry), entry); + const sidebarObservation = await observeGeometryPlanEntry( + sidebarPage, + entry, + geometryObservationCache ); - if (!response?.ok()) throw new Error(`Story capture failed: ${story.storyId}`); - await enableReportCaptureMode(page); - const rightSidebarRailDiscovery = await discoverChatWorkspaceAlignmentRails(page, { - aggregateScopes: ['session.side-panel'], - captureId: `${story.id}:1440x900`, - surfaceFamily: 'right-sidebar', - observationCache: geometryObservationCache, - }); + planObservations.push(sidebarObservation); + const rightSidebarRailDiscovery = sidebarObservation.railDiscovery; const discoveredRightSidebarDetails = createDiscoveryDetails({ - surface: story.surface, - idPrefix: story.id, - viewport, + surface: entry.surface, + idPrefix: entry.detailId, + viewport: entry.viewport, railDiscovery: rightSidebarRailDiscovery, }); const sidePanelScope = rightSidebarRailDiscovery.find( (scope) => scope.scope === 'session.side-panel' ); - if (!sidePanelScope) throw new Error(`${story.storyId} did not expose session.side-panel`); + if (!sidePanelScope) throw new Error(`${entry.storyId} did not expose session.side-panel`); const rightSidebarOverview = createDiscoveryOverviewDetail({ - surface: story.surface, - idPrefix: story.id, - viewport, + surface: entry.surface, + idPrefix: entry.detailId, + viewport: entry.viewport, railDiscovery: rightSidebarRailDiscovery, clip: { x: sidePanelScope.rect.x - 8, @@ -2281,10 +1841,9 @@ test('captures the visual geometry report', async ({ browser }) => { }, }); const rightSidebarDetails = [rightSidebarOverview, ...discoveredRightSidebarDetails]; - const captureId = `${story.id}:1440x900`; - assignDetailsToCapture(rightSidebarDetails, captureId); + assignDetailsToCapture(rightSidebarDetails, entry.captureId); for (const detail of rightSidebarDetails) { - await page.screenshot({ + await sidebarPage.screenshot({ path: path.join(outputDirectory, detail.images.clean), clip: detail.clip, animations: 'disabled', @@ -2293,8 +1852,8 @@ test('captures the visual geometry report', async ({ browser }) => { }); } for (const detail of rightSidebarDetails) { - await showOnlyDetailSemanticGuides(page, detail); - await page.screenshot({ + await showOnlyDetailSemanticGuides(sidebarPage, detail); + await sidebarPage.screenshot({ path: path.join(outputDirectory, detail.images.annotated), clip: detail.clip, animations: 'disabled', @@ -2303,24 +1862,29 @@ test('captures the visual geometry report', async ({ browser }) => { }); } sessionDetails.push(...rightSidebarDetails); - discoverySurfaces.push({ - captureId, - contractDomain: 'right-sidebar', - surface: story.surface, - viewport, - railDiscovery: rightSidebarRailDiscovery, - }); - coverageCaptures.push({ - captureId, - area: 'right-sidebar', - surface: story.surface, - storyId: story.storyId, - viewport, - deviceScaleFactor: 2, - dimensions: DEFAULT_CAPTURE_DIMENSIONS, - }); + await sidebarPage.close(); } + // Both derived from the ONE walk above: a surface the report draws and a + // capture the pipeline reads can never be two different lists. + const discoverySurfaces = planObservations.map((observation) => ({ + captureId: observation.entry.captureId, + contractDomain: observation.entry.area, + surface: observation.entry.surface, + viewport: observation.entry.viewport, + railDiscovery: observation.railDiscovery, + })); + const coverageCaptures = planObservations.map(({ entry }) => ({ + captureId: entry.captureId, + area: entry.area, + surface: entry.surface, + storyId: entry.storyId, + ...(entry.storyGlobals ? { storyGlobals: entry.storyGlobals } : {}), + viewport: entry.viewport, + deviceScaleFactor: entry.deviceScaleFactor, + dimensions: entry.dimensions, + })); + const details = [...workspaceDetails, ...sessionDetails] .map((detail, sourceIndex) => ({ detail, sourceIndex })) .sort( @@ -2385,72 +1949,14 @@ test('captures the visual geometry report', async ({ browser }) => { ); const captureArtifact: GeometryCaptureArtifact = { version: 1, - captures: discoverySurfaces.map((surface) => { - const coverage = coverageByCaptureId.get(surface.captureId); - if (!coverage) throw new Error(`Missing coverage for geometry capture ${surface.captureId}`); - const representative = detailsByCaptureId - .get(surface.captureId) - ?.find((detail) => detail.kind === 'overview'); - const boxModelNodes = Object.assign( - {}, - ...surface.railDiscovery.map((scope) => scope.capturedScope.boxModelNodes ?? {}) - ); - return { - captureId: surface.captureId, - surfaceFamily: surface.contractDomain, - surface: surface.surface, - storyId: coverage.storyId, - viewport: coverage.viewport, - deviceScaleFactor: coverage.deviceScaleFactor, - dimensions: coverage.dimensions, - screenshot: representative?.images.clean ?? '', - scopes: surface.railDiscovery.map((scope) => { - const { boxModelNodes: _boxModelNodes, ...capturedScope } = scope.capturedScope; - return capturedScope; - }), - boxModelNodes, - ...(surface.captureId === 'workspace:wide-expanded' - ? { - semanticAlignments: semanticAlignments.map((entry) => { - const [group, instance] = entry.groupLabel.split(' · '); - return { - group: group ?? entry.groupLabel, - instance: instance ?? null, - axis: entry.axis, - anchor: entry.anchor, - status: entry.status, - line: entry.line, - members: entry.members.map((member) => ({ - name: member.name, - coordinate: member.coordinate, - ...(member.primitiveId ? { primitiveId: member.primitiveId } : {}), - rect: member.rect, - })), - }; - }), - // The baseline rules travel with the alignment rules so - // marker-removal readiness asks one question of every marker. - semanticBaselines: semanticBaselines.map((entry) => { - const [group, instance] = entry.groupLabel.split(' · '); - return { - group: group ?? entry.groupLabel, - instance: instance ?? null, - axis: 'y' as const, - anchor: 'text-baseline' as const, - status: entry.status, - line: entry.line, - members: entry.members.map((member) => ({ - name: member.name, - coordinate: member.coordinate, - ...(member.primitiveId ? { primitiveId: member.primitiveId } : {}), - rect: member.rect, - })), - }; - }), - } - : {}), - }; - }), + captures: planObservations.map((observation) => + buildGeometryCapture( + observation, + detailsByCaptureId + .get(observation.entry.captureId) + ?.find((detail) => detail.kind === 'overview')?.images.clean ?? '' + ) + ), }; const capturePath = path.join(outputDirectory, 'capture.json'); const observationPath = path.join(outputDirectory, 'observation.json'); @@ -2897,5 +2403,7 @@ test('captures the visual geometry report', async ({ browser }) => { if (unexpectedNetworkRequests.length > 0) { throw new Error(`Unexpected network requests: ${unexpectedNetworkRequests.join(', ')}`); } - await context.close(); + for (const planContext of planContexts.values()) { + await planContext.close().catch(() => undefined); + } }); diff --git a/packages/components/tests/e2e/chat-workspace-geometry.spec.ts b/packages/components/tests/e2e/chat-workspace-geometry.spec.ts index df328649e..fbb7cde18 100644 --- a/packages/components/tests/e2e/chat-workspace-geometry.spec.ts +++ b/packages/components/tests/e2e/chat-workspace-geometry.spec.ts @@ -1,4 +1,5 @@ -import { readFile } from 'node:fs/promises'; +import { mkdir, readFile, writeFile } from 'node:fs/promises'; +import path from 'node:path'; import { expect, test } from '@playwright/test'; @@ -12,10 +13,18 @@ import { validateChatWorkspaceGeometry, } from '../../src/lib/chat-workspace-geometry'; import { + checkGeometryLedgerRatchet, compileGeometryContracts, + formatGeometryRatchetViolations, type GeometryContractArtifact, type GeometryLedger, + type GeometryObservationCache, } from '../../src/lib/geometry-constraint-system'; +import { + GEOMETRY_CAPTURE_PLAN, + runGeometryCapturePlan, + runGeometryFindingPipeline, +} from './support/geometry-capture-plan'; import { auditChatWorkspaceSemanticAlignments, captureChatWorkspaceGeometryScopes, @@ -29,6 +38,13 @@ import { validateCompiledGeometryContracts, } from './support/chat-workspace-geometry'; +/** + * Set to write `capture.json` / `observation.json` / `findings.json` beside the + * ratchet run. CI does not need them and does not set it, so the gate stays a + * measurement with no artifacts. + */ +const pipelineOutputDirectory = process.env.GEOMETRY_PIPELINE_OUTPUT_DIR; + const STORY_IDS = { expanded: 'geometry-chatworkspace--expanded-sidebar', collapsed: 'geometry-chatworkspace--collapsed-sidebar', @@ -60,6 +76,56 @@ test('compiled promoted geometry contracts are current and pass by stable locato expect(violations).toEqual([]); }); +/** + * The ratchet. Every other test here proves a RULE; this one proves the LEDGER, + * which until now was a file the report printed and nothing executed: a + * baseline could be exceeded, and a brand new finding could appear, with every + * check still green. + * + * It runs the same `capture → observation → findings` pipeline the report runs, + * over the same capture plan, with no screenshots — the screenshots are what + * make the report a 40-minute review artifact, and none of them are evidence. + */ +test('every measured geometry finding stays inside its reviewed ledger baseline', async ({ + browser, +}) => { + test.setTimeout(1_800_000); + const ledger = JSON.parse( + await readFile(new URL('../../geometry-ledger.json', import.meta.url), 'utf8') + ) as GeometryLedger; + + const observationCache: GeometryObservationCache = new Map(); + const blockedRequests: string[] = []; + const capture = await runGeometryCapturePlan(browser, GEOMETRY_CAPTURE_PLAN, { + observationCache, + blockedRequests, + }); + expect(blockedRequests, 'Geometry fixture made an external request').toEqual([]); + const { observation, findings } = runGeometryFindingPipeline(capture); + + if (pipelineOutputDirectory) { + await mkdir(pipelineOutputDirectory, { recursive: true }); + for (const [name, artifact] of [ + ['capture.json', capture], + ['observation.json', observation], + ['findings.json', findings], + ] as const) { + await writeFile( + path.join(pipelineOutputDirectory, name), + `${JSON.stringify(artifact, null, 2)}\n`, + 'utf8' + ); + } + } + + const violations = checkGeometryLedgerRatchet(findings, ledger, capture); + expect( + violations, + `Geometry ledger ratchet failed:\n\n${formatGeometryRatchetViolations(violations)}` + ).toEqual([]); + +}); + for (const verificationCase of CHAT_WORKSPACE_GEOMETRY_SPEC.verificationCases) { test(`${verificationCase.name} satisfies the authenticated chat workspace contract`, async ({ browser, diff --git a/packages/components/tests/e2e/support/AGENTS.md b/packages/components/tests/e2e/support/AGENTS.md index d3b9a5248..d2a8937e4 100644 --- a/packages/components/tests/e2e/support/AGENTS.md +++ b/packages/components/tests/e2e/support/AGENTS.md @@ -39,15 +39,14 @@ the overlay and capture cannot disagree about one row. `auditChatWorkspaceSemanticAlignments` / `…Baselines` use one cap-band measurement, and all number `[document.body, ...document.body.querySelectorAll('*')]` the same way, so a marker member and a discovered Y candidate are comparable as ONE `dom-N` element. No - pass may leave the document mutated; the parity artifact reports unmatched members - rather than trusting overlapping rectangles. + pass may leave the document mutated. - Accessible names come from one deliberately small accname subset (`computeAccessibleNameInBrowser`), sized to what `getByRole(…, { name })` resolves for - the widgets captured, so a captured name is one Playwright resolves. A content-named - role owns the locator of the primitives inside it. Names are LABELS, never identity: a - repeated row prints `role “row title”` (the longest direct text in it), another named - primitive its own text minus its nested controls' names (`Files Close Files` → `Files`), - an unnamed primitive its row family plus same-role index — never a raw family string. + the widgets captured, so a captured name is one Playwright resolves. A content-named role + owns the locator of the primitives inside it. Names are LABELS, never identity: a repeated + row prints `role “row title”` (its longest direct text), another named primitive its own + text minus its nested controls' names (`Files Close Files` → `Files`), an unnamed + primitive its row family plus same-role index — never a raw family string. ## Proving the markers can go @@ -61,16 +60,28 @@ the overlay and capture cannot disagree about one row. companion for the rule actually being retired: re-ask its marker and Y discovery about one capture, matched by ELEMENT — `coordinateDelta` is whether they measure an element alike, `offsetDelta` whether they place the row line alike; a member only one side saw is - listed, never averaged away. `sidebar.row.visual-center` was the first rule proven this - way (18/18 members, zero deltas) and its declaration, gate coverage, and product - `data-geometry-align-*` markers are gone — discovery alone now covers those rows. Wire - this function into the report again for the NEXT candidate; it is not part of the - standing report run. + listed, never averaged away. `sidebar.row.visual-center` was proven this way (18/18 + members, zero deltas) and its declaration, gate coverage and product + `data-geometry-align-*` markers are gone. Wire this function into the report again for + the NEXT candidate; it is not part of the standing run. -- Replaying a capture — the `--after` repair images, the zoomed Y cards — opens ONE context - per scale and theme, never one per capture: a viewport can be set on an open context, a - device scale cannot, and a fresh context's cold cache re-parses the whole Storybook - bundle, which is minutes of wall clock and a missed readiness deadline. +## The capture plan + +`GEOMETRY_CAPTURE_PLAN` (`geometry-capture-plan.ts`) is the ONE list of captures: the +report shoots it, the gate only measures it, and both open a story through +`showGeometryCaptureStory`, so one cannot settle a page the other could not. The +representative capture alone carries the marker-rule observations, and dropping any capture +to make the gate cheaper moves every merged offset the ledger recorded. + +- Opening a capture — the plan walk, `--after` repair images, zoomed Y cards — opens ONE + context per scale and theme, never one per capture: a viewport can be set on an open + context, a device scale cannot, and a fresh context's cold cache re-parses the whole + Storybook bundle — minutes of wall clock. A fresh PAGE per capture inside it, though: a + page that loaded a dozen stories runs out of memory. +- A card clip holds the row plus a margin, draws the row median and verdict anchor only, + and names it. Zoomed Y cards are the largest-|offset| findings anywhere. Discovery cards + use product-region names, count unique elements not anchor votes, fold one element's + start/center/end offsets into one annotation, and keep candidate rails under outliers. ## Contract resolution and witnesses diff --git a/packages/components/tests/e2e/support/geometry-capture-plan.ts b/packages/components/tests/e2e/support/geometry-capture-plan.ts new file mode 100644 index 000000000..757698635 --- /dev/null +++ b/packages/components/tests/e2e/support/geometry-capture-plan.ts @@ -0,0 +1,585 @@ +import type { Browser, BrowserContext, Page } from '@playwright/test'; + +import { + CHAT_WORKSPACE_GEOMETRY_ANCHORS, + CHAT_WORKSPACE_GEOMETRY_ATTRIBUTE, + CHAT_WORKSPACE_GEOMETRY_SPEC, +} from '../../../src/lib/chat-workspace-geometry'; +import { + createGeometryFindings, + observeGeometryCaptures, + type GeometryCapture, + type GeometryCaptureArtifact, + type GeometryFindingArtifact, + type GeometryObservationArtifact, + type GeometryObservationCache, + type GeometrySurfaceFamily, +} from '../../../src/lib/geometry-constraint-system'; +import { + auditChatWorkspaceSemanticAlignments, + auditChatWorkspaceSemanticBaselines, + discoverChatWorkspaceAlignmentRails, + measureSettledChatWorkspace, + type BrowserAlignmentRailDiscoveryScope, + type BrowserSemanticAlignmentEntry, + type BrowserSemanticBaselineEntry, +} from './chat-workspace-geometry'; + +export const geometryStorybookOrigin = process.env.PLAYWRIGHT_BASE_URL ?? 'http://127.0.0.1:6006'; + +export type GeometryCaptureDimensions = Readonly<{ + theme: string; + locale: string; + density: string; +}>; + +/** Storybook's own globals; there is no density global, so it stays constant. */ +export const DEFAULT_CAPTURE_DIMENSIONS: GeometryCaptureDimensions = { + theme: 'light', + locale: 'en', + density: 'default', +}; + +/** + * One capture the pipeline reads. The report shoots it and the gate only + * measures it: both walk THIS list, so a story the report reviews and a story + * the ratchet enforces can never drift apart. + */ +export type GeometryCapturePlanEntry = Readonly<{ + captureId: string; + /** Short, capture-id-free prefix the report names its detail ids after. */ + detailId: string; + /** Report grouping; also the finding `surfaceFamily`. */ + area: GeometrySurfaceFamily & ('workspace' | 'session' | 'right-sidebar'); + surface: string; + storyId: string; + storyGlobals?: string; + viewport: Readonly<{ width: number; height: number }>; + deviceScaleFactor: number; + dimensions: GeometryCaptureDimensions; + aggregateScopes: readonly string[]; + /** Rendered text proving a globals-driven dimension actually applied. */ + expectedText?: string; + /** Extra readiness selectors this story needs before it is measured. */ + readySelectors?: readonly string[]; + /** + * Marker-rule observations ride ONE representative capture: they exist to ask + * whether discovery has replaced a marker, not to be measured everywhere. + */ + semanticObservations?: boolean; +}>; + +const WIDE_VIEWPORT = { width: 1440, height: 900 } as const; + +/** + * Dimensions vary ONE capture family rather than every story: they exist to show + * whether a finding survives a theme or a locale, and a full matrix would + * multiply the runtime for evidence nobody reads. + */ +export const GEOMETRY_WORKSPACE_DIMENSION_CAPTURES: readonly GeometryCapturePlanEntry[] = [ + { + captureId: 'workspace:wide-expanded-dark', + detailId: 'wide-expanded-dark', + area: 'workspace', + surface: 'Workspace / Chat Landing / Dark', + storyId: 'geometry-chatworkspace--expanded-sidebar', + storyGlobals: 'theme:dark', + viewport: WIDE_VIEWPORT, + deviceScaleFactor: 2, + dimensions: { ...DEFAULT_CAPTURE_DIMENSIONS, theme: 'dark' }, + aggregateScopes: ['sidebar.shell'], + }, + { + captureId: 'workspace:wide-expanded-zh', + detailId: 'wide-expanded-zh', + area: 'workspace', + surface: 'Workspace / Chat Landing / 中文', + storyId: 'geometry-chatworkspace--expanded-sidebar', + storyGlobals: 'locale:zh_CN', + viewport: WIDE_VIEWPORT, + deviceScaleFactor: 2, + dimensions: { ...DEFAULT_CAPTURE_DIMENSIONS, locale: 'zh_CN' }, + aggregateScopes: ['sidebar.shell'], + // A zh_CN capture that still rendered English strings would silently claim a + // locale axis it never varied, so the capture asserts a translated label. + // The Sidebar's Chats section header is the one this fixture translates. + expectedText: '对话', + }, +]; + +export const GEOMETRY_WORKSPACE_STATE_CAPTURES: readonly GeometryCapturePlanEntry[] = ( + [ + ['landing-submitting', 'Submitting', 'geometry-chatworkspace--submission-pending'], + [ + 'landing-no-machine-download', + 'No Machine Download', + 'geometry-chatworkspace--no-machine-download', + ], + [ + 'landing-no-machine-starting', + 'No Machine Starting', + 'geometry-chatworkspace--no-machine-starting', + ], + ['landing-no-agent', 'No Agent', 'geometry-chatworkspace--no-agent-config'], + ['landing-long-model', 'Long Model', 'geometry-chatworkspace--long-model'], + ['landing-pasted-text', 'Pasted Text', 'geometry-chatworkspace--pasted-text'], + ] as const +).map(([id, name, storyId]) => ({ + captureId: `workspace:${id}:1440x900`, + detailId: id, + area: 'workspace' as const, + surface: `Workspace / Chat Landing / ${name}`, + storyId, + viewport: WIDE_VIEWPORT, + deviceScaleFactor: 2, + dimensions: DEFAULT_CAPTURE_DIMENSIONS, + aggregateScopes: ['sidebar.shell'], +})); + +export const GEOMETRY_SESSION_STATE_CAPTURES: readonly GeometryCapturePlanEntry[] = [ + { + captureId: 'session-idle:1440x900', + detailId: 'session-idle', + area: 'session', + surface: 'Chat Session / Idle', + storyId: 'sessions-sessionconversationpage--desktop-idle', + viewport: WIDE_VIEWPORT, + deviceScaleFactor: 2, + dimensions: DEFAULT_CAPTURE_DIMENSIONS, + aggregateScopes: ['session.page'], + }, + { + captureId: 'session-working:1440x900', + detailId: 'session-working', + area: 'session', + surface: 'Chat Session / Working', + storyId: 'sessions-sessionconversationpage--desktop-working-settled', + viewport: WIDE_VIEWPORT, + deviceScaleFactor: 2, + dimensions: DEFAULT_CAPTURE_DIMENSIONS, + aggregateScopes: ['session.page'], + readySelectors: ['[data-stream-phase="indicator-only"]'], + }, + { + captureId: 'session-permission:1440x900', + detailId: 'session-permission', + area: 'session', + surface: 'Chat Session / Permission', + storyId: 'sessions-sessionconversationpage--desktop-permission-approval', + viewport: WIDE_VIEWPORT, + deviceScaleFactor: 2, + dimensions: DEFAULT_CAPTURE_DIMENSIONS, + aggregateScopes: ['session.page'], + readySelectors: ['[data-geometry-capture-reveal="true"]:has(.lucide-info)'], + }, + { + captureId: 'session-question:1440x900', + detailId: 'session-question', + area: 'session', + surface: 'Chat Session / Agent Question', + storyId: 'sessions-sessionconversationpage--desktop-agent-question', + viewport: WIDE_VIEWPORT, + deviceScaleFactor: 2, + dimensions: DEFAULT_CAPTURE_DIMENSIONS, + aggregateScopes: ['session.page'], + }, +]; + +export const GEOMETRY_RIGHT_SIDEBAR_STATE_CAPTURES: readonly GeometryCapturePlanEntry[] = ( + [ + ['session-right-sidebar-changes', 'Changes', 'sessions-sessionsidepaneltabbar--geometry-report'], + ['session-right-sidebar-tabs', 'Tabs', 'sessions-sessionsidepaneltabbar--unified-tabs'], + ['session-right-sidebar-empty', 'Empty', 'sessions-sessionsidepaneltabbar--empty-state'], + ] as const +).map(([id, name, storyId]) => ({ + captureId: `${id}:1440x900`, + detailId: id, + area: 'right-sidebar' as const, + surface: `Chat Session / Right Sidebar / ${name}`, + storyId, + viewport: WIDE_VIEWPORT, + deviceScaleFactor: 2, + dimensions: DEFAULT_CAPTURE_DIMENSIONS, + aggregateScopes: ['session.side-panel'], +})); + +/** The viewport matrix, minus the representative capture that opens the plan. */ +export const GEOMETRY_WORKSPACE_MATRIX_CAPTURES: readonly GeometryCapturePlanEntry[] = + CHAT_WORKSPACE_GEOMETRY_SPEC.verificationCases + .filter((verificationCase) => verificationCase.name !== 'wide-expanded') + .map((verificationCase) => ({ + captureId: `workspace:${verificationCase.name}`, + detailId: `workspace-${verificationCase.name}`, + area: 'workspace' as const, + surface: `Workspace / ${verificationCase.name}`, + storyId: + verificationCase.sidebar === 'expanded' + ? 'geometry-chatworkspace--expanded-sidebar' + : 'geometry-chatworkspace--collapsed-sidebar', + viewport: verificationCase.viewport, + deviceScaleFactor: 1, + dimensions: DEFAULT_CAPTURE_DIMENSIONS, + aggregateScopes: ['sidebar.shell'], + })); + +export const GEOMETRY_REPRESENTATIVE_CAPTURE_ID = 'workspace:wide-expanded'; + +export const GEOMETRY_REPRESENTATIVE_CAPTURE: GeometryCapturePlanEntry = { + captureId: GEOMETRY_REPRESENTATIVE_CAPTURE_ID, + detailId: 'workspace', + area: 'workspace', + surface: 'Workspace / Chat Landing', + storyId: 'geometry-chatworkspace--expanded-sidebar', + viewport: WIDE_VIEWPORT, + deviceScaleFactor: 2, + dimensions: DEFAULT_CAPTURE_DIMENSIONS, + aggregateScopes: ['sidebar.shell'], + semanticObservations: true, +}; + +export const GEOMETRY_CAPTURE_PLAN: readonly GeometryCapturePlanEntry[] = [ + GEOMETRY_REPRESENTATIVE_CAPTURE, + ...GEOMETRY_WORKSPACE_MATRIX_CAPTURES, + ...GEOMETRY_WORKSPACE_DIMENSION_CAPTURES, + ...GEOMETRY_WORKSPACE_STATE_CAPTURES, + ...GEOMETRY_SESSION_STATE_CAPTURES, + ...GEOMETRY_RIGHT_SIDEBAR_STATE_CAPTURES, +]; + +export async function waitForSessionConversationStory(page: Page): Promise { + await page.locator('[data-testid="session-conversation-story"]').waitFor({ + state: 'visible', + timeout: 90_000, + }); + await page.evaluate(async () => { + await document.fonts.ready; + await new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }); + }); +} + +export async function enableGeometryCaptureMode(page: Page): Promise { + // Generous on purpose: the FIRST story in a cold browser context compiles and + // parses the whole Storybook bundle, which is a minute on a loaded machine + // and 4 seconds once warm. This is an explicit readiness signal, never a + // sleep — the deadline only decides how loaded a machine may be. + await page + .locator('[data-geometry-fixture-ready="true"], [data-testid="session-conversation-story"]') + .first() + .waitFor({ state: 'attached', timeout: 180_000 }); + await page.addStyleTag({ + content: ` + [data-geometry-report-capture="true"] * { + pointer-events: none !important; + animation: none !important; + transition: none !important; + } + [data-geometry-actions-visible="true"] [data-geometry-hover-action] { + opacity: 1 !important; + pointer-events: none !important; + } + [data-geometry-actions-visible="true"] [data-geometry-hover-rest] { + opacity: 0 !important; + pointer-events: none !important; + } + [data-geometry-report-capture="true"] [data-geometry-capture-reveal="true"] { + opacity: 1 !important; + pointer-events: none !important; + } + `, + }); + const workspace = page.locator( + `[${CHAT_WORKSPACE_GEOMETRY_ATTRIBUTE}="${CHAT_WORKSPACE_GEOMETRY_ANCHORS.workspaceShell}"]` + ); + await page.locator('body').evaluate((element) => { + element.setAttribute('data-geometry-report-capture', 'true'); + element.setAttribute('data-geometry-actions-visible', 'true'); + + const interactiveSelector = + 'button, [role="button"], a[href], input, select, textarea, summary, [tabindex]:not([tabindex="-1"])'; + for (const candidate of element.querySelectorAll('*')) { + if (candidate.closest('[data-geometry-hover-rest]')) continue; + const style = getComputedStyle(candidate); + const rect = candidate.getBoundingClientRect(); + const occupiesViewport = + rect.width > 0 && + rect.height > 0 && + rect.right >= 0 && + rect.bottom >= 0 && + rect.left <= innerWidth && + rect.top <= innerHeight; + const ownsInteraction = + candidate.matches(interactiveSelector) || candidate.querySelector(interactiveSelector); + if ( + Number.parseFloat(style.opacity) <= 0.01 && + style.display !== 'none' && + style.visibility !== 'hidden' && + occupiesViewport && + ownsInteraction + ) { + candidate.setAttribute('data-geometry-capture-reveal', 'true'); + } + } + }); + if ((await workspace.count()) > 0) { + await workspace.evaluate((element) => { + element.setAttribute('data-geometry-report-capture', 'true'); + element.setAttribute('data-geometry-actions-visible', 'true'); + }); + } + + const revealedSurfaces = page.locator('[data-geometry-capture-reveal="true"]'); + await revealedSurfaces.count(); +} + +export type GeometryReplayCapture = Readonly<{ + storyId: string; + storyGlobals?: string; + viewport: Readonly<{ width: number; height: number }>; + deviceScaleFactor: number; + dimensions?: Readonly<{ theme?: string }>; + expectedText?: string; + readySelectors?: readonly string[]; +}>; + +/** + * A context is only worth opening once per SCALE and THEME. Device scale and + * colour scheme are fixed when a context is created, but a viewport is not, and + * a fresh context starts with a cold HTTP cache — so one context per capture + * re-downloads and re-parses the whole Storybook bundle every time, which is + * minutes of wall clock and the reason a story can miss its readiness deadline. + */ +export function geometryReplayContextKey(capture: GeometryReplayCapture): string { + return `${capture.deviceScaleFactor}|${capture.dimensions?.theme === 'dark' ? 'dark' : 'light'}`; +} + +export async function openGeometryReplayContext( + browser: Browser, + capture: GeometryReplayCapture, + blockedRequests: string[] +): Promise { + const context = await browser.newContext({ + viewport: capture.viewport, + deviceScaleFactor: capture.deviceScaleFactor, + reducedMotion: 'reduce', + colorScheme: capture.dimensions?.theme === 'dark' ? 'dark' : 'light', + }); + await context.route(/https?:\/\//, async (route) => { + const url = new URL(route.request().url()); + if (url.origin === geometryStorybookOrigin) { + await route.continue(); + return; + } + blockedRequests.push(url.href); + await route.abort('blockedbyclient'); + }); + return context; +} + +/** + * Show the story a capture came from, at that capture's viewport and settled + * the same way every other pass settles it. Shared by the gate walk, the + * `--after` replay and the Y cards, so a card, a repair image and a ratchet + * measurement are never taken against a differently composed page. + */ +export async function showGeometryCaptureStory( + context: BrowserContext, + capture: GeometryReplayCapture +): Promise { + // A fresh PAGE per capture, inside the shared context: the context keeps the + // HTTP cache warm, and a page that has loaded a dozen stories in a row runs + // its renderer out of memory and crashes mid-navigation. + const page = await context.newPage(); + await page.setViewportSize(capture.viewport); + const response = await page.goto( + `${geometryStorybookOrigin}/iframe.html?id=${capture.storyId}&viewMode=story${ + capture.storyGlobals ? `&globals=${capture.storyGlobals}` : '' + }` + ); + if (!response?.ok()) throw new Error(`Story capture failed: ${capture.storyId}`); + if (capture.storyId.includes('sessionconversationpage')) { + await waitForSessionConversationStory(page); + } + await enableGeometryCaptureMode(page); + for (const selector of capture.readySelectors ?? []) { + await page.locator(selector).first().waitFor({ state: 'attached', timeout: 60_000 }); + } + if (capture.expectedText) { + await page + .getByText(capture.expectedText, { exact: false }) + .first() + .waitFor({ state: 'visible', timeout: 30_000 }); + } + if (capture.dimensions?.theme === 'dark') { + const isDark = await page.evaluate(() => document.documentElement.classList.contains('dark')); + if (!isDark) throw new Error(`${capture.storyId} did not apply the dark theme global`); + } + if (capture.storyId.startsWith('geometry-chatworkspace--')) { + await measureSettledChatWorkspace(page); + } else { + await page.evaluate(async () => { + await document.fonts.ready; + await new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }); + }); + } + return page; +} + +export type GeometryPlanObservation = Readonly<{ + entry: GeometryCapturePlanEntry; + railDiscovery: readonly BrowserAlignmentRailDiscoveryScope[]; + semanticAlignments?: readonly BrowserSemanticAlignmentEntry[]; + semanticBaselines?: readonly BrowserSemanticBaselineEntry[]; +}>; + +/** + * Run discovery on an already-settled page for one plan entry. The gate and the + * report both come through here, so neither can pass discovery a scope, a + * capture id or a surface family the other does not. + */ +export async function observeGeometryPlanEntry( + page: Page, + entry: GeometryCapturePlanEntry, + observationCache?: GeometryObservationCache +): Promise { + const railDiscovery = await discoverChatWorkspaceAlignmentRails(page, { + aggregateScopes: entry.aggregateScopes, + captureId: entry.captureId, + surfaceFamily: entry.area, + ...(observationCache ? { observationCache } : {}), + }); + if (!entry.semanticObservations) return { entry, railDiscovery }; + return { + entry, + railDiscovery, + semanticAlignments: await auditChatWorkspaceSemanticAlignments(page), + semanticBaselines: await auditChatWorkspaceSemanticBaselines(page), + }; +} + +/** One capture record, assembled the same way for the report and for the gate. */ +export function buildGeometryCapture( + observation: GeometryPlanObservation, + screenshot = '' +): GeometryCapture { + const { entry, railDiscovery } = observation; + const boxModelNodes = Object.assign( + {}, + ...railDiscovery.map((scope) => scope.capturedScope.boxModelNodes ?? {}) + ); + const semanticMembers = ( + members: BrowserSemanticAlignmentEntry['members'] | BrowserSemanticBaselineEntry['members'] + ) => + members.map((member) => ({ + name: member.name, + coordinate: member.coordinate, + ...(member.primitiveId ? { primitiveId: member.primitiveId } : {}), + rect: member.rect, + })); + const splitGroupLabel = (groupLabel: string) => { + const [group, instance] = groupLabel.split(' · '); + return { group: group ?? groupLabel, instance: instance ?? null }; + }; + return { + captureId: entry.captureId, + surfaceFamily: entry.area, + surface: entry.surface, + storyId: entry.storyId, + viewport: entry.viewport, + deviceScaleFactor: entry.deviceScaleFactor, + dimensions: entry.dimensions, + screenshot, + scopes: railDiscovery.map((scope) => { + const { boxModelNodes: _boxModelNodes, ...capturedScope } = scope.capturedScope; + return capturedScope; + }), + boxModelNodes, + ...(observation.semanticAlignments + ? { + semanticAlignments: observation.semanticAlignments.map((item) => ({ + ...splitGroupLabel(item.groupLabel), + axis: item.axis, + anchor: item.anchor, + status: item.status, + line: item.line, + members: semanticMembers(item.members), + })), + // The baseline rules travel with the alignment rules so + // marker-removal readiness asks one question of every marker. + semanticBaselines: (observation.semanticBaselines ?? []).map((item) => ({ + ...splitGroupLabel(item.groupLabel), + axis: 'y' as const, + anchor: 'text-baseline' as const, + status: item.status, + line: item.line, + members: semanticMembers(item.members), + })), + } + : {}), + }; +} + +/** + * Walk the plan and return the capture artifact. `onObserved` is where the + * report takes its screenshots; the gate passes nothing, which is the whole + * difference between a 40-minute review run and a CI ratchet. + */ +export async function runGeometryCapturePlan( + browser: Browser, + entries: readonly GeometryCapturePlanEntry[], + options: Readonly<{ + observationCache?: GeometryObservationCache; + blockedRequests?: string[]; + onObserved?: (page: Page, observation: GeometryPlanObservation) => Promise; + }> = {} +): Promise { + const blockedRequests = options.blockedRequests ?? []; + const contexts = new Map(); + const captures: GeometryCapture[] = []; + try { + for (const entry of entries) { + const contextKey = geometryReplayContextKey(entry); + let context = contexts.get(contextKey); + if (!context) { + context = await openGeometryReplayContext(browser, entry, blockedRequests); + contexts.set(contextKey, context); + } + const page = await showGeometryCaptureStory(context, entry); + try { + const observation = await observeGeometryPlanEntry(page, entry, options.observationCache); + captures.push(buildGeometryCapture(observation)); + await options.onObserved?.(page, observation); + } finally { + await page.close(); + } + } + } finally { + for (const context of contexts.values()) await context.close().catch(() => undefined); + } + if (blockedRequests.length > 0 && !options.blockedRequests) { + throw new Error(`Unexpected network requests: ${blockedRequests.join(', ')}`); + } + return { version: 1, captures }; +} + +export type GeometryPipelineArtifacts = Readonly<{ + capture: GeometryCaptureArtifact; + observation: GeometryObservationArtifact; + findings: GeometryFindingArtifact; +}>; + +/** + * `capture → observation → findings`, each stage reading only the previous. + * The report writes the three files; the gate keeps them in memory. Nothing + * else may re-derive a finding, or the ratchet would enforce a number the + * review never saw. + */ +export function runGeometryFindingPipeline( + capture: GeometryCaptureArtifact +): GeometryPipelineArtifacts { + const observation = observeGeometryCaptures(capture); + return { capture, observation, findings: createGeometryFindings(capture, observation) }; +} diff --git a/packages/components/tests/geometry-constraint-system.test.ts b/packages/components/tests/geometry-constraint-system.test.ts index 24fa84880..32215ed38 100644 --- a/packages/components/tests/geometry-constraint-system.test.ts +++ b/packages/components/tests/geometry-constraint-system.test.ts @@ -14,6 +14,9 @@ import { evaluateGeometryContractResolutions, evaluateGeometryContractValues, explainGeometryOffset, + checkGeometryLedgerRatchet, + formatGeometryRatchetViolations, + geometryFindingDevicePixel, geometryFindingLabel, geometryIdentityLocator, geometryRowFamilyKey, @@ -30,6 +33,7 @@ import { type GeometryCapturedCandidate, type GeometryCapturedScope, type GeometryContract, + type GeometryCaptureArtifact, type GeometryFinding, type GeometryLedger, } from '../src/lib/geometry-constraint-system'; @@ -1749,3 +1753,172 @@ describe('marker removal readiness', () => { expect(ready.rules[0]?.matchedMemberCount).toBe(ready.rules[0]?.memberCount); }); }); + +describe('geometry ledger ratchet', () => { + const ratchetFinding = ( + key: string, + offset: number, + captureId = 'workspace:wide-expanded' + ): GeometryFinding => ({ + key, + kind: 'alignment-rail', + surfaceFamily: 'workspace', + locator: locator('Shifted'), + label: `label for ${key}`, + axis: 'x', + anchor: 'inline-end', + offset, + captureCount: 1, + totalCaptureCount: 1, + evidence: [ + { + captureId, + scopeKey: 'sidebar.shell', + coordinate: offset, + line: 0, + normalizedLine: 0.5, + offset, + yStart: 0, + yEnd: 16, + }, + ], + }); + + const captures = (scales: Readonly>): GeometryCaptureArtifact => ({ + version: 1, + captures: Object.entries(scales).map(([captureId, deviceScaleFactor]) => ({ + captureId, + surfaceFamily: 'workspace', + surface: 'Workspace', + storyId: 'story', + viewport: { width: 1440, height: 900 }, + deviceScaleFactor, + screenshot: '', + scopes: [], + })), + }); + + const retina = captures({ 'workspace:wide-expanded': 2 }); + + it('allows a device pixel of drift above the reviewed baseline and no more', () => { + const ledger: GeometryLedger = { + version: 1, + findings: { 'geometry/workspace/a': { status: 'accepted-debt', baseline: { offset: -2 } } }, + }; + + // 2.5 is exactly baseline + one device pixel at 2x; 2.51 is past it. + expect( + checkGeometryLedgerRatchet( + { version: 1, findings: [ratchetFinding('geometry/workspace/a', 2.5)] }, + ledger, + retina + ) + ).toEqual([]); + const regressed = checkGeometryLedgerRatchet( + { version: 1, findings: [ratchetFinding('geometry/workspace/a', 2.51)] }, + ledger, + retina + ); + expect(regressed).toEqual([ + expect.objectContaining({ + kind: 'offset-regression', + key: 'geometry/workspace/a', + label: 'label for geometry/workspace/a', + status: 'accepted-debt', + baseline: -2, + current: 2.51, + tolerance: 0.5, + }), + ]); + // The message has to carry enough to act on without opening the artifact. + const message = formatGeometryRatchetViolations(regressed); + expect(message).toContain('geometry/workspace/a'); + expect(message).toContain('baseline -2.000px → current 2.510px'); + }); + + it('fails a finding no ledger entry reviews and says how to record it', () => { + const violations = checkGeometryLedgerRatchet( + { version: 1, findings: [ratchetFinding('geometry/workspace/unknown', 0.1)] }, + { version: 1, findings: {} }, + retina + ); + + expect(violations).toEqual([ + expect.objectContaining({ kind: 'unreviewed-finding', key: 'geometry/workspace/unknown' }), + ]); + expect(formatGeometryRatchetViolations(violations)).toContain('pnpm geometry:triage'); + }); + + it('skips ignored entries and leaves promoted ones to the contract check', () => { + const ledger: GeometryLedger = { + version: 1, + findings: { + 'geometry/workspace/ignored': { status: 'ignored', baseline: { offset: 0 } }, + 'geometry/workspace/promoted': { + status: 'promoted', + baseline: { offset: 0 }, + contract: { + name: 'workspace.rail', + story: 'geometry--landing', + members: [locator('First'), locator('Second')], + axis: 'x', + anchor: 'inline-end', + space: 'ink', + tolerance: 0.5, + }, + }, + }, + }; + + expect( + checkGeometryLedgerRatchet( + { + version: 1, + findings: [ + ratchetFinding('geometry/workspace/ignored', 40), + ratchetFinding('geometry/workspace/promoted', 40), + ], + }, + ledger, + retina + ) + ).toEqual([]); + }); + + it('holds an entry baselined at zero to zero', () => { + const ledger: GeometryLedger = { + version: 1, + findings: { 'geometry/workspace/a': { status: 'accepted-debt', baseline: { offset: 0 } } }, + }; + + expect( + checkGeometryLedgerRatchet( + { version: 1, findings: [ratchetFinding('geometry/workspace/a', 1.2)] }, + ledger, + retina + ) + ).toEqual([expect.objectContaining({ kind: 'offset-regression', baseline: 0 })]); + }); + + it('takes its tolerance from the coarsest capture the finding was measured on', () => { + const finding: GeometryFinding = { + ...ratchetFinding('geometry/workspace/a', 1), + evidence: [ + { ...ratchetFinding('geometry/workspace/a', 1).evidence[0]!, captureId: 'retina' }, + { ...ratchetFinding('geometry/workspace/a', 1).evidence[0]!, captureId: 'standard' }, + ], + }; + const mixed = captures({ retina: 2, standard: 1 }); + + expect(geometryFindingDevicePixel(finding, mixed)).toBe(1); + // A finding merged across 1x and 2x is only as precise as the 1x capture. + expect( + checkGeometryLedgerRatchet( + { version: 1, findings: [finding] }, + { version: 1, findings: { 'geometry/workspace/a': { status: 'accepted-debt', baseline: { offset: 0 } } } }, + mixed + ) + ).toEqual([]); + }); + +}); From ae2bae2a19359699dbbffe0fa8ceb0bee6288c94 Mon Sep 17 00:00:00 2001 From: Leeeon233 Date: Thu, 3 Sep 2026 09:46:08 +0000 Subject: [PATCH 2/4] feat(components): split accepted-debt and close the loop with a fixed status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `accepted-debt` carried two different decisions under one word — "this is wrong and we will fix it later" and "this is not a defect" — so any queue built from it mixed work with non-work, and 74 of the ledger's 76 entries sat in it. And there was no way to record the other end of the flywheel at all: a finding whose offset had been driven back to its line stayed `accepted-debt` at its old, loose baseline, so the fix it just received was not protected by anything. - `GeometryLedgerStatus` becomes `new | debt | wont-fix | fixed | ignored | promoted`. All 74 existing entries migrate to `debt`; the distinction between debt and wont-fix is a review, and guessing it here would record decisions nobody made. `triage` records `debt` for the same reason. - `fixed` is baselined at what was measured, which makes it the strictest entry in the file: the ratchet then treats a reopened fix as the regression it is. - Only `promoted` still compiles into a contract; `debt`, `wont-fix`, `fixed` and `ignored` compile into nothing, now stated in code and in `tests/e2e/AGENTS.md`. - Add `pnpm geometry:verify-fix `. It reruns the ratchet gate itself — same capture plan, no screenshots — so "did this fix land" and "did it cost something elsewhere" are one measurement. A finding measured back inside one device pixel, or gone from findings entirely, becomes `fixed` at its new baseline; anything else prints why and leaves the ledger alone. The decision is made in TypeScript (`verifyGeometryFixes`) beside the ratchet, and the script only applies the file the gate writes, exactly as triage does. - A `promoted` finding is refused: it is gated EXACTLY by its contract, and moving it to `fixed` would stop compiling that contract and quietly drop the tightest rule in the file. Retiring a contract stays a deliberate step. The report's status chips and default filter follow: the work queue is new + changed + css-defect + promoted, minus `wont-fix` (a decision) and `fixed` (already done). Verified: `tsgo --noEmit`, unit tests (111 passed), lint, format. Model: claude-opus-5[1m] --- package.json | 1 + packages/components/geometry-ledger.json | 148 +++++++++--------- packages/components/package.json | 1 + .../chat-workspace-geometry-report.html | 31 +++- .../scripts/triage-geometry-findings.mjs | 9 +- .../scripts/verify-geometry-fix.mjs | 122 +++++++++++++++ .../src/lib/geometry-constraint-system.ts | 133 +++++++++++++++- packages/components/tests/e2e/AGENTS.md | 13 +- .../tests/e2e/chat-workspace-geometry.spec.ts | 31 +++- .../tests/geometry-constraint-system.test.ts | 138 ++++++++++++++-- 10 files changed, 517 insertions(+), 110 deletions(-) create mode 100644 packages/components/scripts/verify-geometry-fix.mjs diff --git a/package.json b/package.json index 0461b7149..2dc09d320 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "test:ci": "corepack pnpm -r --workspace-concurrency=2 --filter \"!@lody/electron\" --filter \"!acp-extension-codex\" --filter \"!acp-extension-claude\" run test --maxWorkers=2 && corepack pnpm --filter @lody/electron run test && corepack pnpm test:geometry", "test:geometry": "corepack pnpm --filter @lody/components test:geometry", "geometry:triage": "node packages/components/scripts/triage-geometry-findings.mjs", + "geometry:verify-fix": "corepack pnpm --filter @lody/components geometry:verify-fix", "test:watch": "corepack pnpm -r run test:watch", "test:coverage": "corepack pnpm -r run test:coverage", "lint:fast": "oxlint --quiet --ignore-pattern packages/acp-extension-kimi", diff --git a/packages/components/geometry-ledger.json b/packages/components/geometry-ledger.json index 3c668169d..6864c2841 100644 --- a/packages/components/geometry-ledger.json +++ b/packages/components/geometry-ledger.json @@ -9,7 +9,7 @@ }, "findings": { "geometry/right-sidebar/11pscs0": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 2 }, @@ -21,7 +21,7 @@ } }, "geometry/right-sidebar/171u7h8": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1 }, @@ -33,7 +33,7 @@ } }, "geometry/right-sidebar/177rmie": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 2 }, @@ -45,7 +45,7 @@ } }, "geometry/right-sidebar/1e5wvsl": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 6.5 }, @@ -57,7 +57,7 @@ } }, "geometry/right-sidebar/1ls7j8z": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 0.75 }, @@ -69,7 +69,7 @@ } }, "geometry/right-sidebar/1mxwyl5": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1.5 }, @@ -81,7 +81,7 @@ } }, "geometry/right-sidebar/1r5c99a": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1 }, @@ -93,7 +93,7 @@ } }, "geometry/right-sidebar/1simiqu": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -105,7 +105,7 @@ } }, "geometry/right-sidebar/1vnwrqv": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 0.75 }, @@ -117,7 +117,7 @@ } }, "geometry/right-sidebar/57h0zw": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1.5 }, @@ -129,7 +129,7 @@ } }, "geometry/right-sidebar/6ijj9r": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1 }, @@ -141,7 +141,7 @@ } }, "geometry/right-sidebar/6q3vy3": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1.5 }, @@ -153,7 +153,7 @@ } }, "geometry/right-sidebar/8s2gxh": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 2 }, @@ -165,7 +165,7 @@ } }, "geometry/right-sidebar/b0m4s": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 4 }, @@ -177,7 +177,7 @@ } }, "geometry/right-sidebar/b8dyq5": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 0.75 }, @@ -189,7 +189,7 @@ } }, "geometry/right-sidebar/d3cw9u": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 0.75 }, @@ -201,7 +201,7 @@ } }, "geometry/right-sidebar/d4c4q": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 0.75 }, @@ -213,7 +213,7 @@ } }, "geometry/right-sidebar/daf70p": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1.5 }, @@ -225,7 +225,7 @@ } }, "geometry/right-sidebar/gvz5jb": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -0.75 }, @@ -237,7 +237,7 @@ } }, "geometry/right-sidebar/hdgowf": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 2 }, @@ -249,7 +249,7 @@ } }, "geometry/right-sidebar/im7aci": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 0.75 }, @@ -261,7 +261,7 @@ } }, "geometry/right-sidebar/kbsjam": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -273,7 +273,7 @@ } }, "geometry/right-sidebar/l4kgf": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 0.75 }, @@ -285,7 +285,7 @@ } }, "geometry/right-sidebar/nerzug": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1 }, @@ -297,7 +297,7 @@ } }, "geometry/right-sidebar/u9ipw4": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 0.75 }, @@ -309,7 +309,7 @@ } }, "geometry/right-sidebar/vvedhu": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -0.75 }, @@ -321,7 +321,7 @@ } }, "geometry/right-sidebar/x7rij5": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1 }, @@ -333,7 +333,7 @@ } }, "geometry/right-sidebar/xrp5i4": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 0.75 }, @@ -345,7 +345,7 @@ } }, "geometry/session/1aje3gt": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1.25 }, @@ -357,7 +357,7 @@ } }, "geometry/session/1euuz5v": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -3 }, @@ -369,7 +369,7 @@ } }, "geometry/session/1p4pgog": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -3 }, @@ -381,7 +381,7 @@ } }, "geometry/session/1t257v2": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -393,7 +393,7 @@ } }, "geometry/session/1xffq47": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -405,7 +405,7 @@ } }, "geometry/session/1y86m8m": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 25 }, @@ -417,7 +417,7 @@ } }, "geometry/session/37kibv": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -3 }, @@ -429,7 +429,7 @@ } }, "geometry/session/5875hb": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 2.5 }, @@ -441,7 +441,7 @@ } }, "geometry/session/88iul9": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 9.5 }, @@ -453,7 +453,7 @@ } }, "geometry/session/9slq61": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 3 }, @@ -465,7 +465,7 @@ } }, "geometry/session/i4jvzg": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -477,7 +477,7 @@ } }, "geometry/session/le2uqr": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -3 }, @@ -489,7 +489,7 @@ } }, "geometry/session/nu8q04": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1.25 }, @@ -501,7 +501,7 @@ } }, "geometry/session/pothvh": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1.25 }, @@ -513,7 +513,7 @@ } }, "geometry/session/r7rfec": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1.5 }, @@ -525,7 +525,7 @@ } }, "geometry/session/wy7fax": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 12.5 }, @@ -537,7 +537,7 @@ } }, "geometry/session/y2yn1n": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1.5 }, @@ -549,7 +549,7 @@ } }, "geometry/session/zhdd1c": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 9.5 }, @@ -561,7 +561,7 @@ } }, "geometry/workspace/14n6pir": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1 }, @@ -573,7 +573,7 @@ } }, "geometry/workspace/1fccih": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -4 }, @@ -585,7 +585,7 @@ } }, "geometry/workspace/1jrw0q7": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -597,7 +597,7 @@ } }, "geometry/workspace/1lbwj6e": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1.5 }, @@ -609,7 +609,7 @@ } }, "geometry/workspace/1my7gbo": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -2.5 }, @@ -621,7 +621,7 @@ } }, "geometry/workspace/1oezpj5": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 5.269230769230769 }, @@ -633,7 +633,7 @@ } }, "geometry/workspace/1p79krt": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -4 }, @@ -645,7 +645,7 @@ } }, "geometry/workspace/1t2nsoi": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -657,7 +657,7 @@ } }, "geometry/workspace/1y8h8dm": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1.375 }, @@ -669,7 +669,7 @@ } }, "geometry/workspace/1ysr2o": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -0.75 }, @@ -681,7 +681,7 @@ } }, "geometry/workspace/23ysa1": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1.5 }, @@ -693,7 +693,7 @@ } }, "geometry/workspace/3qj791": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -4.076923076923077 }, @@ -705,7 +705,7 @@ } }, "geometry/workspace/4gx82n": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 2.25 }, @@ -717,7 +717,7 @@ } }, "geometry/workspace/4ik93p": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -3.375 }, @@ -729,7 +729,7 @@ } }, "geometry/workspace/5p59kz": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 5 }, @@ -741,7 +741,7 @@ } }, "geometry/workspace/6bmnyr": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1.8125 }, @@ -753,7 +753,7 @@ } }, "geometry/workspace/bu6kch": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1 }, @@ -765,7 +765,7 @@ } }, "geometry/workspace/cqs9ix": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -777,7 +777,7 @@ } }, "geometry/workspace/fi8gpa": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -3.375 }, @@ -789,7 +789,7 @@ } }, "geometry/workspace/h1bkqs": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -4 }, @@ -801,7 +801,7 @@ } }, "geometry/workspace/hg2c2f": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -4 }, @@ -813,7 +813,7 @@ } }, "geometry/workspace/k13zs1": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 3 }, @@ -825,7 +825,7 @@ } }, "geometry/workspace/ky1uql": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 12.5 }, @@ -837,7 +837,7 @@ } }, "geometry/workspace/lkcnsw": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 1.8125 }, @@ -886,7 +886,7 @@ } }, "geometry/workspace/mcouew": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -898,7 +898,7 @@ } }, "geometry/workspace/qltk66": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -1 }, @@ -946,7 +946,7 @@ } }, "geometry/workspace/u25p0b": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": 2.5 }, @@ -958,7 +958,7 @@ } }, "geometry/workspace/w5fbub": { - "status": "accepted-debt", + "status": "debt", "baseline": { "offset": -4 }, diff --git a/packages/components/package.json b/packages/components/package.json index 8f0d8e874..2094a224d 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -64,6 +64,7 @@ "test:e2e:ui": "playwright test --ui", "test:geometry": "playwright test tests/e2e/chat-workspace-geometry.spec.ts --workers=1 --retries=0", "geometry:report": "node scripts/generate-chat-workspace-geometry-report.mjs", + "geometry:verify-fix": "node scripts/verify-geometry-fix.mjs", "geometry:report:open": "node scripts/generate-chat-workspace-geometry-report.mjs --open", "storybook": "storybook dev -p 6006", "build-storybook": "storybook build" diff --git a/packages/components/scripts/templates/chat-workspace-geometry-report.html b/packages/components/scripts/templates/chat-workspace-geometry-report.html index 257cc0087..cae637f3f 100644 --- a/packages/components/scripts/templates/chat-workspace-geometry-report.html +++ b/packages/components/scripts/templates/chat-workspace-geometry-report.html @@ -657,12 +657,23 @@

明确排除

const STATUS_LABELS = { new: '新增', changed: '偏移变化', - 'accepted-debt': '已接受债务', + debt: '待修复债务', + 'wont-fix': '不修复', + fixed: '已修复', promoted: '已提升为 Contract', ignored: '已忽略', unreviewed: '未纳入 ledger', }; - const STATUS_ORDER = ['new', 'changed', 'accepted-debt', 'promoted', 'ignored', 'unreviewed']; + const STATUS_ORDER = [ + 'new', + 'changed', + 'debt', + 'wont-fix', + 'fixed', + 'promoted', + 'ignored', + 'unreviewed', + ]; const CLASSIFICATION_ORDER = ['css-defect', 'structural', 'optical-residual', '']; const CLASSIFICATION_LABELS = { 'css-defect': 'CSS 缺陷', @@ -674,11 +685,15 @@

明确排除

// The default view is the work queue: what is new, what moved, what the // box model says is a repairable CSS defect, and what is already promoted // to a contract — a promoted rule is the one a regression would break. + // A `wont-fix` finding is a decision, and a `fixed` one is already done: + // neither is work, so neither belongs in the queue however it classifies. const isDefaultDetail = (detail) => - detailStatus(detail) === 'new' || - detailStatus(detail) === 'changed' || - detailStatus(detail) === 'promoted' || - detail.classification === 'css-defect'; + detailStatus(detail) !== 'wont-fix' && + detailStatus(detail) !== 'fixed' && + (detailStatus(detail) === 'new' || + detailStatus(detail) === 'changed' || + detailStatus(detail) === 'promoted' || + detail.classification === 'css-defect'); const statusCounts = new Map(); for (const detail of report.details) { @@ -698,7 +713,9 @@

明确排除

`status:${status}`, STATUS_LABELS[status], statusCounts.get(status) ?? 0, - status === 'new' || status === 'changed' ? 'dot--candidate' : 'dot--insufficient', + status === 'new' || status === 'changed' || status === 'debt' + ? 'dot--candidate' + : 'dot--insufficient', ]), ['css-defect', 'CSS 缺陷', cssDefects.length, 'dot--fail'], ['structural', '结构性', structural.length, 'dot--candidate'], diff --git a/packages/components/scripts/triage-geometry-findings.mjs b/packages/components/scripts/triage-geometry-findings.mjs index 7224d46e6..4ec26cdbd 100644 --- a/packages/components/scripts/triage-geometry-findings.mjs +++ b/packages/components/scripts/triage-geometry-findings.mjs @@ -7,9 +7,12 @@ const argumentsList = process.argv.slice(2); const requestedReport = argumentsList.find((argument) => !argument.startsWith('--')); const statusArgument = argumentsList.find((argument) => argument.startsWith('--status=')); const ledgerArgument = argumentsList.find((argument) => argument.startsWith('--ledger=')); -const status = statusArgument?.slice('--status='.length) ?? 'accepted-debt'; -if (status !== 'new' && status !== 'accepted-debt') { - throw new Error('--status must be new or accepted-debt'); +// New entries land on `debt`, never `wont-fix`. Telling "we will fix this" from +// "this is not a defect" IS the review; a tool that guessed would be recording +// a decision nobody made. +const status = statusArgument?.slice('--status='.length) ?? 'debt'; +if (status !== 'new' && status !== 'debt') { + throw new Error('--status must be new or debt'); } const reportDirectory = path.resolve(packageRoot, requestedReport ?? 'geometry-report'); diff --git a/packages/components/scripts/verify-geometry-fix.mjs b/packages/components/scripts/verify-geometry-fix.mjs new file mode 100644 index 000000000..3072709ce --- /dev/null +++ b/packages/components/scripts/verify-geometry-fix.mjs @@ -0,0 +1,122 @@ +import { readFile, writeFile } from 'node:fs/promises'; +import { spawn } from 'node:child_process'; +import { createServer } from 'node:net'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const packageRoot = fileURLToPath(new URL('..', import.meta.url)); +const argumentsList = process.argv.slice(2); +const positional = argumentsList.filter((argument) => !argument.startsWith('--')); +const ledgerArgument = argumentsList.find((argument) => argument.startsWith('--ledger=')); +const [requestedOutput, ...requestedKeys] = positional; + +if (!requestedOutput || requestedKeys.length === 0) { + throw new Error('Usage: pnpm geometry:verify-fix [--ledger=]'); +} + +const outputDirectory = path.resolve(packageRoot, requestedOutput); +const ledgerPath = ledgerArgument + ? path.resolve(packageRoot, ledgerArgument.slice('--ledger='.length)) + : path.join(packageRoot, 'geometry-ledger.json'); + +function portIsAvailable(port) { + return new Promise((resolve) => { + const server = createServer(); + server.once('error', () => resolve(false)); + server.listen(port, '127.0.0.1', () => server.close(() => resolve(true))); + }); +} + +async function resolvePort() { + if (process.env.GEOMETRY_REPORT_PORT) return Number(process.env.GEOMETRY_REPORT_PORT); + for (let port = 6100; port < 6120; port += 1) { + if (await portIsAvailable(port)) return port; + } + throw new Error('No available Storybook port in the range 6100-6119'); +} + +function run(command, arguments_, options = {}) { + return new Promise((resolve, reject) => { + const child = spawn(command, arguments_, { + cwd: packageRoot, + env: process.env, + stdio: 'inherit', + ...options, + }); + child.once('error', reject); + child.once('exit', (code, signal) => { + if (code === 0) { + resolve(); + return; + } + reject( + new Error( + `${command} exited with ${signal ? `signal ${signal}` : `status ${String(code)}`}` + ) + ); + }); + }); +} + +// The ratchet gate IS the verification run: it walks the same capture plan with +// no screenshots and fails if any OTHER finding regressed, so "did this fix +// land" and "did it cost something elsewhere" are one measurement rather than +// two runs that could disagree. The decision itself is made in TypeScript +// beside the ratchet; this script only applies the ledger it writes out. +const port = await resolvePort(); +const pnpmCommand = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm'; +await run( + pnpmCommand, + [ + 'exec', + 'playwright', + 'test', + 'tests/e2e/chat-workspace-geometry.spec.ts', + '--grep', + 'reviewed ledger baseline', + '--workers=1', + '--retries=0', + '--reporter=line', + ], + { + env: { + ...process.env, + GEOMETRY_PIPELINE_OUTPUT_DIR: outputDirectory, + GEOMETRY_VERIFY_FIX_KEYS: requestedKeys.join(','), + PLAYWRIGHT_PORT: String(port), + PLAYWRIGHT_BASE_URL: `http://127.0.0.1:${port}`, + VITE_PREVIEW_PUBLIC_BASE_DOMAIN: + process.env.VITE_PREVIEW_PUBLIC_BASE_DOMAIN ?? 'local.invalid', + ...(process.platform === 'darwin' && process.env.PLAYWRIGHT_USE_SYSTEM_CHROME == null + ? { PLAYWRIGHT_USE_SYSTEM_CHROME: '1' } + : {}), + }, + } +); + +const verificationPath = path.join(outputDirectory, 'fix-verification.json'); +const verification = JSON.parse(await readFile(verificationPath, 'utf8')); +if (verification.version !== 1 || !Array.isArray(verification.verifications)) { + throw new Error(`Unsupported geometry fix verification: ${verificationPath}`); +} + +for (const item of verification.verifications) { + const measured = item.resolved + ? 'no longer reported by any rail' + : `|offset| ${Math.abs(item.offset).toFixed(3)}px`; + console.log( + `${item.passed ? 'FIXED' : 'STILL OFF'} ${item.key}\n ${item.label}\n ${measured}${ + item.reason ? `\n ${item.reason}` : '' + }` + ); +} + +if (!verification.passed) { + console.error('\nGeometry ledger unchanged: not every requested finding is fixed.'); + process.exitCode = 1; +} else { + await writeFile(ledgerPath, `${JSON.stringify(verification.ledger, null, 2)}\n`, 'utf8'); + console.log( + `\nGeometry ledger: marked ${verification.verifications.length} finding(s) fixed in ${ledgerPath}` + ); +} diff --git a/packages/components/src/lib/geometry-constraint-system.ts b/packages/components/src/lib/geometry-constraint-system.ts index 1c301e688..ae4729b86 100644 --- a/packages/components/src/lib/geometry-constraint-system.ts +++ b/packages/components/src/lib/geometry-constraint-system.ts @@ -378,16 +378,34 @@ export type GeometryFindingArtifact = Readonly<{ findings: readonly GeometryFinding[]; }>; -export type GeometryLedgerStatus = 'new' | 'accepted-debt' | 'ignored' | 'promoted'; - /** - * Statuses the ratchet holds to their reviewed baseline. `ignored` opts out — - * that is what ignoring one means — and `promoted` is left to the contract - * check that already gates it exactly, rather than gated twice and loosely. + * How a human reviewed one finding. + * + * `accepted-debt` used to carry two different decisions under one word — "this + * is wrong and we will fix it later" and "this is not a defect" — so a queue + * built from it mixed work with non-work. They are `debt` and `wont-fix` now. + * `fixed` is the other end of the same flywheel: a finding measured back to + * zero, re-baselined there, and therefore held to the STRICTEST ratchet of + * all — reopening it is a regression, not a new review. + * + * Only `promoted` compiles into a contract. `new`, `debt`, `wont-fix`, `fixed` + * and `ignored` compile into nothing; what separates them is the ratchet, + * which skips `ignored` and holds every other status to its baseline. */ +export type GeometryLedgerStatus = + | 'new' + | 'debt' + | 'wont-fix' + | 'fixed' + | 'ignored' + | 'promoted'; + +/** Statuses the ratchet holds to their reviewed baseline. */ export const GEOMETRY_RATCHETED_LEDGER_STATUSES: readonly GeometryLedgerStatus[] = [ 'new', - 'accepted-debt', + 'debt', + 'wont-fix', + 'fixed', ]; /** @@ -2513,11 +2531,15 @@ export function diffGeometryFindings( }; } -/** Record previously unseen findings without moving an existing baseline. */ +/** + * Record previously unseen findings without moving an existing baseline. New + * entries land on `debt`, never `wont-fix`: telling those two apart is the + * review, and a tool that guessed it would be recording a decision nobody made. + */ export function triageGeometryFindings( artifact: GeometryFindingArtifact, ledger: GeometryLedger, - status: Extract = 'accepted-debt' + status: Extract = 'debt' ): GeometryLedger { const findings: Record = { ...ledger.findings }; const reviewedIdentity = (finding: GeometryFinding): GeometryReviewedIdentity => ({ @@ -2914,6 +2936,94 @@ export function checkGeometryLedgerRatchet( }); } +export type GeometryFixVerification = Readonly<{ + key: string; + label: string; + /** Absent from findings: the rail no longer reports this element at all. */ + resolved: boolean; + offset: number; + tolerance: number; + passed: boolean; + reason?: string; +}>; + +/** + * Close the flywheel: a finding whose offset is now within one device pixel of + * its line is `fixed`, and its baseline moves to what was just measured — which + * makes it the STRICTEST entry in the ledger from then on. Everything else is + * reported and changes nothing: a verification that re-baselined a failure + * would be a ratchet that only ever loosens. + */ +export function verifyGeometryFixes( + artifact: GeometryFindingArtifact, + ledger: GeometryLedger, + captures: GeometryCaptureArtifact, + keys: readonly string[] +): Readonly<{ + verifications: readonly GeometryFixVerification[]; + ledger: GeometryLedger; +}> { + const byKey = new Map(artifact.findings.map((finding) => [finding.key, finding])); + const findings: Record = { ...ledger.findings }; + const verifications = keys.map((key): GeometryFixVerification => { + const entry = ledger.findings[key]; + const finding = byKey.get(key); + const label = finding?.label ?? entry?.identity?.label ?? key; + const refuse = (reason: string): GeometryFixVerification => ({ + key, + label, + resolved: !finding, + offset: finding?.offset ?? 0, + tolerance: 0, + passed: false, + reason, + }); + if (!entry) return refuse('no ledger entry reviews this finding'); + // A promoted finding is gated EXACTLY by its contract. Moving it to `fixed` + // would leave that contract uncompiled and silently drop the tightest rule + // in the file, so retiring the contract has to be the deliberate step. + if (entry.status === 'promoted') { + return refuse('a promoted finding is gated by its contract; retire the contract first'); + } + if (!finding) { + // The element is no longer measured off any line: nothing left to allow. + findings[key] = { ...entry, status: 'fixed', baseline: { offset: 0 } }; + return { key, label, resolved: true, offset: 0, tolerance: 0, passed: true }; + } + const tolerance = geometryFindingDevicePixel(finding, captures); + const passed = Math.abs(finding.offset) <= tolerance; + if (passed) { + findings[key] = { ...entry, status: 'fixed', baseline: { offset: finding.offset } }; + } + return { + key, + label, + resolved: false, + offset: finding.offset, + tolerance, + passed, + ...(passed + ? {} + : { + reason: `|offset| ${Math.abs(finding.offset).toFixed(3)}px exceeds one device pixel (${tolerance.toFixed(3)}px)`, + }), + }; + }); + if (verifications.some((verification) => !verification.passed)) { + return { verifications, ledger }; + } + return { + verifications, + ledger: { + version: 1, + ...(ledger.tokens ? { tokens: ledger.tokens } : {}), + findings: Object.fromEntries( + Object.entries(findings).sort(([left], [right]) => left.localeCompare(right)) + ), + }, + }; +} + export function formatGeometryRatchetViolations( violations: readonly GeometryRatchetViolation[] ): string { @@ -2936,6 +3046,13 @@ export function formatGeometryRatchetViolations( .join('\n\n'); } +/** + * Only `promoted` compiles. `new`, `debt`, `wont-fix`, `fixed` and `ignored` + * produce no contract at all — a status is a REVIEW, and a review that started + * gating the product without a human writing the contract would be a rule + * nobody wrote. A `fixed` finding is held to its own near-zero baseline by the + * ratchet; promoting it to a contract is a separate, deliberate step. + */ export function compileGeometryContracts(ledger: GeometryLedger): GeometryContractArtifact { const contracts = Object.entries(ledger.findings).flatMap(([findingKey, entry]) => { if (entry.status !== 'promoted') return []; diff --git a/packages/components/tests/e2e/AGENTS.md b/packages/components/tests/e2e/AGENTS.md index 20f78cd0b..699a46165 100644 --- a/packages/components/tests/e2e/AGENTS.md +++ b/packages/components/tests/e2e/AGENTS.md @@ -83,8 +83,8 @@ new. Pairing is one-to-one; an entry without a recorded identity stays resolved. `css-defect` / `optical-residual` / `structural` and `dimensionSensitivity` are arithmetic over the evidence explanations and never alter a verdict; thresholds, terms and axes sit beside the code ([src/lib](../../src/lib/AGENTS.md)). Review lives in checked-in -`geometry-ledger.json`; `geometry-contracts.json` compiles only `promoted` entries, each -declaring `ink` or `layout-box`. +`geometry-ledger.json`; `geometry-contracts.json` compiles ONLY `promoted` entries, each +declaring `ink` or `layout-box` — `new`, `debt`, `wont-fix`, `fixed`, `ignored` compile none. - A baseline is EXECUTED, not printed: the gate reruns the pipeline over the whole capture plan with no screenshots, and fails when a finding's |offset| passes |baseline| plus one @@ -92,6 +92,10 @@ declaring `ink` or `layout-box`. (`geometry:triage`, like a lockfile). `ignored` opts out; `promoted` belongs to the contract check. Offsets are means over the WHOLE plan, so a baseline belongs to the platform that recorded it: re-baseline where CI runs, never trim the plan for speed. +- `debt` and `wont-fix` are two decisions, not one word; `triage` records `debt` rather + than guess. `geometry:verify-fix ` reruns that gate and only + then moves a finding back inside one device pixel to `fixed` at its new baseline, the + strictest entry there is. - Two contract members never cover one element twice; member resolution, the ink witness and named tokens (the ledger records only the `--spacing-*` property): see [support](support/AGENTS.md). Relations are a small deterministic algebra there; widen @@ -109,8 +113,9 @@ Discovery or proposal presence is never a report assertion; coverage: findings or replacing evidence. Replay: [support](support/AGENTS.md). - Steady state, not delta: every finding gets a card grouped by ledger status and classification, with baseline vs current offset, capture count, dimension sensitivity and - repair text. Chips filter both, default new + changed + css-defect + promoted; the meta - line totals new/changed/resolved. One JSON payload, one renderer, images as files. + repair text. Chips filter both, default new + changed + css-defect + promoted minus + wont-fix and fixed; the meta line totals new/changed/resolved. One JSON payload, one + renderer, images as files. - Violation images label each deviating member in place with role, physical direction, measured offset, actual anchor and a leader to it. A Y card comes from the FINISHED findings, never a second pipeline printing another number: each annotation IS that diff --git a/packages/components/tests/e2e/chat-workspace-geometry.spec.ts b/packages/components/tests/e2e/chat-workspace-geometry.spec.ts index fbb7cde18..4a226bbb2 100644 --- a/packages/components/tests/e2e/chat-workspace-geometry.spec.ts +++ b/packages/components/tests/e2e/chat-workspace-geometry.spec.ts @@ -16,6 +16,7 @@ import { checkGeometryLedgerRatchet, compileGeometryContracts, formatGeometryRatchetViolations, + verifyGeometryFixes, type GeometryContractArtifact, type GeometryLedger, type GeometryObservationCache, @@ -40,10 +41,19 @@ import { /** * Set to write `capture.json` / `observation.json` / `findings.json` beside the - * ratchet run. CI does not need them and does not set it, so the gate stays a - * measurement with no artifacts. + * ratchet run. `pnpm geometry:verify-fix` reads them; CI does not need them and + * does not set this, so the gate stays a measurement with no artifacts. */ const pipelineOutputDirectory = process.env.GEOMETRY_PIPELINE_OUTPUT_DIR; +/** + * `pnpm geometry:verify-fix` sets this. The DECISION — is this finding fixed, + * and what does the ledger become — is made here in TypeScript, beside the + * ratchet it must not regress; the script only applies the file it writes. + */ +const verifyFixKeys = (process.env.GEOMETRY_VERIFY_FIX_KEYS ?? '') + .split(',') + .map((key) => key.trim()) + .filter(Boolean); const STORY_IDS = { expanded: 'geometry-chatworkspace--expanded-sidebar', @@ -124,6 +134,23 @@ test('every measured geometry finding stays inside its reviewed ledger baseline' `Geometry ledger ratchet failed:\n\n${formatGeometryRatchetViolations(violations)}` ).toEqual([]); + if (!pipelineOutputDirectory || verifyFixKeys.length === 0) return; + const keys = [...new Set(verifyFixKeys)]; + const verification = verifyGeometryFixes(findings, ledger, capture, keys); + await writeFile( + path.join(pipelineOutputDirectory, 'fix-verification.json'), + `${JSON.stringify( + { + version: 1, + passed: verification.verifications.every((item) => item.passed), + verifications: verification.verifications, + ledger: verification.ledger, + }, + null, + 2 + )}\n`, + 'utf8' + ); }); for (const verificationCase of CHAT_WORKSPACE_GEOMETRY_SPEC.verificationCases) { diff --git a/packages/components/tests/geometry-constraint-system.test.ts b/packages/components/tests/geometry-constraint-system.test.ts index 32215ed38..9e3d0cf4c 100644 --- a/packages/components/tests/geometry-constraint-system.test.ts +++ b/packages/components/tests/geometry-constraint-system.test.ts @@ -17,6 +17,7 @@ import { checkGeometryLedgerRatchet, formatGeometryRatchetViolations, geometryFindingDevicePixel, + verifyGeometryFixes, geometryFindingLabel, geometryIdentityLocator, geometryRowFamilyKey, @@ -378,7 +379,7 @@ describe('geometry constraint artifacts', () => { }, }, 'geometry/workspace/resolved': { - status: 'accepted-debt', + status: 'debt', baseline: { offset: 2 }, }, }, @@ -425,7 +426,7 @@ describe('geometry constraint artifacts', () => { ); expect(second.findings[finding.key]).toEqual({ - status: 'accepted-debt', + status: 'debt', baseline: { offset: 4 }, identity: { label: 'Shifted', @@ -1558,7 +1559,7 @@ describe('re-keying a reviewed finding', () => { version: 1, findings: { 'geometry/workspace/old-machine': { - status: 'accepted-debt', + status: 'debt', reason: 'Reviewed: optical centring of the machine icon.', baseline: { offset: -4 }, identity: { @@ -1569,7 +1570,7 @@ describe('re-keying a reviewed finding', () => { }, }, 'geometry/workspace/old-zh-machine': { - status: 'accepted-debt', + status: 'debt', baseline: { offset: -4 }, identity: { label: '机器', @@ -1579,7 +1580,7 @@ describe('re-keying a reviewed finding', () => { }, }, 'geometry/workspace/old-unreviewed': { - status: 'accepted-debt', + status: 'debt', baseline: { offset: 9 }, }, }, @@ -1657,7 +1658,7 @@ describe('re-keying a reviewed finding', () => { expect(migrated.findings['geometry/workspace/old-machine']).toBeUndefined(); expect(migrated.findings['geometry/workspace/new-machine']).toEqual({ - status: 'accepted-debt', + status: 'debt', reason: 'Reviewed: optical centring of the machine icon.', // The baseline travels: a structural re-key must not silently accept the // current offset as the reviewed one. @@ -1803,7 +1804,7 @@ describe('geometry ledger ratchet', () => { it('allows a device pixel of drift above the reviewed baseline and no more', () => { const ledger: GeometryLedger = { version: 1, - findings: { 'geometry/workspace/a': { status: 'accepted-debt', baseline: { offset: -2 } } }, + findings: { 'geometry/workspace/a': { status: 'debt', baseline: { offset: -2 } } }, }; // 2.5 is exactly baseline + one device pixel at 2x; 2.51 is past it. @@ -1824,7 +1825,7 @@ describe('geometry ledger ratchet', () => { kind: 'offset-regression', key: 'geometry/workspace/a', label: 'label for geometry/workspace/a', - status: 'accepted-debt', + status: 'debt', baseline: -2, current: 2.51, tolerance: 0.5, @@ -1885,10 +1886,10 @@ describe('geometry ledger ratchet', () => { ).toEqual([]); }); - it('holds an entry baselined at zero to zero', () => { + it('holds a fixed finding to its near-zero baseline', () => { const ledger: GeometryLedger = { version: 1, - findings: { 'geometry/workspace/a': { status: 'accepted-debt', baseline: { offset: 0 } } }, + findings: { 'geometry/workspace/a': { status: 'fixed', baseline: { offset: 0 } } }, }; expect( @@ -1897,7 +1898,9 @@ describe('geometry ledger ratchet', () => { ledger, retina ) - ).toEqual([expect.objectContaining({ kind: 'offset-regression', baseline: 0 })]); + ).toEqual([ + expect.objectContaining({ kind: 'offset-regression', status: 'fixed', baseline: 0 }), + ]); }); it('takes its tolerance from the coarsest capture the finding was measured on', () => { @@ -1915,10 +1918,121 @@ describe('geometry ledger ratchet', () => { expect( checkGeometryLedgerRatchet( { version: 1, findings: [finding] }, - { version: 1, findings: { 'geometry/workspace/a': { status: 'accepted-debt', baseline: { offset: 0 } } } }, + { version: 1, findings: { 'geometry/workspace/a': { status: 'debt', baseline: { offset: 0 } } } }, mixed ) ).toEqual([]); }); + it('marks a finding fixed only when it is inside one device pixel', () => { + const ledger: GeometryLedger = { + version: 1, + findings: { + 'geometry/workspace/a': { status: 'debt', baseline: { offset: -3 } }, + 'geometry/workspace/b': { status: 'debt', baseline: { offset: 5 } }, + }, + }; + + const still = verifyGeometryFixes( + { version: 1, findings: [ratchetFinding('geometry/workspace/a', 1.5)] }, + ledger, + retina, + ['geometry/workspace/a'] + ); + expect(still.verifications[0]).toMatchObject({ passed: false }); + expect(still.verifications[0]?.reason).toContain('exceeds one device pixel'); + expect(still.ledger).toBe(ledger); + + const fixed = verifyGeometryFixes( + { version: 1, findings: [ratchetFinding('geometry/workspace/a', 0.25)] }, + ledger, + retina, + ['geometry/workspace/a'] + ); + expect(fixed.verifications[0]).toMatchObject({ passed: true, offset: 0.25 }); + expect(fixed.ledger.findings['geometry/workspace/a']).toEqual({ + status: 'fixed', + baseline: { offset: 0.25 }, + }); + // Untouched entries keep their review. + expect(fixed.ledger.findings['geometry/workspace/b']).toEqual( + ledger.findings['geometry/workspace/b'] + ); + }); + + it('treats a finding no rail reports any more as fixed at zero', () => { + const ledger: GeometryLedger = { + version: 1, + findings: { 'geometry/workspace/a': { status: 'debt', baseline: { offset: -3 } } }, + }; + + const result = verifyGeometryFixes({ version: 1, findings: [] }, ledger, retina, [ + 'geometry/workspace/a', + ]); + + expect(result.verifications[0]).toMatchObject({ passed: true, resolved: true }); + expect(result.ledger.findings['geometry/workspace/a']).toEqual({ + status: 'fixed', + baseline: { offset: 0 }, + }); + }); + + it('refuses to unpromote a finding its contract already gates', () => { + const ledger: GeometryLedger = { + version: 1, + findings: { + 'geometry/workspace/a': { + status: 'promoted', + baseline: { offset: 0 }, + contract: { + name: 'workspace.rail', + story: 'geometry--landing', + members: [locator('First'), locator('Second')], + axis: 'x', + anchor: 'inline-end', + space: 'ink', + tolerance: 0.5, + }, + }, + }, + }; + + // Marking it `fixed` would stop compiling the contract and quietly drop the + // tightest rule in the file. + const result = verifyGeometryFixes( + { version: 1, findings: [ratchetFinding('geometry/workspace/a', 0)] }, + ledger, + retina, + ['geometry/workspace/a'] + ); + + expect(result.verifications[0]).toMatchObject({ passed: false }); + expect(result.verifications[0]?.reason).toContain('retire the contract'); + expect(result.ledger).toBe(ledger); + }); + + it('refuses to verify a finding no ledger entry reviews', () => { + const result = verifyGeometryFixes( + { version: 1, findings: [ratchetFinding('geometry/workspace/a', 0)] }, + { version: 1, findings: {} }, + retina, + ['geometry/workspace/a'] + ); + + expect(result.verifications[0]).toMatchObject({ passed: false }); + expect(result.verifications[0]?.reason).toContain('no ledger entry'); + }); + + it('compiles no contract from a fixed, debt or wont-fix entry', () => { + const ledger: GeometryLedger = { + version: 1, + findings: { + 'geometry/workspace/fixed': { status: 'fixed', baseline: { offset: 0 } }, + 'geometry/workspace/debt': { status: 'debt', baseline: { offset: 2 } }, + 'geometry/workspace/wont-fix': { status: 'wont-fix', baseline: { offset: 2 } }, + }, + }; + + expect(compileGeometryContracts(ledger)).toEqual({ version: 1, contracts: [] }); + }); }); From a7f52e4322fbf2de8c7e3e37ce98090436bb845f Mon Sep 17 00:00:00 2001 From: Leeeon233 Date: Thu, 3 Sep 2026 09:49:04 +0000 Subject: [PATCH 3/4] feat(components): point geometry repairs at their source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GeometryRepairTerm.element` was a rendered DOM description — `div[role=button][data-slot=…]` — which a designer recognises and an agent cannot open. Nothing in a finding said which component wrote that node, so the repair text told a reader the size of the problem and nothing about where to make the edit. Collect two more fields for every box-model node during observation: the `class` attribute verbatim, and the nearest function component above it in the React fiber tree. `geometry-react-fiber.ts` walks `fiber.return`, unwrapping the `memo` and `forwardRef` objects React puts in `type`'s place and preferring an explicit `displayName`; it is closure-free like every other function capture serializes into the page, and it introduces no dependency. React 19 dropped `_debugSource`, so a component NAME is the whole pointer — there is no file or line to read, and guessing one would be worse than none. Both fields are optional and both are LABELS: they ride the repair term and the proposal, never a finding key, for the same reason an accessible name may not — renaming a component is not a new finding. A node React never rendered simply contributes none and nothing fails. A report run resolves a component for 3389 of 3429 captured nodes and a class list for 3247. The report's repair text becomes one executable sentence per term, built by the pure `formatGeometryRepairProposal`: SidebarRowShared 里 div[role=button] 的 padding-inline-end 多 2px（class: pe-3） `geometryRepairCssProperty` names the property that actually exists on each edge: `padding-inline-end`, `border-inline-start-width`, and `column-gap`/`row-gap` for a gap, which belongs to an axis rather than to one of its two edges. A real class list runs to hundreds of characters, so only the card body carries it; the collapsed one-line summary omits it rather than truncate it, because half a class list greps for nothing. Verified: `tsgo --noEmit`, unit tests (120 passed), lint, format. Model: claude-opus-5[1m] --- .../src/lib/geometry-constraint-system.ts | 99 ++++++++++++++++--- .../src/lib/geometry-react-fiber.ts | 67 +++++++++++++ .../tests/chat-workspace-geometry.test.ts | 59 +++++++++++ .../chat-workspace-geometry-report.spec.ts | 40 ++++---- .../components/tests/e2e/support/AGENTS.md | 6 ++ .../e2e/support/chat-workspace-geometry.ts | 38 +++++++ .../tests/geometry-constraint-system.test.ts | 67 +++++++++++++ 7 files changed, 342 insertions(+), 34 deletions(-) create mode 100644 packages/components/src/lib/geometry-react-fiber.ts diff --git a/packages/components/src/lib/geometry-constraint-system.ts b/packages/components/src/lib/geometry-constraint-system.ts index ae4729b86..d25c0843d 100644 --- a/packages/components/src/lib/geometry-constraint-system.ts +++ b/packages/components/src/lib/geometry-constraint-system.ts @@ -69,6 +69,14 @@ export type GeometryBoxModelContribution = Readonly<{ export type GeometryBoxModelPathStep = Readonly<{ nodeId: string; element: string; + /** + * The node's `class` attribute verbatim, and the nearest function component + * above it in the React fiber tree. Both are LABELS on a repair ticket, so + * that an agent handed a finding can open a file instead of hunting a + * rendered DOM description; neither may ever reach a finding key. + */ + className?: string; + component?: string; parentId?: string; startToParent: number; endToParent: number; @@ -282,6 +290,9 @@ export type GeometryRepairTerm = Readonly<{ term: GeometryDeclaredBoxModelTerm; /** Rendered description of the box-model node that owns the differing term. */ element: string; + /** That node's `class` attribute and owning component: source pointers, not identity. */ + className?: string; + component?: string; memberElement?: string; referenceElement?: string; memberValue: number; @@ -292,6 +303,9 @@ export type GeometryRepairTerm = Readonly<{ export type GeometryRepairProposal = Readonly<{ commonAncestor: string; + /** The common ancestor's own source pointers, on the same evidence footing. */ + className?: string; + component?: string; edge: 'inline-start' | 'inline-end' | 'block-start' | 'block-end'; terms: readonly GeometryRepairTerm[]; }>; @@ -1116,7 +1130,7 @@ function proposeBoxModelRepair( const dominantOwner = ( chain: readonly GeometryBoxModelPathStep[], term: GeometryDeclaredBoxModelTerm - ) => { + ): GeometryBoxModelPathStep | undefined => { let bestIndex = -1; let bestValue = 0; chain.forEach((step, index) => { @@ -1130,8 +1144,8 @@ function proposeBoxModelRepair( // padding, border and gap are declared on the containing box; margin sits // on the node itself. return PARENT_OWNED_TERMS.has(term) - ? (chain[bestIndex - 1]?.element ?? commonAncestor.element) - : chain[bestIndex]?.element; + ? (chain[bestIndex - 1] ?? commonAncestor) + : chain[bestIndex]; }; const terms: GeometryRepairTerm[] = []; for (const term of DECLARED_BOX_MODEL_TERMS) { @@ -1139,19 +1153,22 @@ function proposeBoxModelRepair( const referenceValue = total(referenceChain, term); const delta = memberValue - referenceValue; if (Math.abs(delta) < 0.5) continue; - const memberElement = dominantOwner(memberChain, term); - const referenceElement = dominantOwner(referenceChain, term); + const memberOwner = dominantOwner(memberChain, term); + const referenceOwner = dominantOwner(referenceChain, term); const dominantSide = Math.abs(memberValue) >= Math.abs(referenceValue) ? 'member' : 'reference'; + const owner = + (dominantSide === 'member' ? memberOwner : referenceOwner) ?? + memberOwner ?? + referenceOwner ?? + commonAncestor; terms.push({ side: dominantSide, term, - element: - (dominantSide === 'member' ? memberElement : referenceElement) ?? - memberElement ?? - referenceElement ?? - commonAncestor.element, - ...(memberElement ? { memberElement } : {}), - ...(referenceElement ? { referenceElement } : {}), + element: owner.element, + ...(owner.className ? { className: owner.className } : {}), + ...(owner.component ? { component: owner.component } : {}), + ...(memberOwner ? { memberElement: memberOwner.element } : {}), + ...(referenceOwner ? { referenceElement: referenceOwner.element } : {}), memberValue: Number(memberValue.toFixed(4)), referenceValue: Number(referenceValue.toFixed(4)), delta: Number(delta.toFixed(4)), @@ -1160,6 +1177,8 @@ function proposeBoxModelRepair( if (terms.length === 0) return undefined; return { commonAncestor: commonAncestor.element, + ...(commonAncestor.className ? { className: commonAncestor.className } : {}), + ...(commonAncestor.component ? { component: commonAncestor.component } : {}), edge, terms: terms.sort( (left, right) => @@ -2858,6 +2877,62 @@ export function summarizeGeometryInkCenters( }; } +/** + * The CSS property a repair term actually edits. `padding` and `margin` have + * logical longhands per edge; a border edge is a width; a gap belongs to the + * axis, not to one of its two edges. Guessing `gap-inline-end` would hand an + * agent a property that does not exist. + */ +export function geometryRepairCssProperty( + term: GeometryDeclaredBoxModelTerm, + edge: GeometryRepairProposal['edge'] +): string { + if (term === 'gap') return edge.startsWith('inline') ? 'column-gap' : 'row-gap'; + if (term === 'border') return `border-${edge}-width`; + return `${term}-${edge}`; +} + +export type GeometryRepairTextOptions = Readonly<{ + maxTerms?: number; + /** + * A Tailwind class list runs to hundreds of characters, which is what an + * agent greps for and what makes a one-line summary unreadable. So the card + * body keeps it and the summary line leaves it out; neither truncates it, + * because half a class list greps for nothing. + */ + includeClassName?: boolean; +}>; + +/** + * One repair term as a sentence an agent can act on: which component, which + * rendered node inside it, which property, how far off, and the class list that + * most likely declares it. + */ +export function formatGeometryRepairTerm( + term: GeometryRepairTerm, + proposal: Pick, + options: GeometryRepairTextOptions = {} +): string { + const owner = term.component ?? proposal.component ?? proposal.commonAncestor; + const property = geometryRepairCssProperty(term.term, proposal.edge); + const magnitude = Number(Math.abs(term.delta).toFixed(2)); + const direction = term.delta > 0 ? '多' : '少'; + const className = + term.className && options.includeClassName !== false ? `（class: ${term.className}）` : ''; + return `${owner} 里 ${term.element} 的 ${property} ${direction} ${magnitude}px${className}`; +} + +/** The proposal's terms as sentences, strongest first. */ +export function formatGeometryRepairProposal( + proposal: GeometryRepairProposal, + options: GeometryRepairTextOptions = {} +): string { + return proposal.terms + .slice(0, options.maxTerms ?? 3) + .map((term) => formatGeometryRepairTerm(term, proposal, options)) + .join('；'); +} + export type GeometryRatchetViolation = Readonly<{ kind: 'offset-regression' | 'unreviewed-finding'; key: string; diff --git a/packages/components/src/lib/geometry-react-fiber.ts b/packages/components/src/lib/geometry-react-fiber.ts new file mode 100644 index 000000000..0a45582a7 --- /dev/null +++ b/packages/components/src/lib/geometry-react-fiber.ts @@ -0,0 +1,67 @@ +/** + * Which component wrote a rendered element. A geometry finding names a DOM + * description a designer recognises but an agent cannot open; the React fiber + * still remembers the component, so a repair ticket can point at source. + * + * Everything here is EVIDENCE, never identity: a finding key must not move + * because a component was renamed, so nothing in this file may reach a key. + * React 19 dropped `_debugSource`, so a component NAME is the whole pointer — + * there is no file or line to read, and guessing one would be worse than none. + * + * Keep these functions closure-free: capture serializes them into the page. + */ + +/** A fiber, as much of one as reading a component name needs. */ +export type GeometryReactFiberLike = Readonly<{ + type?: unknown; + return?: GeometryReactFiberLike | null; +}>; + +/** + * The component name a fiber `type` carries, unwrapping the `memo` and + * `forwardRef` objects React puts in `type`'s place. A host element (`'div'`) + * and an anonymous function have no name to give and return undefined, so the + * walk keeps going up rather than reporting a blank. + */ +export function geometryReactFiberComponentName( + fiber: GeometryReactFiberLike | null | undefined, + maxDepth = 64 +): string | undefined { + const typeName = (type: unknown, unwrapDepth: number): string | undefined => { + if (!type || unwrapDepth > 8) return undefined; + if (typeof type === 'function') { + const named = type as Readonly<{ displayName?: unknown; name?: unknown }>; + const display = typeof named.displayName === 'string' ? named.displayName : undefined; + const intrinsic = typeof named.name === 'string' ? named.name : undefined; + const resolved = display ?? intrinsic; + return resolved && resolved.length > 0 ? resolved : undefined; + } + if (typeof type !== 'object') return undefined; + const wrapper = type as Readonly<{ displayName?: unknown; render?: unknown; type?: unknown }>; + if (typeof wrapper.displayName === 'string' && wrapper.displayName.length > 0) { + return wrapper.displayName; + } + // `forwardRef` keeps the component on `render`, `memo` on `type`. + return typeName(wrapper.render, unwrapDepth + 1) ?? typeName(wrapper.type, unwrapDepth + 1); + }; + let node = fiber; + for (let depth = 0; node && depth < maxDepth; depth += 1) { + const name = typeName(node.type, 0); + if (name) return name; + node = node.return; + } + return undefined; +} + +/** + * The fiber React attached to a rendered node. The key carries a per-renderer + * suffix, so it is found by prefix rather than spelled out. + */ +export function geometryElementReactFiber(element: Element): GeometryReactFiberLike | undefined { + for (const key of Object.keys(element)) { + if (!key.startsWith('__reactFiber$')) continue; + const fiber = (element as unknown as Record)[key]; + if (fiber && typeof fiber === 'object') return fiber as GeometryReactFiberLike; + } + return undefined; +} diff --git a/packages/components/tests/chat-workspace-geometry.test.ts b/packages/components/tests/chat-workspace-geometry.test.ts index 956d98725..d26a949cc 100644 --- a/packages/components/tests/chat-workspace-geometry.test.ts +++ b/packages/components/tests/chat-workspace-geometry.test.ts @@ -26,6 +26,11 @@ import { type LayoutTopologyNode, } from '../src/lib/chat-workspace-geometry'; import { geometryCanvasFontString } from '../src/lib/geometry-text-cap-band'; +import { + geometryElementReactFiber, + geometryReactFiberComponentName, + type GeometryReactFiberLike, +} from '../src/lib/geometry-react-fiber'; const anchors = CHAT_WORKSPACE_GEOMETRY_ANCHORS; @@ -1348,3 +1353,57 @@ describe('the canvas font string', () => { expect(geometryCanvasFontString(style)).not.toContain('tabular-nums'); }); }); + +describe('React component names as repair evidence', () => { + const fiber = (type: unknown, parent?: GeometryReactFiberLike): GeometryReactFiberLike => ({ + type, + return: parent ?? null, + }); + + it('walks up to the nearest function component', () => { + function SidebarRowShared() { + return null; + } + const host = fiber('span', fiber('div', fiber(SidebarRowShared))); + + expect(geometryReactFiberComponentName(host)).toBe('SidebarRowShared'); + }); + + it('unwraps forwardRef and memo, and prefers an explicit displayName', () => { + function Inner() { + return null; + } + const forwarded = { $$typeof: Symbol.for('react.forward_ref'), render: Inner }; + const memoized = { $$typeof: Symbol.for('react.memo'), type: forwarded }; + const renamed = { $$typeof: Symbol.for('react.memo'), type: Inner, displayName: 'SidebarRow' }; + + expect(geometryReactFiberComponentName(fiber(memoized))).toBe('Inner'); + expect(geometryReactFiberComponentName(fiber(renamed))).toBe('SidebarRow'); + }); + + it('returns nothing rather than a blank when no ancestor names a component', () => { + expect(geometryReactFiberComponentName(fiber('div', fiber('span')))).toBeUndefined(); + expect(geometryReactFiberComponentName(fiber(() => null))).toBeUndefined(); + expect(geometryReactFiberComponentName(undefined)).toBeUndefined(); + expect(geometryReactFiberComponentName(null)).toBeUndefined(); + }); + + it('stops walking instead of looping on a cyclic return chain', () => { + const cycle: { type: unknown; return: unknown } = { type: 'div', return: null }; + cycle.return = cycle; + + expect(geometryReactFiberComponentName(cycle as GeometryReactFiberLike, 8)).toBeUndefined(); + }); + + it('finds the fiber React attached under its per-renderer key', () => { + function Row() { + return null; + } + const attached = { __reactFiber$abc123: fiber(Row) } as unknown as Element; + const bare = {} as unknown as Element; + + expect(geometryReactFiberComponentName(geometryElementReactFiber(attached))).toBe('Row'); + // A node React never rendered simply contributes no component name. + expect(geometryElementReactFiber(bare)).toBeUndefined(); + }); +}); diff --git a/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts b/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts index e18dba286..7991a2c07 100644 --- a/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts +++ b/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts @@ -21,6 +21,7 @@ import { computeGeometryQualityMetrics, createGeometryFindings, diffGeometryFindings, + formatGeometryRepairProposal, geometryIdentityLocator, geometryLocatorMatches, observeGeometryCaptures, @@ -342,27 +343,18 @@ const CLASSIFICATION_LABELS: Readonly> = { - padding: 'padding', - border: 'border', - margin: 'margin', - gap: 'gap', -}; - -function formatRepairProposal(proposal: GeometryRepairProposal): string { - const edge = proposal.edge === 'inline-start' ? '起始边' : '结束边'; - const terms = proposal.terms - .slice(0, 3) - .map((term) => { - const owner = term.side === 'member' ? '本项' : '参照'; - return `${BOX_MODEL_TERM_LABELS[term.term] ?? term.term} 本项 ${Number( - term.memberValue.toFixed(2) - )} vs 参照 ${Number(term.referenceValue.toFixed(2))}（Δ${Number( - term.delta.toFixed(2) - )}px；差值来自${owner}的 ${term.element}）`; - }) - .join('；'); - return `修复建议（${edge}）：${terms}`; +/** + * One executable sentence per term — which component, which node, which CSS + * property, how far off, and the class list that most likely declares it. The + * old text printed the two box-model totals and a rendered DOM description, + * which told a reviewer the size of the problem and an agent nothing about + * where to make the edit. + */ +function formatRepairProposal( + proposal: GeometryRepairProposal, + options?: Parameters[1] +): string { + return `修复建议：${formatGeometryRepairProposal(proposal, options)}`; } function semanticAlignmentTitle(entry: BrowserSemanticAlignmentEntry): string { @@ -2263,7 +2255,11 @@ test('captures the visual geometry report', async ({ browser }) => { const repairProposal = finding.repairProposal ? formatRepairProposal(finding.repairProposal) : undefined; - const repairFinding = repairProposal ? ` · ${repairProposal}` : ''; + // The collapsed summary line drops the class list the card body keeps: a + // Tailwind class list is hundreds of characters and would swamp it. + const repairFinding = finding.repairProposal + ? ` · ${formatRepairProposal(finding.repairProposal, { includeClassName: false })}` + : ''; const dimensionSensitivity = (finding.dimensionSensitivity ?? []).map( (item) => `${item.axis}=${item.value}` ); diff --git a/packages/components/tests/e2e/support/AGENTS.md b/packages/components/tests/e2e/support/AGENTS.md index d2a8937e4..eb6fd8b00 100644 --- a/packages/components/tests/e2e/support/AGENTS.md +++ b/packages/components/tests/e2e/support/AGENTS.md @@ -78,6 +78,12 @@ to make the gate cheaper moves every merged offset the ledger recorded. context, a device scale cannot, and a fresh context's cold cache re-parses the whole Storybook bundle — minutes of wall clock. A fresh PAGE per capture inside it, though: a page that loaded a dozen stories runs out of memory. +- Every box-model node also records its `class` and the nearest function component above it + in the fiber tree (`geometry-react-fiber.ts`: walk `fiber.return`, unwrap `memo` and + `forwardRef`). Both are EVIDENCE — they turn a rendered DOM description into a file an + agent can open — and neither may reach a key, for the reason an accessible name may not. + React 19 has no `_debugSource`, so the NAME is the whole pointer; a node React never + rendered has none and nothing fails. - A card clip holds the row plus a margin, draws the row median and verdict anchor only, and names it. Zoomed Y cards are the largest-|offset| findings anywhere. Discovery cards use product-region names, count unique elements not anchor votes, fold one element's diff --git a/packages/components/tests/e2e/support/chat-workspace-geometry.ts b/packages/components/tests/e2e/support/chat-workspace-geometry.ts index 121a39a3c..3df51da57 100644 --- a/packages/components/tests/e2e/support/chat-workspace-geometry.ts +++ b/packages/components/tests/e2e/support/chat-workspace-geometry.ts @@ -37,6 +37,10 @@ import { geometryCapBandCenter, measureGeometryCapBand, } from '../../../src/lib/geometry-text-cap-band'; +import { + geometryElementReactFiber, + geometryReactFiberComponentName, +} from '../../../src/lib/geometry-react-fiber'; import { evaluateGeometryContractResolutions, geometryContractRelationProperties, @@ -646,6 +650,8 @@ export async function installGeometryBrowserHelpers(page: Page): Promise { `globalThis.__lodyGeometrySelectRowSlots = ${selectVisualRowSlots.toString()};`, `globalThis.__lodyGeometryIsPaintedShape = ${isGeometryPaintedShape.toString()};`, `globalThis.__lodyMeasureGeometryBlockAnchors = ${measureGeometryBlockAnchorsInBrowser.toString()};`, + `globalThis.__lodyGeometryElementFiber = ${geometryElementReactFiber.toString()};`, + `globalThis.__lodyGeometryFiberComponentName = ${geometryReactFiberComponentName.toString()};`, ].join('\n'), }); } @@ -687,6 +693,19 @@ export async function captureChatWorkspaceGeometryScopes( } ).__lodyGeometryIsPaintedShape; if (!isPaintedShapeStyle) throw new Error('Geometry painted-shape helper is missing'); + const elementFiber = ( + globalThis as typeof globalThis & { + __lodyGeometryElementFiber?: typeof geometryElementReactFiber; + } + ).__lodyGeometryElementFiber; + const fiberComponentName = ( + globalThis as typeof globalThis & { + __lodyGeometryFiberComponentName?: typeof geometryReactFiberComponentName; + } + ).__lodyGeometryFiberComponentName; + if (!elementFiber || !fiberComponentName) { + throw new Error('Geometry React component-name helper is missing'); + } const isRendered = (element: Element) => { const rect = element.getBoundingClientRect(); const style = getComputedStyle(element); @@ -759,6 +778,24 @@ export async function captureChatWorkspaceGeometryScopes( .join(''); return `${element.tagName.toLowerCase()}${attributes}`; }; + /** + * The two source pointers a repair ticket needs. Both are optional and + * failure-tolerant: a node React never rendered, or a production build + * with no readable fiber, simply contributes no component name. + */ + const sourcePointers = (element: Element) => { + const className = element.getAttribute('class')?.replace(/\s+/g, ' ').trim(); + let component: string | undefined; + try { + component = fiberComponentName(elementFiber(element)); + } catch { + component = undefined; + } + return { + ...(className ? { className } : {}), + ...(component ? { component } : {}), + }; + }; const boxModelPath = ( element: Element, boundary: Element @@ -816,6 +853,7 @@ export async function captureChatWorkspaceGeometryScopes( path.push({ nodeId: idByElement.get(node) ?? 'dom-unknown', element: describeBoxModelNode(node), + ...sourcePointers(node), ...(parent ? { parentId: idByElement.get(parent) ?? 'dom-unknown' } : {}), startToParent, endToParent, diff --git a/packages/components/tests/geometry-constraint-system.test.ts b/packages/components/tests/geometry-constraint-system.test.ts index 9e3d0cf4c..2fb019096 100644 --- a/packages/components/tests/geometry-constraint-system.test.ts +++ b/packages/components/tests/geometry-constraint-system.test.ts @@ -16,7 +16,9 @@ import { explainGeometryOffset, checkGeometryLedgerRatchet, formatGeometryRatchetViolations, + formatGeometryRepairProposal, geometryFindingDevicePixel, + geometryRepairCssProperty, verifyGeometryFixes, geometryFindingLabel, geometryIdentityLocator, @@ -37,6 +39,7 @@ import { type GeometryCaptureArtifact, type GeometryFinding, type GeometryLedger, + type GeometryRepairProposal, } from '../src/lib/geometry-constraint-system'; const locator = (name: string) => ({ @@ -2036,3 +2039,67 @@ describe('geometry ledger ratchet', () => { expect(compileGeometryContracts(ledger)).toEqual({ version: 1, contracts: [] }); }); }); + +describe('geometry repair tickets', () => { + const proposal = ( + overrides: Partial & { + term?: Partial; + } = {} + ): GeometryRepairProposal => { + const { term, ...rest } = overrides; + return { + commonAncestor: 'div[data-slot=sidebar-row]', + component: 'SidebarRowShared', + edge: 'inline-end', + terms: [ + { + side: 'member', + term: 'padding', + element: 'div[role=button]', + className: 'pe-3', + component: 'SidebarRowShared', + memberValue: 12, + referenceValue: 10, + delta: 2, + ...term, + }, + ], + ...rest, + }; + }; + + it('names the CSS property an agent would actually edit', () => { + expect(geometryRepairCssProperty('padding', 'inline-end')).toBe('padding-inline-end'); + expect(geometryRepairCssProperty('margin', 'block-start')).toBe('margin-block-start'); + expect(geometryRepairCssProperty('border', 'inline-start')).toBe('border-inline-start-width'); + // A gap belongs to the axis; `gap-inline-end` is not a property. + expect(geometryRepairCssProperty('gap', 'inline-end')).toBe('column-gap'); + expect(geometryRepairCssProperty('gap', 'block-end')).toBe('row-gap'); + }); + + it('renders a repair a reader can act on without opening the artifact', () => { + expect(formatGeometryRepairProposal(proposal())).toBe( + 'SidebarRowShared 里 div[role=button] 的 padding-inline-end 多 2px（class: pe-3）' + ); + expect( + formatGeometryRepairProposal(proposal({ term: { className: undefined, delta: -2 } })) + ).toBe('SidebarRowShared 里 div[role=button] 的 padding-inline-end 少 2px'); + }); + + it('leaves the class list out where a whole class list would not fit', () => { + // A real Tailwind class list is hundreds of characters. The card body keeps + // it whole, because half of one greps for nothing; the one-line summary + // drops it rather than truncate it. + expect(formatGeometryRepairProposal(proposal(), { includeClassName: false })).toBe( + 'SidebarRowShared 里 div[role=button] 的 padding-inline-end 多 2px' + ); + }); + + it('falls back to the common ancestor when React rendered no component name', () => { + // A node React never rendered still gets a sentence, just a less precise one. + expect( + formatGeometryRepairProposal(proposal({ component: undefined, term: { component: undefined } })) + ).toBe('div[data-slot=sidebar-row] 里 div[role=button] 的 padding-inline-end 多 2px（class: pe-3）'); + }); + +}); From faf596965d4b75cd5f2269c77780ddc6106a977c Mon Sep 17 00:00:00 2001 From: Leeeon233 Date: Thu, 3 Sep 2026 09:51:33 +0000 Subject: [PATCH 4/4] feat(components): fold geometry cards by repair identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The report deduplicated by LABEL: `groupLabel.split(' · ')[0]` plus a member-offset signature, which collapses two instances of one named marker group and nothing else. Ten discovered rows that share one wrong padding still produced ten cards, so the queue's length measured how many elements a defect touched rather than how many defects there were. Give a `css-defect` finding a `repairGroup` in the FINDINGS stage — the component that owns the edit (or, where React rendered no component, the common ancestor's DOM description), the declared term, the edge, and the node the term sits on. It is computed from the repair proposal, so a second row of the same component reporting the same edit lands in the same group whatever its label, its measured value or which of the two compared sides dominated. Findings do not merge. Keys, baselines, evidence and the ratchet are untouched: `repairGroup` is a grouping LABEL, and it never reaches a key, exactly like the component name it is built from. The report folds cards on it — one card per repair, naming the findings it stands for — and folds strictly inside one ledger status, because a `new` finding hidden under a reviewed one would be a review that never happened. The `groupLabel` signature stays where it is: it is what decides that N instances with identical member offsets are one measurement-model divergence, which is a verdict rather than a dedup, and its single card is that verdict's card. `geometry:verify-fix` takes `--repair-group=` so one edit can be verified and closed for every finding it covers in one gate run. Verified: `tsgo --noEmit`, unit tests (125 passed), lint, format. Model: claude-opus-5[1m] --- .../chat-workspace-geometry-report.html | 9 +- .../scripts/verify-geometry-fix.mjs | 9 +- .../src/lib/geometry-constraint-system.ts | 53 +++++- packages/components/tests/e2e/AGENTS.md | 9 +- .../chat-workspace-geometry-report.spec.ts | 52 +++++- .../tests/e2e/chat-workspace-geometry.spec.ts | 16 +- .../components/tests/e2e/support/AGENTS.md | 3 +- .../tests/geometry-constraint-system.test.ts | 151 +++++++++++++++++- 8 files changed, 278 insertions(+), 24 deletions(-) diff --git a/packages/components/scripts/templates/chat-workspace-geometry-report.html b/packages/components/scripts/templates/chat-workspace-geometry-report.html index cae637f3f..b9da69e6b 100644 --- a/packages/components/scripts/templates/chat-workspace-geometry-report.html +++ b/packages/components/scripts/templates/chat-workspace-geometry-report.html @@ -932,7 +932,14 @@

明确排除

factsEl.append(span); } if (factsEl.childElementCount > 0) inner.append(factsEl); - for (const text of [detail.repairProposal, detail.inkCenterWitness]) { + // One card per REPAIR: the findings folded into it are named, so a + // reviewer can see they were reviewed rather than dropped. + const foldedKeys = detail.repairGroupKeys ?? []; + const foldedText = + foldedKeys.length > 0 + ? `同一处修复覆盖 ${foldedKeys.length + 1} 条 finding：${foldedKeys.join('、')}` + : undefined; + for (const text of [detail.repairProposal, foldedText, detail.inkCenterWitness]) { if (!text) continue; const paragraph = document.createElement('p'); paragraph.className = 'detail-repair'; diff --git a/packages/components/scripts/verify-geometry-fix.mjs b/packages/components/scripts/verify-geometry-fix.mjs index 3072709ce..d912a2678 100644 --- a/packages/components/scripts/verify-geometry-fix.mjs +++ b/packages/components/scripts/verify-geometry-fix.mjs @@ -7,11 +7,15 @@ import { fileURLToPath } from 'node:url'; const packageRoot = fileURLToPath(new URL('..', import.meta.url)); const argumentsList = process.argv.slice(2); const positional = argumentsList.filter((argument) => !argument.startsWith('--')); +const repairGroupArgument = argumentsList.find((argument) => argument.startsWith('--repair-group=')); const ledgerArgument = argumentsList.find((argument) => argument.startsWith('--ledger=')); +const repairGroup = repairGroupArgument?.slice('--repair-group='.length); const [requestedOutput, ...requestedKeys] = positional; -if (!requestedOutput || requestedKeys.length === 0) { - throw new Error('Usage: pnpm geometry:verify-fix [--ledger=]'); +if (!requestedOutput || (requestedKeys.length === 0 && !repairGroup)) { + throw new Error( + 'Usage: pnpm geometry:verify-fix [findingKey...] [--repair-group=] [--ledger=]' + ); } const outputDirectory = path.resolve(packageRoot, requestedOutput); @@ -83,6 +87,7 @@ await run( ...process.env, GEOMETRY_PIPELINE_OUTPUT_DIR: outputDirectory, GEOMETRY_VERIFY_FIX_KEYS: requestedKeys.join(','), + ...(repairGroup ? { GEOMETRY_VERIFY_FIX_REPAIR_GROUP: repairGroup } : {}), PLAYWRIGHT_PORT: String(port), PLAYWRIGHT_BASE_URL: `http://127.0.0.1:${port}`, VITE_PREVIEW_PUBLIC_BASE_DOMAIN: diff --git a/packages/components/src/lib/geometry-constraint-system.ts b/packages/components/src/lib/geometry-constraint-system.ts index d25c0843d..659b3a614 100644 --- a/packages/components/src/lib/geometry-constraint-system.ts +++ b/packages/components/src/lib/geometry-constraint-system.ts @@ -308,6 +308,12 @@ export type GeometryRepairProposal = Readonly<{ component?: string; edge: 'inline-start' | 'inline-end' | 'block-start' | 'block-end'; terms: readonly GeometryRepairTerm[]; + /** + * Which repair this proposal IS, independent of which element reported it. + * Findings never merge on it — the key is untouched — but a report folds one + * card per group, so ten rows sharing one wrong padding read as one ticket. + */ + repairGroup?: string; }>; export type GeometryOffsetExplanation = Readonly<{ @@ -382,6 +388,12 @@ export type GeometryFinding = Readonly<{ /** Alignment-rail findings only; derived from evidence explanations alone. */ classification?: GeometryFindingClassification; repairProposal?: GeometryRepairProposal; + /** + * Which REPAIR this finding belongs to, for `css-defect` findings that name + * one. Grouping only: the key, the evidence and the review are untouched, and + * a finding without a repair proposal simply has none. + */ + repairGroup?: string; /** Set when every evidence row shares one value of a varying capture axis. */ dimensionSensitivity?: readonly GeometryDimensionSensitivity[]; evidence: readonly GeometryFindingEvidence[]; @@ -1175,16 +1187,35 @@ function proposeBoxModelRepair( }); } if (terms.length === 0) return undefined; - return { + const sorted = terms.sort( + (left, right) => + Math.abs(right.delta) - Math.abs(left.delta) || left.term.localeCompare(right.term) + ); + const proposal: GeometryRepairProposal = { commonAncestor: commonAncestor.element, ...(commonAncestor.className ? { className: commonAncestor.className } : {}), ...(commonAncestor.component ? { component: commonAncestor.component } : {}), edge, - terms: terms.sort( - (left, right) => - Math.abs(right.delta) - Math.abs(left.delta) || left.term.localeCompare(right.term) - ), + terms: sorted, }; + return { ...proposal, repairGroup: geometryRepairGroupKey(proposal) }; +} + +/** + * Repair identity: WHICH edit closes this, not which element reported it. The + * component that owns the edit (or, unrendered by React, the common ancestor's + * DOM description), the term, the edge, and the node the term sits on. It is a + * grouping label only — findings are never merged on it and no key reads it — + * so ten rows sharing one wrong padding stay ten reviewed findings and become + * one ticket. + */ +export function geometryRepairGroupKey( + proposal: Omit +): string | undefined { + const dominant = proposal.terms[0]; + if (!dominant) return undefined; + const owner = dominant.component ?? proposal.component ?? proposal.commonAncestor; + return makeFindingKey(['repair', owner, dominant.term, proposal.edge, dominant.element]); } function declaredTermDelta(explanation: GeometryOffsetExplanation): number { @@ -2088,6 +2119,7 @@ export function createGeometryFindings( totalCaptureCount: totalBySurface.get(group.surfaceFamily) ?? evidence.length, classification, ...(repairProposal ? { repairProposal } : {}), + ...(repairProposal?.repairGroup ? { repairGroup: repairProposal.repairGroup } : {}), ...(sensitivity.length > 0 ? { dimensionSensitivity: sensitivity } : {}), evidence, }; @@ -3099,6 +3131,17 @@ export function verifyGeometryFixes( }; } +/** Every finding key the artifact assigns to one repair group. */ +export function geometryFindingKeysInRepairGroup( + artifact: GeometryFindingArtifact, + repairGroup: string +): readonly string[] { + return artifact.findings + .filter((finding) => finding.repairGroup === repairGroup) + .map((finding) => finding.key) + .sort(); +} + export function formatGeometryRatchetViolations( violations: readonly GeometryRatchetViolation[] ): string { diff --git a/packages/components/tests/e2e/AGENTS.md b/packages/components/tests/e2e/AGENTS.md index 699a46165..35e39864a 100644 --- a/packages/components/tests/e2e/AGENTS.md +++ b/packages/components/tests/e2e/AGENTS.md @@ -93,7 +93,7 @@ declaring `ink` or `layout-box` — `new`, `debt`, `wont-fix`, `fixed`, `ignored contract check. Offsets are means over the WHOLE plan, so a baseline belongs to the platform that recorded it: re-baseline where CI runs, never trim the plan for speed. - `debt` and `wont-fix` are two decisions, not one word; `triage` records `debt` rather - than guess. `geometry:verify-fix ` reruns that gate and only + than guess. `geometry:verify-fix ` reruns that gate and only then moves a finding back inside one device pixel to `fixed` at its new baseline, the strictest entry there is. - Two contract members never cover one element twice; member resolution, the ink witness @@ -113,9 +113,10 @@ Discovery or proposal presence is never a report assertion; coverage: findings or replacing evidence. Replay: [support](support/AGENTS.md). - Steady state, not delta: every finding gets a card grouped by ledger status and classification, with baseline vs current offset, capture count, dimension sensitivity and - repair text. Chips filter both, default new + changed + css-defect + promoted minus - wont-fix and fixed; the meta line totals new/changed/resolved. One JSON payload, one - renderer, images as files. + repair text. Cards FOLD by `repairGroup` — one per repair, naming the findings it stands + for, never across statuses; folding merges no finding and moves no key. Chips filter + both, default new + changed + css-defect + promoted minus wont-fix/fixed; the meta line + totals new/changed/resolved. One JSON payload, one renderer, images as files. - Violation images label each deviating member in place with role, physical direction, measured offset, actual anchor and a leader to it. A Y card comes from the FINISHED findings, never a second pipeline printing another number: each annotation IS that diff --git a/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts b/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts index 7991a2c07..e1e4ba11e 100644 --- a/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts +++ b/packages/components/tests/e2e/chat-workspace-geometry-report.spec.ts @@ -82,6 +82,9 @@ type ReportDetail = Readonly<{ totalCaptureCount?: number; dimensionSensitivity?: readonly string[]; repairProposal?: string; + /** The repair this card stands for, and the findings it folded into it. */ + repairGroup?: string; + repairGroupKeys?: readonly string[]; inkCenterWitness?: string; id: string; title: string; @@ -2164,10 +2167,43 @@ test('captures the visual geometry report', async ({ browser }) => { : [] ) ); + const findingLedgerStatus = (finding: GeometryFinding): GeometryLedgerStatus | 'changed' => + newFindingKeys.has(finding.key) + ? 'new' + : changedFindingKeys.has(finding.key) + ? 'changed' + : (ledger.findings[finding.key]?.status ?? 'new'); + /** + * Cards fold by REPAIR, not by label. Ten rows that share one wrong padding + * are ten reviewed findings — the keys, the baselines and the ratchet are + * untouched — and exactly one ticket, so the report shows one card and names + * the findings it stands for. Folding never crosses a ledger status: a `new` + * finding hidden under a reviewed one would be a review that never happened. + */ + const repairGroupKey = (finding: GeometryFinding) => + finding.repairGroup ? `${finding.repairGroup}\u0000${findingLedgerStatus(finding)}` : undefined; + const repairGroups = new Map(); + for (const finding of persistedFindings.findings) { + const key = repairGroupKey(finding); + if (!key) continue; + repairGroups.set(key, [...(repairGroups.get(key) ?? []), finding]); + } + const repairGroupLead = new Map( + [...repairGroups.entries()].map(([key, members]) => [ + key, + [...members].sort( + (left, right) => + Math.abs(right.offset) - Math.abs(left.offset) || left.key.localeCompare(right.key) + )[0]?.key, + ]) + ); + // The report shows the steady state, not only the delta: every finding in // findings.json gets a card, and the ledger status says how it was reviewed. const displayedDetails = persistedFindings.findings.flatMap((finding): ReportDetail[] => { if (finding.kind === 'measurement-model-divergence') return []; + const groupKey = repairGroupKey(finding); + if (groupKey && repairGroupLead.get(groupKey) !== finding.key) return []; const evidence = finding.evidence[0]; if (!evidence) return []; let representative = rawDetailByFindingKey.get(finding.key); @@ -2265,11 +2301,11 @@ test('captures the visual geometry report', async ({ browser }) => { ); const dimensionFinding = dimensionSensitivity.length > 0 ? ` · 仅出现在 ${dimensionSensitivity.join('、')}` : ''; - const ledgerStatus: GeometryLedgerStatus | 'changed' = newFindingKeys.has(finding.key) - ? 'new' - : changedFindingKeys.has(finding.key) - ? 'changed' - : (ledger.findings[finding.key]?.status ?? 'new'); + const ledgerStatus = findingLedgerStatus(finding); + const foldedKeys = (groupKey ? (repairGroups.get(groupKey) ?? []) : []) + .map((member) => member.key) + .filter((key) => key !== finding.key) + .sort(); const baseline = ledger.findings[finding.key]?.baseline?.offset; const inkCenters = inkCentersByFindingKey.get(finding.key); const inkCenterWitness = inkCenters @@ -2292,6 +2328,8 @@ test('captures the visual geometry report', async ({ browser }) => { ...(dimensionSensitivity.length > 0 ? { dimensionSensitivity } : {}), ...(repairProposal ? { repairProposal } : {}), ...(inkCenterWitness ? { inkCenterWitness } : {}), + ...(finding.repairGroup ? { repairGroup: finding.repairGroup } : {}), + ...(foldedKeys.length > 0 ? { repairGroupKeys: foldedKeys } : {}), description: `${CLASSIFICATION_LABELS[classification]} · ${finding.label} · ${finding.captureCount}/${finding.totalCaptureCount} 个捕获一致`, finding: `[${CLASSIFICATION_LABELS[classification]}] ${finding.label} ${DISCOVERY_ANCHOR_LABELS[finding.anchor as keyof typeof DISCOVERY_ANCHOR_LABELS] ?? finding.anchor} ${direction} · ${finding.evidence.length} 条 evidence${repairFinding}${dimensionFinding}${boxModelFinding}`, }, @@ -2354,6 +2392,8 @@ test('captures the visual geometry report', async ({ browser }) => { totalCaptureCount, dimensionSensitivity, repairProposal, + repairGroup, + repairGroupKeys, inkCenterWitness, id, title, @@ -2378,6 +2418,8 @@ test('captures the visual geometry report', async ({ browser }) => { ? { dimensionSensitivity } : {}), ...(repairProposal ? { repairProposal } : {}), + ...(repairGroup ? { repairGroup } : {}), + ...(repairGroupKeys && repairGroupKeys.length > 0 ? { repairGroupKeys } : {}), ...(inkCenterWitness ? { inkCenterWitness } : {}), id, captureId, diff --git a/packages/components/tests/e2e/chat-workspace-geometry.spec.ts b/packages/components/tests/e2e/chat-workspace-geometry.spec.ts index 4a226bbb2..c3cd49a5b 100644 --- a/packages/components/tests/e2e/chat-workspace-geometry.spec.ts +++ b/packages/components/tests/e2e/chat-workspace-geometry.spec.ts @@ -16,6 +16,7 @@ import { checkGeometryLedgerRatchet, compileGeometryContracts, formatGeometryRatchetViolations, + geometryFindingKeysInRepairGroup, verifyGeometryFixes, type GeometryContractArtifact, type GeometryLedger, @@ -46,7 +47,7 @@ import { */ const pipelineOutputDirectory = process.env.GEOMETRY_PIPELINE_OUTPUT_DIR; /** - * `pnpm geometry:verify-fix` sets this. The DECISION — is this finding fixed, + * `pnpm geometry:verify-fix` sets these. The DECISION — is this finding fixed, * and what does the ledger become — is made here in TypeScript, beside the * ratchet it must not regress; the script only applies the file it writes. */ @@ -54,6 +55,7 @@ const verifyFixKeys = (process.env.GEOMETRY_VERIFY_FIX_KEYS ?? '') .split(',') .map((key) => key.trim()) .filter(Boolean); +const verifyFixRepairGroup = process.env.GEOMETRY_VERIFY_FIX_REPAIR_GROUP; const STORY_IDS = { expanded: 'geometry-chatworkspace--expanded-sidebar', @@ -134,8 +136,16 @@ test('every measured geometry finding stays inside its reviewed ledger baseline' `Geometry ledger ratchet failed:\n\n${formatGeometryRatchetViolations(violations)}` ).toEqual([]); - if (!pipelineOutputDirectory || verifyFixKeys.length === 0) return; - const keys = [...new Set(verifyFixKeys)]; + if (!pipelineOutputDirectory || (verifyFixKeys.length === 0 && !verifyFixRepairGroup)) return; + const keys = [ + ...new Set([ + ...verifyFixKeys, + ...(verifyFixRepairGroup + ? geometryFindingKeysInRepairGroup(findings, verifyFixRepairGroup) + : []), + ]), + ]; + expect(keys, 'No finding matches the requested keys or repair group').not.toEqual([]); const verification = verifyGeometryFixes(findings, ledger, capture, keys); await writeFile( path.join(pipelineOutputDirectory, 'fix-verification.json'), diff --git a/packages/components/tests/e2e/support/AGENTS.md b/packages/components/tests/e2e/support/AGENTS.md index eb6fd8b00..95d190b68 100644 --- a/packages/components/tests/e2e/support/AGENTS.md +++ b/packages/components/tests/e2e/support/AGENTS.md @@ -83,7 +83,8 @@ to make the gate cheaper moves every merged offset the ledger recorded. `forwardRef`). Both are EVIDENCE — they turn a rendered DOM description into a file an agent can open — and neither may reach a key, for the reason an accessible name may not. React 19 has no `_debugSource`, so the NAME is the whole pointer; a node React never - rendered has none and nothing fails. + rendered has none and nothing fails. `repairGroup` (component, term, edge, owning node) + is a label too: it folds report cards and merges no finding. - A card clip holds the row plus a margin, draws the row median and verdict anchor only, and names it. Zoomed Y cards are the largest-|offset| findings anywhere. Discovery cards use product-region names, count unique elements not anchor votes, fold one element's diff --git a/packages/components/tests/geometry-constraint-system.test.ts b/packages/components/tests/geometry-constraint-system.test.ts index 2fb019096..f383dc96a 100644 --- a/packages/components/tests/geometry-constraint-system.test.ts +++ b/packages/components/tests/geometry-constraint-system.test.ts @@ -18,7 +18,9 @@ import { formatGeometryRatchetViolations, formatGeometryRepairProposal, geometryFindingDevicePixel, + geometryFindingKeysInRepairGroup, geometryRepairCssProperty, + geometryRepairGroupKey, verifyGeometryFixes, geometryFindingLabel, geometryIdentityLocator, @@ -36,8 +38,10 @@ import { type GeometryCapturedCandidate, type GeometryCapturedScope, type GeometryContract, + type GeometryBoxModelPathStep, type GeometryCaptureArtifact, type GeometryFinding, + type GeometryFindingArtifact, type GeometryLedger, type GeometryRepairProposal, } from '../src/lib/geometry-constraint-system'; @@ -179,6 +183,77 @@ describe('geometry constraint artifacts', () => { ]); }); + it('carries the repair group onto the finding without touching its key', () => { + // Two rails whose outlier is off by the same declared padding, on the same + // component: one ticket, two reviewed findings. + const contribution = (padding: number) => ({ + padding, + border: 0, + margin: 0, + gap: 0, + layout: 0, + }); + const node = ( + nodeId: string, + padding: number, + parentId?: string + ): GeometryBoxModelPathStep => ({ + nodeId, + element: 'div[role=button]', + className: 'pe-3', + component: 'SidebarRowShared', + ...(parentId ? { parentId } : {}), + startToParent: 0, + endToParent: padding, + centerToParent: 0, + inlineStart: contribution(0), + inlineEnd: contribution(padding), + }); + const measuredRail = (suffix: string) => + railCandidates(suffix).map((item) => ({ ...item, boxModelNodeRef: item.primitiveId })); + // The outlier's own trailing padding is 4px short of its row's, which is + // exactly the offset the rail measured. + const boxModelNodes = Object.fromEntries( + ['a', 'b'].flatMap((suffix) => [ + ...['one', 'two', 'three'].map( + (name) => [`${name}-${suffix}`, node(`${name}-${suffix}`, 14, 'root')] as const + ), + [`shifted-${suffix}`, node(`shifted-${suffix}`, 10, 'root')] as const, + ['root', { ...node('root', 0), element: 'aside[complementary]' }] as const, + ]) + ); + const captureArtifact = { + version: 1 as const, + captures: [ + { + ...capture('one', [scope('sidebar-one', 'sidebar', 200, measuredRail('-a'))]), + boxModelNodes, + }, + { + ...capture('two', [scope('sidebar-two', 'sidebar', 240, measuredRail('-b'))]), + boxModelNodes, + }, + ], + }; + const findings = createGeometryFindings( + captureArtifact, + observeGeometryCaptures(captureArtifact) + ); + + const finding = findings.findings[0]; + expect(finding?.classification).toBe('css-defect'); + expect(finding?.repairGroup).toBe(finding?.repairProposal?.repairGroup); + expect(finding?.repairGroup).toBe(geometryRepairGroupKey(finding!.repairProposal!)); + // Grouping is evidence, so the key is exactly what it was without it. + expect(finding?.key).toBe( + alignmentFindingKey({ + surfaceFamily: 'workspace', + locator: geometryIdentityLocator(finding!.locator!), + anchor: 'inline-end', + }) + ); + }); + it('explains an offset as box-model arithmetic to a common ancestor', () => { const contribution = (padding: number, border = 0) => ({ padding, @@ -235,6 +310,8 @@ describe('geometry constraint artifacts', () => { residual: 0, repair: { commonAncestor: 'aside[complementary]', + // Grouping evidence: the ticket this offset belongs to, never identity. + repairGroup: expect.stringMatching(/^geometry\/repair\//), edge: 'inline-end', terms: [ { @@ -742,6 +819,7 @@ describe('geometry constraint artifacts', () => { expect(explanation?.residual).toBe(0); expect(explanation?.repair).toEqual({ commonAncestor: 'div[role=tabpanel]', + repairGroup: expect.stringMatching(/^geometry\/repair\//), edge: 'inline-start', terms: [ { @@ -2040,7 +2118,7 @@ describe('geometry ledger ratchet', () => { }); }); -describe('geometry repair tickets', () => { +describe('geometry repair identity', () => { const proposal = ( overrides: Partial & { term?: Partial; @@ -2068,6 +2146,37 @@ describe('geometry repair tickets', () => { }; }; + it('groups by the edit, not by the element that reported it', () => { + const first = geometryRepairGroupKey(proposal()); + const sameEdit = geometryRepairGroupKey( + // A different row, a different label, a different measured value: the + // edit that closes it is the same one. + proposal({ term: { memberValue: 14, referenceValue: 12, delta: 2, side: 'reference' } }) + ); + const otherComponent = geometryRepairGroupKey( + proposal({ component: 'ProjectRow', term: { component: 'ProjectRow' } }) + ); + const otherEdge = geometryRepairGroupKey(proposal({ edge: 'inline-start' })); + const otherTerm = geometryRepairGroupKey(proposal({ term: { term: 'margin' } })); + + expect(first).toBeDefined(); + expect(sameEdit).toBe(first); + expect(otherComponent).not.toBe(first); + expect(otherEdge).not.toBe(first); + expect(otherTerm).not.toBe(first); + }); + + it('groups by the common ancestor when React rendered no component name', () => { + const withoutComponent = proposal({ component: undefined, term: { component: undefined } }); + + expect(geometryRepairGroupKey(withoutComponent)).toBeDefined(); + expect(geometryRepairGroupKey(withoutComponent)).not.toBe(geometryRepairGroupKey(proposal())); + }); + + it('has no group when there is nothing to repair', () => { + expect(geometryRepairGroupKey({ ...proposal(), terms: [] })).toBeUndefined(); + }); + it('names the CSS property an agent would actually edit', () => { expect(geometryRepairCssProperty('padding', 'inline-end')).toBe('padding-inline-end'); expect(geometryRepairCssProperty('margin', 'block-start')).toBe('margin-block-start'); @@ -2098,8 +2207,44 @@ describe('geometry repair tickets', () => { it('falls back to the common ancestor when React rendered no component name', () => { // A node React never rendered still gets a sentence, just a less precise one. expect( - formatGeometryRepairProposal(proposal({ component: undefined, term: { component: undefined } })) - ).toBe('div[data-slot=sidebar-row] 里 div[role=button] 的 padding-inline-end 多 2px（class: pe-3）'); + formatGeometryRepairProposal( + proposal({ component: undefined, term: { component: undefined } }) + ) + ).toBe( + 'div[data-slot=sidebar-row] 里 div[role=button] 的 padding-inline-end 多 2px（class: pe-3）' + ); }); + it('collects every finding one repair group covers', () => { + const artifact: GeometryFindingArtifact = { + version: 1, + findings: [ + { ...ratchetLikeFinding('geometry/workspace/a'), repairGroup: 'geometry/repair/x' }, + { ...ratchetLikeFinding('geometry/workspace/c'), repairGroup: 'geometry/repair/x' }, + { ...ratchetLikeFinding('geometry/workspace/b'), repairGroup: 'geometry/repair/y' }, + ratchetLikeFinding('geometry/workspace/d'), + ], + }; + + expect(geometryFindingKeysInRepairGroup(artifact, 'geometry/repair/x')).toEqual([ + 'geometry/workspace/a', + 'geometry/workspace/c', + ]); + expect(geometryFindingKeysInRepairGroup(artifact, 'geometry/repair/missing')).toEqual([]); + }); }); + +function ratchetLikeFinding(key: string): GeometryFinding { + return { + key, + kind: 'alignment-rail', + surfaceFamily: 'workspace', + label: key, + axis: 'x', + anchor: 'inline-end', + offset: 1, + captureCount: 1, + totalCaptureCount: 1, + evidence: [], + }; +}