test(serve): stabilize graph_type_filter flake with a 15s budget (#389)#390
Merged
Conversation
…ke (#389) `graph_type_filter_renders_when_under_budget` fetched `/graph?types=requirement` with the default 5s read timeout. That endpoint does a BFS + layout over the dogfood corpus (~742 nodes / 1477 edges), which on a loaded CI runner can brush past 5s; `fetch_with_timeout` parses a timed-out/empty response's status as 0, so the timeout surfaced as `status == 0` and failed the `== 200` assertion. Same chronic flake the focus-graph tests already fixed with a 15s budget — this one test was missed. It falsely failed CI on #380, #387, and #388. Switch it to `fetch_with_timeout(..., 15s)`, matching the other graph endpoints. The test asserts on the response shape (SVG or budget message), not a hard latency bound, so the wider timeout is safe. Verified locally: passes 3/3 runs. Trace: skip Closes: #389 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📐 Rivet artifact deltaNo artifact changes in this PR. Code-only changes (renderer, CLI wiring, tests) don't touch the artifact graph. |
![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: c470403 | Previous: 6ccc2f3 | Ratio |
|---|---|---|---|
store_insert/10000 |
16062143 ns/iter (± 897846) |
12470120 ns/iter (± 253802) |
1.29 |
link_graph_build/10000 |
42706749 ns/iter (± 2459613) |
25193457 ns/iter (± 926623) |
1.70 |
validate/10000 |
25213404 ns/iter (± 1799316) |
12522737 ns/iter (± 423960) |
2.01 |
diff/10000 |
11317023 ns/iter (± 869925) |
7633418 ns/iter (± 60183) |
1.48 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Closes #389.
graph_type_filter_renders_when_under_budgetfetched/graph?types=requirementwith the default 5s read timeout. That endpoint runs a BFS + layout over the dogfood corpus (~742 nodes / 1477 edges); on a loaded CI runner it can brush past 5s.fetch_with_timeoutparses a timed-out/empty response's status as0, so the timeout surfaced asstatus == 0and failedassert_eq!(status, 200).This is the same chronic flake the focus-graph tests already fixed by moving to a 15s budget (per the comment block above
fetch_with_timeout) — this one test was missed. It produced false CI failures (Test / Proptest-extended jobs) on #380, #387, and #388, each verified to pass locally.Fix: use
fetch_with_timeout(..., Duration::from_secs(15)), matching the sibling graph endpoints. The test asserts on the response shape (SVG or budget message), not a hard latency bound, so the wider timeout is safe.Verification: passes 3/3 local runs; clippy
--all-targets+ fmt clean.🤖 Generated with Claude Code