fix(playwright): bound graph E2E node count so dataset growth doesn't trip render budget (REQ-171)#435
Merged
Merged
Conversation
… trip render budget (REQ-171) Self-found triaging why Playwright E2E was red across multiple merges (masked: it's a non-required check and main runs are concurrency-cancelled, so it never reports a conclusive green). Three graph/diagram tests failed reproducibly (through CI retry) — root cause is dogfood-dataset growth, not a serve bug: - graph.spec.ts "custom polygon shapes" hit /graph?types=requirement, design-decision&depth=2 with no ?limit=. The dataset grew past the default 200-node render budget for that filter, so the view returns the "above node budget" placeholder (no SVG) and the polygon count is 0. Add &limit=2000. - diagram-viewer.spec.ts graph page used /graph?limit=2000 (full ~871-node graph), now heavy enough that goto+settle exceeded the timeout, failing the .svg-viewer toolbar/fullscreen checks. Point it at a focused subgraph (/graph?focus=REQ-001&depth=1&limit=2000) — same wrapper invariant, fast. Verified green in a real browser locally: 7/7 in the affected specs (was 3 failing). `rivet validate` still PASS. Verifies: REQ-171 Refs: FEAT-001 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📐 Rivet artifact delta
Graphgraph LR
REQ_171["REQ-171"]:::added
classDef added fill:#d4edda,stroke:#28a745,color:#155724
classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Added
Posted by |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ 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: 0835015 | Previous: 897de61 | Ratio |
|---|---|---|---|
store_insert/10000 |
17074997 ns/iter (± 1290257) |
13207612 ns/iter (± 898633) |
1.29 |
This comment was automatically generated by workflow using github-action-benchmark.
avrabe
added a commit
that referenced
this pull request
Jun 5, 2026
…436) (#481) Issue #436: Playwright E2E (and Criterion benchmarks) were red/cancelled across consecutive main merges and nobody noticed, because main never reached a conclusive result. Root cause (verified): the concurrency group fell back to `github.ref` on push, so every main commit shared one group (`<workflow>-refs/heads/main`). With `cancel-in-progress` false the *running* job completes, but GitHub still supersedes *queued* runs in the group — so in a PR-merge train each new main push cancelled the previous queued run, and main rarely produced a conclusive Playwright/benchmark result. A real regression in any covered view would have looked identical to the benign dataset-growth failures (#435). Fix: fall back to `github.sha` (not `github.ref`) on push, so every main commit gets its own concurrency group and is never superseded. PRs are unchanged — `head_ref` still groups per source branch, so cancel-in-progress keeps superseding stale PR runs. Applied to both workflows that shared the pattern (ci.yml, benchmarks.yml). This is the pure-hygiene half of #436; whether to promote Playwright to a required status check remains a separate maintainer decision.
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.
Finding (self-found triaging red Playwright E2E)
Three graph/diagram E2E tests have been failing reproducibly (through the
CI retry) across multiple merges. It went unnoticed because Playwright E2E
is a non-required check and
mainruns are concurrency-cancelled, so itnever reports a conclusive green — see companion issue.
Root cause is dogfood-dataset growth, not a serve regression (verified in a
real browser locally):
graph.spec.tscustom polygon shapes/graph?types=requirement,design-decision&depth=2(no?limit=)svg polygoncount = 0diagram-viewer.spec.tsgraph: viewer toolbar present/graph?limit=2000(full ~871-node graph)goto+settle exceeds the timeout →.svg-viewerchecks faildiagram-viewer.spec.tsgraph: fullscreen toggles classFix (test-only)
&limit=2000(the filtered subset still renders fast)./graph?focus=REQ-001&depth=1&limit=2000— this test pins the.svg-viewertoolbar invariant, not graph size, so a bounded graph exercises the same
wrapper deterministically.
Verification
Ran the affected specs in a real Chromium locally: 7/7 passed (was 3
failing).
rivet validatestill PASS.Verifies: REQ-171 · Refs: FEAT-001
🤖 Generated with Claude Code