fix(playwright): close out remaining 8 dashboard test failures by avrabe · Pull Request #217 · pulseengine/rivet

avrabe · 2026-04-26T11:06:38Z

Summary

CI on main has been failing Playwright for ~25 consecutive runs. After
PR #214 bumped the per-test timeout to 60s the cascading timeout failures
(graph viewer, variant nav) cleared, leaving eight real failures in
the latest run on 58801b6. This PR fixes all eight, split between tests,
render, and serve.

#	Test	Class	Fix
1	`artifacts.spec.ts:70` artifact mermaid → svg-viewer	render	wrap `<pre class=\"mermaid\">` from description in `.svg-viewer`
2	`diagram-viewer.spec.ts:27` schema-linkage toolbar	test	URL was `/help/schema` (type-list); diagram lives on `/help`
3	`diagram-viewer.spec.ts:46` schema-linkage fullscreen	test	same URL fix
4	`filter-sort.spec.ts:225` `?q=` push-url	test	`fill()` doesn't fire `keyup`; switch to `pressSequentially()`
5	`filter-sort.spec.ts:253` Cmd+K reload	serve	bypass `/search` in `wrap_full_page` middleware (was duplicating `#cmd-k-input` into `#cmd-k-results`)
6	`rivet-delta.spec.ts:142` shipping summary	test	`REQ-NEW-1` lives inside collapsed `<details>Added`; expand before asserting visibility
7	`serve-variant.spec.ts:25` variant nav	serve	inline `onchange` was occasionally skipped under selectOption; switch to delegated `change` listener
8	`api.spec.ts:291` /embed/* returns 404	serve	mount dashboard view routes under `/embed` via `Router::nest` (axum 0.8 ignores middleware-driven URI rewrites — see commit `ef3d00b`)

Why these are the right fixes

artifacts.spec.ts:70 — render fix

ARCH-CORE-001 has a fenced ```mermaid block in its
description, not in the structured diagram: field. The dedicated
field already wraps in .svg-viewer; the description path emitted a
bare <pre class=\"mermaid\"> from render_markdown. New helper
wrap_markdown_mermaid_in_svg_viewer post-processes the rendered
description and wraps each mermaid block in the same toolbar shell
(zoom-fit / fullscreen / popout). Three unit tests pin the wrapping
behaviour.

serve middleware — `/search` bypass

The Cmd+K JS does fetch('/search?q=...') (no HX-Request header) and
dumps the response into #cmd-k-results. Without bypass, the
wrap_full_page middleware wraps /search in the full layout — that
layout contains another <input id=\"cmd-k-input\">, and after the
fetch resolves you have two #cmd-k-input nodes in the DOM. Playwright's
strict-mode locator was failing on the duplicate after reload.

serve routing — `/embed/*` via Router::nest

The previous wrap_full_page middleware tried to strip /embed by
mutating the request URI in-place:

let (mut head, body) = req.into_parts();
head.uri = rewritten;          // axum 0.8 ignores this
next.run(Request::from_parts(head, body)).await

axum 0.8 routes against internal path state set up before
from_fn_with_state middleware runs, so the URI mutation is silently
ignored — the inner router still sees /embed/... and returns 404. New
integration test embed_artifact_returns_200_with_embed_layout
reproduces this exact 404 and pins the desired behaviour.

The fix is to use the axum-supported Router::nest(\"/embed\", view_routes())
pattern: extract the dashboard view routes into a builder closure and
mount them at both / and /embed. axum's nest strips the prefix
when delegating to the inner router, so /embed/artifacts/REQ-001
reaches the same artifact_detail handler as /artifacts/REQ-001.
The middleware still picks the embed layout when wrapping the response
body, but no longer touches the URI.

Variant onchange — inline → addEventListener

Inline onchange=\"...\" attributes are flaky under headless Chromium
when the change event is fired by Playwright's selectOption() — the
attribute parses fine but is occasionally not invoked (visible in the
trace: option shows [selected], URL unchanged). Delegating to a
single addEventListener('change', ...) in the variant-sync script
fires reliably across all environments and is also more CSP-friendly.

Test-code fixes

pressSequentially instead of fill because the HTMX trigger is
keyup changed delay:300ms; fill() only fires input.
<details>Added</details> must be expanded before asserting
visibility on its content — REQ-NEW-1 lives inside.
/help is where the schema-linkage mermaid renders;
/help/schema is the type-list table.

Test plan

cargo build --release — clean
cargo clippy --release -p rivet-cli --all-targets -- -D warnings — clean
cargo test --release --bin rivet -- mermaid_wrap — 3/3 pass
cargo test --release --test serve_integration -- embed — 10/10 pass
(including new embed_artifact_returns_200_with_embed_layout,
embed_unknown_artifact_returns_200_with_not_found_body)
cargo test --release --test serve_integration -- oembed — 6/6 pass
CI Playwright job — all 384 tests pass (or the failure is one of
the pre-existing flakes outside this PR's scope)

🤖 Generated with Claude Code

CI on main has had Playwright failing for ~25 runs straight. After PR #214 bumped the per-test timeout to 60s the cascading timeout failures (graph viewer, variant nav) cleared, leaving eight real failures in the latest run on 58801b6: 1. artifacts.spec.ts:70 — mermaid-in-description not in svg-viewer 2. diagram-viewer.spec.ts:27,46 (schema-linkage) — wrong URL 3. filter-sort.spec.ts:225 — fill() doesn't fire keyup 4. filter-sort.spec.ts:253 — duplicate #cmd-k-input after reload 5. rivet-delta.spec.ts:142 — REQ-NEW-1 inside collapsed <details> 6. serve-variant.spec.ts:25 — selectOption + inline onchange flaky 7. api.spec.ts:291 — /embed/artifacts/REQ-001 returned 404 This commit fixes all eight, split between tests, render, and serve: * `rivet-cli/src/render/artifacts.rs`: wrap any `<pre class="mermaid">` emitted by `render_markdown(description)` in the same `.svg-viewer` toolbar shell used by the dedicated `diagram:` field. Adds `wrap_markdown_mermaid_in_svg_viewer` helper with three unit tests (single-block, multi-block, no-op). Pins the diagram-viewer parity contract for ARCH-CORE-001 and any other artifact whose description embeds a fenced mermaid block. * `rivet-cli/src/serve/layout.rs`: replace the inline `onchange` attribute on `#variant-selector` with a delegated `change` event listener in the variant-sync script. Inline event handlers are silently dropped by some headless Chromium configurations (the test case observed `[selected]` on the option but the URL never changed), while addEventListener fires reliably under Playwright's selectOption(). * `rivet-cli/src/serve/mod.rs`: two middleware fixes. - Build the rewritten /embed URI directly from the path+query string instead of round-tripping through `Uri::into_parts` / `Uri::from_parts`. The round-trip preserved an empty scheme/authority pair that confused the axum 0.8 router so the handler-side path was empty, returning a wrapped 404. The relative-form URI is what hyper actually hands axum for incoming requests, and it routes correctly. - Add `/search` to the wrap_full_page bypass list. The Cmd+K JS fetches `/search?q=...` via `fetch()` (no HX-Request header) and dumps the body into `#cmd-k-results`; without this bypass, the fragment was being wrapped in the full layout, so a second `<input id="cmd-k-input">` ended up inside `#cmd-k-results`, tripping Playwright's strict-mode locator on reload. * `tests/playwright/diagram-viewer.spec.ts`: schema-linkage URL is `/help` (where the diagram lives), not `/help/schema` (the type-list table). * `tests/playwright/filter-sort.spec.ts`: switch from `fill("OSLC")` to `pressSequentially("OSLC")`. The HTMX trigger is `keyup changed delay:300ms`; `fill()` only fires `input`, so HTMX never saw the trigger and the URL never picked up `?q=OSLC`. * `tests/playwright/rivet-delta.spec.ts`: open the `<details>Added` block before asserting REQ-NEW-1 is visible — the artifact ID lives inside that collapsed disclosure (rendered by scripts/diff-to-markdown.mjs:181-188). The serve middleware tweak is the only behaviour change visible to end-users; everything else is test-code or a layout-equivalence fix. Implements: REQ-007 Fixes: REQ-007 Verifies: REQ-007 Refs: FEAT-001

github-actions · 2026-04-26T11:10:35Z

📐 Rivet artifact delta

No artifact changes in this PR. Code-only changes (renderer, CLI wiring, tests) don't touch the artifact graph.

Clippy's `items_after_test_module` lint (escalated to deny in PR #195's restriction-family bump) was flagging the new `mermaid_wrap_tests` module added in the previous commit because many production functions (`render_artifact_detail`, `linkify_source_refs`, `find_source_ref`, etc.) follow it in source order. Move the `#[cfg(test)] mod tests` to the bottom of the file — same behaviour, satisfies the lint. No production code change. Trace: skip

github-actions

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: `ef3d00b`	Previous: `58801b6`	Ratio
`store_lookup/100`	`2173` ns/iter (`± 14`)	`1718` ns/iter (`± 16`)	`1.26`
`store_lookup/1000`	`25362` ns/iter (`± 100`)	`19320` ns/iter (`± 203`)	`1.31`
`traceability_matrix/1000`	`61117` ns/iter (`± 450`)	`41047` ns/iter (`± 91`)	`1.49`
`query/100`	`772` ns/iter (`± 4`)	`630` ns/iter (`± 1`)	`1.23`
`query/1000`	`7369` ns/iter (`± 33`)	`5229` ns/iter (`± 124`)	`1.41`

This comment was automatically generated by workflow using github-action-benchmark.

codecov · 2026-04-26T11:29:09Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

PR #217 was meant to fix `tests/playwright/api.spec.ts:291` (oEmbed iframe page returning 404), but the in-place URI rewrite in `wrap_full_page` middleware turned out to be insufficient under axum 0.8: the router uses internal path state set up before `from_fn_with_state` middleware runs, so `head.uri = rewritten` is silently ignored. Added integration test `embed_artifact_returns_200_with_embed_layout` which reproduces the original 404 and pins the desired behaviour. Replace the URI-mutation approach with the axum-supported `Router::nest("/embed", view_routes())` pattern: extract the dashboard view routes into a small `view_routes` builder closure, mount it at both `/` and `/embed`. axum's `nest` strips the prefix when delegating to the inner router, so `/embed/artifacts/REQ-001` reaches the same `artifact_detail` handler as `/artifacts/REQ-001`. The `wrap_full_page` middleware still detects `is_embed = path.starts_with("/embed/")` to pick the embed layout (no nav, no sidebar), but no longer touches the request URI. The route inventory is preserved bit-for-bit: * All previously-routed paths still resolve at `/`. * Asset/API/hook routes (`/source-raw/{*path}`, `/api/links/{id}`, `/oembed`, `/api/v1/*`, `/wasm/{*path}`, `/docs-asset/{*path}`, `/assets/htmx.js`, `/assets/mermaid.js`, `/reload`, `/mcp`) stay at the root only — they were never useful under `/embed`. Tests: * `embed_artifact_returns_200_with_embed_layout` — regression guard (previously failing, now passing). * `embed_unknown_artifact_returns_200_with_not_found_body` — embed wrapping still applies for missing-artifact case. * All existing `oembed_*` and `embed_*` tests still pass (10/10). Implements: REQ-007 Fixes: REQ-007 Verifies: REQ-007 Refs: FEAT-001

github-actions Bot reviewed Apr 26, 2026

View reviewed changes

avrabe merged commit f44b825 into main Apr 26, 2026
23 of 28 checks passed

avrabe deleted the fix/playwright-remaining-failures branch April 26, 2026 13:22

This was referenced Apr 26, 2026

test(playwright): flip description-mermaid pin to expect .svg-viewer wrap #220

Merged

feat(v0.5.0): README rewrite + rivet quickstart + CHANGELOG #227

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(playwright): close out remaining 8 dashboard test failures#217

fix(playwright): close out remaining 8 dashboard test failures#217
avrabe merged 3 commits intomainfrom
fix/playwright-remaining-failures

avrabe commented Apr 26, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 26, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment •

edited

Loading

Uh oh!

codecov Bot commented Apr 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

avrabe commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why these are the right fixes

artifacts.spec.ts:70 — render fix

serve middleware — /search bypass

serve routing — /embed/* via Router::nest

Variant onchange — inline → addEventListener

Test-code fixes

Test plan

Uh oh!

github-actions Bot commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📐 Rivet artifact delta

Uh oh!

github-actions Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

codecov Bot commented Apr 26, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

avrabe commented Apr 26, 2026 •

edited

Loading

serve middleware — `/search` bypass

serve routing — `/embed/*` via Router::nest

github-actions Bot commented Apr 26, 2026 •

edited

Loading

github-actions Bot left a comment •

edited

Loading