refactor: remove the screenshot capture layer - #1308
Merged
Conversation
9 tasks
Astro-Han
force-pushed
the
refactor/visual-smoke-remove-capture-layer
branch
3 times, most recently
from
July 21, 2026 13:29
7ecdee5 to
5028b5e
Compare
This was referenced Jul 21, 2026
The Electron screenshot capture/baseline/diff harness was local-only, never ran in CI, and half the settings baselines were never captured — a rotting tool AGENTS.md pointed contributors to. Retire it; the visual baseline is now Storybook page stories, the regression lock is computed-style/text contract tests, and PR evidence is one live-app screenshot. Deleted (capture layer only): - scripts/capture-screenshots.mjs, diff-screenshots.mjs, check-chat-chrome-screenshot.mjs and the seven screenshots:* npm scripts - renderer auto-capture branch in app-shell-visual-smoke.ts and its waitForVisualSmokeElement helper - visualSmoke:capture IPC handler + capturePage on the main-window controller + the MAKA_VISUAL_SMOKE_AUTO_CAPTURE env passthrough - autoCaptureVariant field/parse from the fixture + VisualSmokeState Kept (the fixture seed mechanism the Playwright E2E suite and the CI audit-alignment.mjs script depend on): MAKA_VISUAL_SMOKE_FIXTURE seeders, getVisualSmokeState, applyVisualSmokeFixture, theme/locale/ timezone/reduced-motion overrides, and the visualSmoke:getState IPC. The chat-chrome seam/corner invariant guarded by check-chat-chrome-screenshot.mjs (flat shell, no sidebar border-right, 12px radius + drop-shadow) is already locked by the source-level chat-chrome-no-gradient-contract.test.ts, so deleting the pixel script drops no invariant. Refs #1303
Astro-Han
force-pushed
the
refactor/visual-smoke-remove-capture-layer
branch
from
July 21, 2026 13:41
5028b5e to
1693220
Compare
Astro-Han
added a commit
that referenced
this pull request
Jul 21, 2026
* refactor(desktop): rename visual-smoke to e2e-fixture Mechanically rename the visual-smoke identifier family to e2e-fixture across files, directories, TypeScript identifiers, IPC channels, env vars, DOM data attributes, and docs, and refresh the stale "screenshot pipeline / auto-capture" rationale comments in the fixture code so they describe the fixture's current role: Playwright E2E seeding, the CI alignment audit, and interactive fixture use. The screenshot capture/baseline/diff layer was removed in #1308; this brings the surviving fixture mechanism's text in line with that reality. Casing follows existing repo convention (E2E_USER_DATA_DIR, E2E_SCENARIOS, createCuE2eFixture): E2E stays uppercase; the env family uses the MAKA_E2E_FIXTURE_* prefix to stay distinct from the generic E2E_* runtime vars. DOM dataset access is makaE2eFixture per the HTML spec kebab-to-camel conversion. No behavior change. Refs #1303. * fix(desktop): sync e2e-fixture CSS hooks and finish the stale-text sweep Addresses codex review of #1314. - Rename the data-maka-visual-smoke CSS selectors in base.css and sidebar.css to data-maka-e2e-fixture so the deterministic-render hooks (decorative animation pause, running-status spinner stop) actually match the attribute the renderer writes. The first-pass rename grep had missed .css, so fixture renders silently went non-deterministic. - Add a contract test asserting the e2e-fixture CSS selectors exist. base.css is skipped wholesale by the !important audit (isA11yOnlyFile), which is why the missing selector was not caught. - Rename the leftover `smoke` local variables (the e2eFixture.getState() return values) to `fixtureState`, and update the two source-grep contract tests that asserted the old name. - Finish the stale-rationale sweep the first pass missed: Visual-smoke comments, fixture-context "captures / screenshot / baseline" wording, and the reduced-motion env name in base.css. Unrelated smoke terms (voice device smoke test, real-window smoke, prompt smoke) are left intact. No behavior change beyond restoring the CSS hooks the renderer already expected. Refs #1303. * fix(desktop): correct e2e-fixture casing, finish stale-text sweep, drop dead allowlist Addresses a second fresh-eye codex review of #1314. - TS casing: rename E2EFixture* -> E2eFixture* to match the repo's existing TypeScript acronym convention (withE2eWindow, createCuE2eFixture, isE2e). SCREAMING E2E_FIXTURE constants/env stay uppercase; the DOM dataset makaE2eFixture already followed this rule. The prior E2EFixture introduced a second, inconsistent casing and clashed with the dataset read. - Comment accuracy: the first refresh had rewritten "screenshot" as "E2E/audit", but Playwright only binds 7 scenarios and many fixtures have no E2E spec at all. Reworded to the real consumer — the CI alignment audit, which covers every fixture — or to a neutral "the fixture renders X" where no external consumer exists (e.g. turn-control-history). - Residual sweep the prior passes missed: docs/frontend-css-governance (.md + .zh-CN.md), chat-empty-hero.tsx, stale-sessions.ts, the smokeGate/smokeWin test-local vars, and the "smoke-only" test title. - Revert one over-rename: use-onboarding-snapshot said "covered by smoke", meaning the Playwright first-run E2E (emptyWindow, no fixture), not the e2e-fixture. Now reads "Playwright E2E". - Drop the dead app-ipc-main.ts entry from check-console.mjs's allowlist; the file has no console.* and the entry referenced the removed capture tooling. No behavior change. Refs #1303. * fix(desktop): make every fixture comment name a real consumer Addresses a third fresh-eye codex review of #1314. The prior comment refreshes had replaced "screenshot" with "alignment audit" / "E2E" without checking which scenarios those consumers actually cover. The CI alignment audit iterates only 12 scenarios and Playwright binds only 6, so most fixture comments were naming a consumer that never runs them. Reworded every comment to one of: - the real consumer, where the scenario is actually covered (module-skills, module-mcp, plan-reminders, settings-bots-onboarding -> alignment audit; long-transcript -> scroll-geometry Playwright spec); - a neutral "the fixture renders/exposes X" with no consumer claim, everywhere else. Also cleared the last screenshot/baseline/PNG-diff residue the prior passes missed (chat.tsx, chat-empty-hero, the PNG-diff note in e2e-fixture.test, search-modal-lifecycle, turn-control-matrix, visible-copy-hygiene), the "visual fixture" term in three spots, and the "Visual smoke screenshots" reference in the real-window smoke runner (it contrasted the removed automated capture layer). 2761 desktop tests + format/lint/audit green. No behavior change. Refs #1303.
This was referenced Jul 22, 2026
Astro-Han
added a commit
that referenced
this pull request
Jul 22, 2026
…in e2e
Add a Playwright E2E contract that boots the `sidebar-long-sessions`
fixture (60 sessions) and asserts the rendered sidebar geometry the
retired screenshot baseline used to guard:
- the sidebar list scroller overflows its own constrained grid row and
scrolling it moves its own scrollTop while the chat viewport's
scrollTop is unaffected (the seed opens a short session, so chat
stays pinned at 0), and
- the Settings/version footer's bounding rect stays fully inside both
the sidebar panel and the window viewport, before and after scrolling
the list to the bottom.
This locks the P0 (WAWQAQ msg `761141c5`) footer-off-screen invariant
that #1308 left unlocked when it retired the screenshot harness: the
static `sidebar-scroll-contract.test.ts` only greps the CSS, and
`scroll-geometry.spec.ts` probes the chat viewport, not the sidebar.
Adds the `sidebarLongSessionsWindow` fixture following the
`longTranscriptWindow` pattern.
Fixes #1311
Astro-Han
added a commit
that referenced
this pull request
Jul 22, 2026
…in e2e (#1317) Add a Playwright E2E contract that boots the `sidebar-long-sessions` fixture (60 sessions) and asserts the rendered sidebar geometry the retired screenshot baseline used to guard: - the sidebar list scroller overflows its own constrained grid row and scrolling it moves its own scrollTop while the chat viewport's scrollTop is unaffected (the seed opens a short session, so chat stays pinned at 0), and - the Settings/version footer's bounding rect stays fully inside both the sidebar panel and the window viewport, before and after scrolling the list to the bottom. This locks the P0 (WAWQAQ msg `761141c5`) footer-off-screen invariant that #1308 left unlocked when it retired the screenshot harness: the static `sidebar-scroll-contract.test.ts` only greps the CSS, and `scroll-geometry.spec.ts` probes the chat viewport, not the sidebar. Adds the `sidebarLongSessionsWindow` fixture following the `longTranscriptWindow` pattern. Fixes #1311
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Retires the Electron screenshot capture/baseline/diff layer. The harness was local-only, never ran in CI, and half the settings baselines were never captured — a rotting tool
AGENTS.mdpointed contributors to. This is the blocking prerequisite (checkbox 1) of #1303.After this lands the visual baseline is Storybook page stories, the regression lock is computed-style/text contract tests, and PR evidence is one live-app screenshot.
Deleted (capture layer only)
scripts/capture-screenshots.mjs,diff-screenshots.mjs,check-chat-chrome-screenshot.mjsand the sevenscreenshots:*npm scripts.apps/desktop/tests/screenshots-baseline/(24 PNGs +manifest.json, ~11 MB) that the diff script compared against — they had no remaining consumer and would mislead contributors into thinking a committed baseline still exists. The stale.gitignorecomment pointing at them is removed too.app-shell-visual-smoke.tsand itswaitForVisualSmokeElementhelper.visualSmoke:captureIPC handler +capturePageon the main-window controller + theMAKA_VISUAL_SMOKE_AUTO_CAPTUREenv passthrough +sanitizeSegment(only the capture handler used it).autoCaptureVariantfield/parser from the fixture andVisualSmokeState.scenariofield onVisualSmokeState(the main→renderer payload) — its only renderer reader was the deletedvisualSmoke.capture({ scenario })call. The fixture's internalscenario(used for seeding + workspace naming) stays.Kept (fixture seed mechanism)
MAKA_VISUAL_SMOKE_FIXTUREseeders,getVisualSmokeState,applyVisualSmokeFixture, and the theme/locale/timezone/reduced-motion overrides stay — the Playwright E2E suite and the CIaudit-alignment.mjsscript depend on them.Review focus & tracked gaps
A Codex review surfaced two rendered invariants the deleted pixel scripts were the only (CI-unenforced) lock for. Per
AGENTS.md, computed-style/text contracts are the preferred lock, so those are tracked as follow-up sub-issues of #1303 rather than restored as a pixel matrix:sidebar-long-sessionsfooter-visibility geometry invariant (thescroll-geometryE2E spec boots thelong-transcriptfixture and checks the chat viewport, not the sidebar; the source-levelsidebar-scroll-contract.test.tschecks the scroll CSS, not the rendered footer geometry).chat-chrome-no-gradient-contract.test.tsis a source-string contract (it regex-matches the first CSS rule body), so it locks the source declarations but not the rendered cascade; the rendered computed-style contract is the authoritative replacement.The
chat-chrome-no-gradient-contract.test.tssource contract is retained as a fast pre-check; #1312 upgrades the rendered half.A P3 from the same review — present-tense "screenshot pipeline" rationale comments in the kept fixture code — is deferred to the rename PR (PR2). PR2's scope is widened from a mechanical
visual-smoke→e2e-fixturerename to a textual-alignment pass that also refreshes those stale rationale comments, since it already rewrites everyvisual-smokereference; sweeping the comments here and re-touching them in PR2 would be duplicated churn.Verification
npm --workspace @maka/desktop run typecheck(preload + main + renderer + storybook tsconfigs) — green.npm run format:checkandnpm run lint(Biome) — green, no fixes.node --test dist/main/**/*.test.js(the CItest:distnode suite) — 2760/2760 pass (fixture, chat-chrome, sidebar-scroll, command-palette, preload, storybook, real-window contracts).node scripts/audit-alignment.mjs— all fixtures clean (the CI path that reuses the fixture mechanism).scroll-geometry(long-transcript),settings(settings-bots),permission-takeover(permission-destructive),locale-renderer(allfixture, zh+en),bot-onboarding(settings-bots-onboarding QR fixture) — 15/15 pass.npm run build-storybook— the new baseline surface builds.Not run: the full E2E suite and packaged-build smoke (out of scope for a dev-only tooling deletion; the fixture mechanism E2E above covers the regression risk).
Refs #1303.