Uh oh!
There was an error while loading. Please reload this page.
test(windows): repair and enforce UI config contract - #829
Conversation
PR Reviewer Guide 🔍(Review updated until commit 12d0bf8)Here are some key observations to aid the review process:
|
appergb
left a comment
There was a problem hiding this comment.
Independent review — exact head 12d0bf8
Strengths
- Strict RED was independently reproduced on PR base d6ca07a. The original contract exited 1 at the real current Tauri value with: expected 220, got 460. The base package also had no check:windows-ui-config script.
- Exact-head GREEN passed both direct Node execution and npm run check:windows-ui-config under Node 22. The script has valid Node syntax, package.json parses, and ci.yml parses.
- The repaired values match the actual product intent introduced by #761, not merely the current incidental text: tauri.conf.json uses 460x180; capsuleLayout.ts defines and returns the shared 460x180 stage with zero host insets; Capsule.tsx consumes the host metrics and proportional badge anchor; native capsule_window_bounds uses 460x180/0 and capsule_visual_height uses 140. Rust already has executable unit tests for those native values.
- The flush-shell assertion matches the intentional ol-console-main treatment in FloatingShell.tsx: radius 0 and no shadow. The new native/frontend assertions replace stale unreachable checks rather than deleting geometry coverage.
- The workflow step is inside the existing macOS/Windows/Linux matrix after clean npm ci and uses only the cross-platform Node script. GitHub job-step evidence shows Check Windows UI configuration contract completed successfully on all three operating systems.
- package.json only adds a named check entry; ci.yml only adds the contract step. No production source, lockfile, release workflow or runtime configuration changed.
- Current beta advanced after this head only through #828. A merge-tree against current beta is conflict-free, and the combined package.json retains both the pinned local tsx runner and this Windows UI contract script.
- Clean npm ci, the Windows and macOS contracts, Android IPC prebuild, npm run build and npm audit --audit-level=high all passed; audit found zero vulnerabilities.
- All five checks at the exact reviewed SHA are completed and successful: Windows, macOS, Linux, Android cargo check and PR-Agent. CI run: https://github.com/Open-Less/openless/actions/runs/29391799716
Critical
None.
Important
None.
Minor
None.
Ready-to-merge verdict
COMMENTED — logical APPROVE. No blocking finding at the reviewed exact head. The change satisfies issue #803, locks the current frontend/native Windows geometry intent without weakening coverage, runs portably in the cross-platform CI matrix, and has no product behavior side effects. It is ready from a code-review perspective subject to normal Draft and repository policy.
Uh oh!
There was an error while loading. Please reload this page.
Persistent review updated to latest commit 12d0bf8 |
User description
Summary
check:windows-ui-configand run it in every cross-platform CI matrix jobTDD evidence
RED on current Beta: the existing test stopped immediately at
expected 220, got 460.After repairing only that first assertion, the test exposed two more stale assumptions that the original issue could not reach: it still expected the pre-1.3.14 floating radius token and the old 196px Windows pill with 12px insets. Current product code intentionally uses a flush main panel plus the shared 460x180 voice-orb stage. The repaired contract now asserts those exact frontend and native values instead of deleting geometry coverage.
GREEN: the complete contract passes through the new npm script.
Verification
npm run check:windows-ui-config: passednpm run check:macos-capsule-spaces: passednpm ci: 0 vulnerabilitiesnpm run build: passed, including Android IPC prebuild boundarynpm audit --audit-level=high: 0 vulnerabilitiesnode --checkandgit diff --check: passedTest and CI only; no product or UI behavior change.
Closes#803
PR Type
Bug fix, Tests
Description
Update stale capsule bootstrap size assertion from 220x110 to 460x180
Replace legacy Windows assertions with exact flush main shell and shared voice-orb geometry contracts
Add
check:windows-ui-confignpm script and run it in cross-platform CIDiagram Walkthrough
File Walkthrough
ci.yml
Add Windows UI config check to CI.github/workflows/ci.yml
package.json
Add npm script for Windows UI contractopenless-all/app/package.json
windows-ui-config.test.mjs
Repair and modernize Windows UI config testsopenless-all/app/scripts/windows-ui-config.test.mjs
(460x180, zero insets)
and 140px visual height assertions
proportional badge positioning