UN-3797 [MISC] Add a browser-based frontend layout test tier#2196
Open
athul-rs wants to merge 4 commits into
Open
UN-3797 [MISC] Add a browser-based frontend layout test tier#2196athul-rs wants to merge 4 commits into
athul-rs wants to merge 4 commits into
Conversation
The meta cluster is anchored on the row's right edge, so content-sized columns shifted "Owned By" sideways row by row depending on the length of the owner label. Pin the owner and updated columns; bold the title to match the agentic Prompt Studio project list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The meta wrapper was still shrinkable, so once its columns became fixed they overflowed the box and overlapped the action icons (26px at 1440, 66px at 1024). Pin the wrapper too and let the left column, which ellipsizes, absorb the shortfall. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…TS (#2188) * UN-3636 [FEAT] Merge overlay manifests via UNSTRACT_RIG_EXTRA_MANIFESTS load_groups() now merges extra group manifests listed in the env var (os.pathsep-separated, REPO_ROOT-relative) onto the base tests/groups.yaml before validation, so cross-manifest depends_on and the platform-gate invariant are checked over the union. Name collisions are an error. Lets a downstream repo (the cloud build) contribute its own test groups by copying a groups.cloud.yaml into the merged tree, without editing the OSS manifest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz * UN-3636 [FIX] Skip org lookup when no org in context UserContext.get_organization() ran Organization.objects.get() even with no org id in StateStore (import time, or management commands with no request), catching only DoesNotExist/ProgrammingError — a DB-less/unmigrated setup hit an uncaught OperationalError. Short-circuit when there's no org id: no query, so serializers/managers that reference org-scoped querysets at class-def can be imported during DB-free test collection. * UN-3636 [FIX] Scope overlay manifests to the default manifest Address review feedback on the UNSTRACT_RIG_EXTRA_MANIFESTS overlay: - Overlays now apply only when loading the default manifest, so an explicit `load_groups(path)` (test fixture, ad-hoc manifest) can no longer absorb a downstream repo's ambient overlay. - `_merge_manifest` returns the merged defaults so an overlay can rename `platform_gate_group` instead of having it silently ignored. - A bad path in the env var raises a ValueError naming the variable rather than a bare FileNotFoundError/IsADirectoryError. - Extract `_load_manifest_dict` to single-source manifest parsing and its error message. - Tests: drive the real default-manifest path; cover overlay isolation, overlay defaults, malformed overlay, and a missing overlay path. - Pin the truthy branch of UserContext.get_organization so inverting the guard can't pass unnoticed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz * UN-3636 [FIX] Stop re-running cross-tier deps in every tier leg `--tier X` expanded each selected group's `depends_on` transitively, with no tier bound. `integration-workflow-execution` and `e2e-smoke` both declare `depends_on: [unit-sdk1, unit-workers]`, so those two unit groups ran again in the integration leg and a third time in the e2e leg. Tiers run as separate CI legs and the unit leg already covers them, so dep expansion is now bounded to the requested tier. Explicitly named groups are never dropped, and intra-tier deps (e2e-smoke -> e2e-login) still expand and order as before. Unrun deps do not weaken gating: `blocked_by` intersects with groups that failed in the same run. Measured on the last main run: ~88s of unit-workers and ~48s of unit-sdk1 re-executed per run. On the cloud CI runner the same duplication costs ~350s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz * UN-3636 [MISC] Drop the ENVIRONMENT gate on the LLM mock (#2191) * UN-3636 [FIX] Drop the ENVIRONMENT gate on the LLM mock It did not defend the case it was added for. The threat was a worker env block copied out of the test overlay into a real deployment, but the gate was written into that same block, so a copy carries it. Base compose also sets ENVIRONMENT=development on both workers that run the injection, so any deployment derived from it satisfied the gate regardless. That left one real case -- the mock var set alone somewhere that sets no ENVIRONMENT at all -- which holds by accident rather than design, in exchange for depending on a variable nothing else in the codebase reads. What actually guards the hatch is unchanged: it is off unless someone sets UNSTRACT_LLM_MOCK_RESPONSE, and it warns once per process while active. Making mocked spend distinguishable downstream is the defence worth having, and it belongs on the usage record rather than in a config check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * UN-3636 [MISC] Drop the unread ENVIRONMENT variable from compose Nothing reads it: no service, worker, frontend or plugin looks the variable up, and the one consumer it ever had — the LLM mock gate — was removed in the previous commit. Dropping it everywhere keeps a dead knob from looking load-bearing to the next reader. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ryg9chVDJQggCybpq3YoY3 --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * UN-3636 [MISC] Tighten code comments Drop lines that restate the code, trim session-specific detail, and merge comments that duplicated each other across a module and its test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz * UN-3636 [FIX] Gate overlay merging on an omitted path, not its value `load_groups(DEFAULT_MANIFEST)` merged overlays even though the caller named a manifest explicitly, because the check compared path values. Path equality is also spelling-sensitive, so the same file relative and absolute behaved differently. Key on `path is None` instead: an explicit path loads exactly what it names. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ryg9chVDJQggCybpq3YoY3 * UN-3636 [FIX] Stop ide_prompt_complete tests from hitting the network TestIdePromptComplete drives the full success path, which reaches client_plugin_registry.get_client_plugin("subscription_usage"). In OSS no such plugin is installed, so the lookup returns None instantly. In a tree with the cloud plugins copied in, it resolves to a real plugin that POSTs to the backend; with no backend running the call only fails after a multi-second connect timeout, and _track_subscription_usage swallows the error so the tests still pass. That accounted for ~190s of the cloud unit-workers run. The file already declared _PATCH_GET_PLUGIN but never applied it outside the dedicated subscription-usage classes. Apply it as a class-scoped fixture so the lookup is pinned to the OSS answer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughThe pull request adds browser-based frontend layout testing and CI gating, extracts optional plugin resolution, guards organization lookups without context, supports rig manifest overlays and tier-aware selection, removes environment-based LLM mock restrictions, and isolates plugin-dependent callback tests. ChangesFrontend validation
Runtime guards and test isolation
Rig manifest and selection
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant PathFilters
participant FrontendJob
participant Vitest
participant Playwright
participant ReportJob
GitHubActions->>PathFilters: evaluate frontend/** changes
PathFilters-->>GitHubActions: frontend relevance output
GitHubActions->>FrontendJob: run frontend tests
FrontendJob->>Vitest: run unit suite
FrontendJob->>Playwright: run Chromium layout suite
Playwright-->>FrontendJob: upload failure screenshots
FrontendJob-->>ReportJob: frontend result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
| Filename | Overview |
|---|---|
| .github/workflows/ci-test.yaml | New frontend job wires vitest unit + layout tiers into CI; all actions except actions/cache are pinned to commit SHAs, inconsistent with the repo's pinning convention. |
| frontend/vitest.config.mjs | Splits vitest into unit (happy-dom) and layout (real Chromium/playwright) projects; reuses optionalPluginImports so cloud-plugin stubs throw on import — the correct signal for runtime fallbacks. |
| frontend/src/test-utils/layout/mount.jsx | Width-convergence harness with proper onTestFinished cleanup; iterative proportional correction handles percentage-based sizing, and the post-loop clamp assertion prevents false-broad coverage. |
| frontend/src/test-utils/layout/assertions.js | Five relational geometry assertions using a shared 1.5px epsilon; all include coordinates and text content in failure messages for fast diagnosis. |
| frontend/src/components/widgets/list-view/ListView.layout.test.jsx | 13 relational layout tests covering column alignment, containment, overlap, overflow and truncation across 800/1100/1400px widths; fixture guard prevents alignment assertions from silently passing on identical labels. |
| frontend/vite-plugins/optional-plugin-imports.js | Byte-identical extraction of optionalPluginImports from vite.config.js into its own module so vitest.config.mjs can share it. |
| unstract/sdk1/src/unstract/sdk1/llm.py | Removes the two-condition ENVIRONMENT gate for mock responses; UNSTRACT_LLM_MOCK_RESPONSE being set is now the only condition, with documented rationale. |
| docker/docker-compose.yaml | Removes ENVIRONMENT=development from all 11 services now that no code reads the variable. |
| tests/rig/groups.py | Adds overlay manifest support via UNSTRACT_RIG_EXTRA_MANIFESTS; overlays are merged before validation so cross-manifest depends_on is checked over the full union; name collisions are rejected. |
| tests/rig/selection.py | Adds tier-scoped dep expansion: dep-injected groups from other tiers are filtered out while explicit positional requests always survive. |
| backend/utils/user_context.py | Adds an early-return guard for falsy organization_id to avoid a DB query on DB-less/unmigrated setups; pinned by new regression tests. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Push / PR] --> B[changes job\npaths-filter]
B -->|frontend/**| C[frontend job]
B -->|backend/**| D[test job\nPython unit/integration]
B -->|any non-docs| E[e2e job]
C --> C1[bun install --frozen-lockfile]
C1 --> C2[Cache Playwright browsers]
C2 --> C3[playwright install chromium --with-deps]
C3 --> C4[bun run test:unit\nvitest · happy-dom]
C4 --> C5[bun run test:layout\nvitest · real Chromium]
C5 -->|on failure| C6[Upload __screenshots__]
D --> F[report job]
E --> F
C --> F
F --> G{All pass/skip?}
G -->|yes| H[Branch protection: green]
G -->|no| I[Branch protection: red]
Reviews (2): Last reviewed commit: "UN-3797 [MISC] Add a browser-based front..." | Re-trigger Greptile
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci-test.yaml:
- Around line 161-181: Regenerate and commit frontend/bun.lock so it includes
the declared `@vitest/browser` and playwright versions, then update the Install
dependencies step to run bun install --frozen-lockfile. Keep the existing
Playwright cache key based on the updated lockfile.
In `@tests/compose/docker-compose.test.yaml`:
- Around line 20-27: Update the UNSTRACT_LLM_MOCK_RESPONSE environment entries
for both worker-executor-v2 and worker-file-processing-v2 to fail closed when
the variable is absent, using MOCK_LLM_OK as the default or enforcing startup
failure for an empty value. Preserve the guarantee that execute-path E2E runs
cannot reach a real provider.
In `@tests/rig/groups.py`:
- Around line 171-178: Update _load_manifest_dict to validate that raw["groups"]
is a mapping in addition to validating the top-level manifest and groups key.
Raise the existing manifest-specific ValueError for non-mapping groups values,
preserving consistent validation for base and overlay manifests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f9a127f-ee0e-475d-87c2-a38cac16d795
📒 Files selected for processing (23)
.github/workflows/ci-test.yaml.gitignorebackend/utils/tests/test_user_context.pybackend/utils/user_context.pydocker/docker-compose.yamlfrontend/package.jsonfrontend/src/components/widgets/list-view/ListView.cssfrontend/src/components/widgets/list-view/ListView.layout.test.jsxfrontend/src/test-utils/layout/assertions.jsfrontend/src/test-utils/layout/mount.jsxfrontend/src/test-utils/layout/setup.jsfrontend/vite-plugins/optional-plugin-imports.jsfrontend/vite.config.jsfrontend/vitest.config.mjstests/README.mdtests/compose/docker-compose.test.yamltests/rig/groups.pytests/rig/selection.pytests/rig/tests/test_groups.pytests/rig/tests/test_selection.pyunstract/sdk1/src/unstract/sdk1/llm.pyunstract/sdk1/tests/test_mock_response.pyworkers/tests/test_ide_callback.py
💤 Files with no reviewable changes (3)
- unstract/sdk1/src/unstract/sdk1/llm.py
- unstract/sdk1/tests/test_mock_response.py
- docker/docker-compose.yaml
Comment on lines
+20
to
27
| # Execute-path e2e must never reach a real provider. | ||
| worker-executor-v2: | ||
| environment: | ||
| - ENVIRONMENT=test | ||
| - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-} | ||
|
|
||
| worker-file-processing-v2: | ||
| environment: | ||
| - ENVIRONMENT=test | ||
| - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-} |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail closed when the mock variable is absent.
${UNSTRACT_LLM_MOCK_RESPONSE:-} expands to an empty value, which the LLM adapter treats as unset; a compose run outside the rig can therefore call a real provider despite the comment’s “must never” guarantee. Default both worker services to MOCK_LLM_OK or fail startup when the variable is empty.
Proposed fix
- - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-}
+ - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-MOCK_LLM_OK}Apply the same change to worker-file-processing-v2.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Execute-path e2e must never reach a real provider. | |
| worker-executor-v2: | |
| environment: | |
| - ENVIRONMENT=test | |
| - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-} | |
| worker-file-processing-v2: | |
| environment: | |
| - ENVIRONMENT=test | |
| - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-} | |
| # Execute-path e2e must never reach a real provider. | |
| worker-executor-v2: | |
| environment: | |
| - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-MOCK_LLM_OK} | |
| worker-file-processing-v2: | |
| environment: | |
| - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-MOCK_LLM_OK} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/compose/docker-compose.test.yaml` around lines 20 - 27, Update the
UNSTRACT_LLM_MOCK_RESPONSE environment entries for both worker-executor-v2 and
worker-file-processing-v2 to fail closed when the variable is absent, using
MOCK_LLM_OK as the default or enforcing startup failure for an empty value.
Preserve the guarantee that execute-path E2E runs cannot reach a real provider.
Comment on lines
+171
to
+178
| def _load_manifest_dict(manifest_path: Path) -> dict[str, Any]: | ||
| """Parse a manifest YAML, rejecting anything that is not a ``groups:`` mapping.""" | ||
| if not manifest_path.is_file(): | ||
| raise ValueError(f"{manifest_path}: manifest file not found") | ||
| raw = yaml.safe_load(manifest_path.read_text()) | ||
| if not isinstance(raw, dict) or "groups" not in raw: | ||
| raise ValueError(f"{manifest_path}: expected top-level `groups:` mapping") | ||
| return raw |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate that groups is a mapping.
groups: [] passes this check and later raises AttributeError at Line 130. Reject non-mapping values here so base and overlay manifests consistently raise the intended ValueError.
Proposed fix
- if not isinstance(raw, dict) or "groups" not in raw:
+ if not isinstance(raw, dict) or not isinstance(raw.get("groups"), dict):
raise ValueError(f"{manifest_path}: expected top-level `groups:` mapping")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _load_manifest_dict(manifest_path: Path) -> dict[str, Any]: | |
| """Parse a manifest YAML, rejecting anything that is not a ``groups:`` mapping.""" | |
| if not manifest_path.is_file(): | |
| raise ValueError(f"{manifest_path}: manifest file not found") | |
| raw = yaml.safe_load(manifest_path.read_text()) | |
| if not isinstance(raw, dict) or "groups" not in raw: | |
| raise ValueError(f"{manifest_path}: expected top-level `groups:` mapping") | |
| return raw | |
| def _load_manifest_dict(manifest_path: Path) -> dict[str, Any]: | |
| """Parse a manifest YAML, rejecting anything that is not a ``groups:`` mapping.""" | |
| if not manifest_path.is_file(): | |
| raise ValueError(f"{manifest_path}: manifest file not found") | |
| raw = yaml.safe_load(manifest_path.read_text()) | |
| if not isinstance(raw, dict) or not isinstance(raw.get("groups"), dict): | |
| raise ValueError(f"{manifest_path}: expected top-level `groups:` mapping") | |
| return raw |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/rig/groups.py` around lines 171 - 178, Update _load_manifest_dict to
validate that raw["groups"] is a mapping in addition to validating the top-level
manifest and groups key. Raise the existing manifest-specific ValueError for
non-mapping groups values, preserving consistent validation for base and overlay
manifests.
List rows are a table drawn with flexbox: columns only line up across rows because their widths are pinned, and a wrapper that shrinks lets pinned children spill onto the row actions. Both failure modes shipped recently and were caught by human QA — neither is visible in a CSS diff, and neither can be caught by the existing vitest setup, whose happy-dom environment measures every rectangle as zero. Adds a second vitest project running real Chromium via playwright, asserting the geometric contracts of ListView at the widths its own CSS claims to support: columns are straight, metadata stays inside its box and clear of the action icons, rows never overflow, over-long owner labels truncate instead of displacing the layout. Assertions are relational, never absolute pixels — a redesign that moves the columns keeps passing, only a column that stops being a column fails. The shared helpers in test-utils/layout are component-agnostic. Wires both projects into the existing ci-test workflow as a `frontend` job, gated by a frontend/** paths filter and joined to the report job's merge gate, so every PR touching the frontend runs them and a red assertion blocks. This is also the first CI execution of the pre-existing unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
athul-rs
force-pushed
the
UN-3741-fe-layout-tests
branch
from
July 22, 2026 20:47
cd8ef15 to
71195c9
Compare
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
Contributor
Unstract test resultsPer-group results
Critical paths
|
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
3 participants
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.
What
Adds a second vitest project that runs the UI in real Chromium and asserts the
geometric contracts of the shared
ListViewrow: that its columns line upacross rows, that the metadata cluster stays inside its own box and clear of the
action icons, that rows never overflow, and that an over-long owner label
truncates instead of displacing the layout.
Also wires the frontend tests into
ci-test.yamlas afrontendjob. This isthe first time any vitest test runs in CI — the existing ones have only ever run
on dev machines.
Why
Two CSS regressions reached staging in the last week and were both caught by
human QA rather than by CI:
grows to absorb slack, so the metadata cluster is anchored on the row's right
edge — which means a content-sized owner column starts at a different x on
every row. Measured drift between a row owned by "Me" and one owned by a long
email address: 181–222px.
shrinkable. A shrinkable box with rigid children lets the children spill out
of it and print on top of the action icons: +26px at 1440, +66px at 1024.
Neither is visible when reading the CSS diff, and neither is catchable by the
existing test setup —
vitest.config.mjsruns on happy-dom, which has no layoutengine, so every
getBoundingClientRect()returns zeros. No amount of unittesting in that environment can see either bug.
How
vitest.config.mjsnow defines two projects:@vitest/browser+playwright, running only*.layout.test.jsx.src/test-utils/layout/holds component-agnostic helpers:expectAlignedX,expectContained,expectNoHorizontalOverlap,expectNoHorizontalOverflow,expectTruncated, and amountAtWidthharness that renders a component at arequested width and waits for fonts and antd's runtime styles to settle.
Every assertion is relational, never absolute. Nothing asserts a pixel
position, a padding, a gap, or a CSS property value — a redesign that moves the
columns keeps passing, and only a column that stops being a column fails. This
is deliberate: absolute-pixel layout tests get deleted within a month of the
first innocuous design tweak.
The production build's
optionalPluginImportsplugin moved tofrontend/vite-plugins/sovitest.config.mjscan reuse it. Tests need thesame treatment the build gives absent cloud plugins — importantly, it makes a
missing plugin throw, which is the signal
pluginRegistryand ~30 otherruntime fallbacks match on. A lookalike stub that resolved to an empty module
instead would silently push every one of those fallbacks down the wrong branch.
Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
No product code changes — the diff is tests, test config, CI, and one
build-config extraction.
The extraction is the only line of risk:
optionalPluginImportsmoved out ofvite.config.jsintovite-plugins/optional-plugin-imports.jsand is importedback. The function body is byte-identical apart from added braces on three
single-line returns.
npx vite buildwas run before and after and succeeds.CI risk is one new job. It is gated on
frontend/**, so non-frontend PRs skipit (skip reads as pass, per the existing filter convention), and it runs in
parallel with the python tiers — no added wall-clock on a PR that also runs e2e.
Database Migrations
None.
Env Config
None.
Relevant Docs
None.
Related Issues or PRs
Depends on #2193 (the CSS fix these tests hold in place, UN-3741). Follows #2182.
Tracked under UN-3797, a sub-task of UN-3635 alongside the python rig work.
Dependencies Versions
Two new frontend devDependencies:
@vitest/browser@3.2.6(matches the pinnedvitest) and
playwright@^1.61.1. CI caches~/.cache/ms-playwrightand installsonly Chromium.
bun.lockis regenerated.@vitest/browseris pinned exactly rather than witha caret: its peer dependency on vitest is an exact version, so a caret would
resolve to 3.2.7 against the repo's vitest 3.2.6 and warn on every install.
Notes on Testing
The suite was validated against three states of the CSS it protects, to prove it
fails for the right reason rather than merely passing:
main(drift bug live)main+ first fix commit (columns pinned, wrapper still shrinkable)main+ complete #2193Each bug is caught by the assertion aimed at it, and neither masks the other.
Harness fidelity was checked against a deployed build in a dev namespace: at a
900px list the harness reproduces
owner 240px / updated 170px / meta 430px / 24px clearance to the actions— the same numbers measured live in the browseron the running app.
Runtime: 29 tests (16 existing unit + 13 layout) in ~3s locally. The CI job is
~2–4 minutes, dominated by dependency install and the Chromium fetch on a cold
cache.
Widths tested are 800 / 1100 / 1400 — the min and max
.list-view-wrapperitself declares, plus the midpoint. The harness asserts it actually achieved the
requested width, so a clamp can't silently collapse three cases into one.
What this does not cover
shell, so a page-level stylesheet override or a sidebar squeezing the content
area is still QA's to catch.
src/plugins/**(gitignored cloud content, absent in CI) and non-Chromiumbrowsers.
ListView. The helpers are reusable; add a*.layout.test.jsxbeside a component when it earns one.Screenshots
Failure output is the interesting artifact here — an assertion reports the
measured drift and the offending rows, and vitest writes a screenshot of the
failing render, uploaded as a CI artifact:
Checklist
I have read and understood the Contribution Guidelines.