feat(serve,export): render the test-result trace as a fold/expand tree (REQ-274) - #843
Merged
Merged
Conversation
Customer-reported low contrast in the dashboard and the static compliance
export, which share rivet-cli/src/render/styles.rs.
Oracle first: six unit tests compute WCAG 2.1 relative luminance over the
palette and assert 4.5:1 for body text, 3:1 for large text and UI. They parse
hex values OUT OF the live CSS constant instead of copying them, so a
stylesheet edit cannot drift away from its own audit. The suite was red on
three pairs before any colour changed -- accent on --bg 3.20:1, accent on
--surface 3.48:1, white-on-accent 3.48:1 -- matching the values measured when
the requirement was triaged.
Fix: accent #3a86ff -> #2059b8, accent-hover #2568d6 -> #18458d, hue preserved
and chosen for headroom rather than the bare minimum (worst pair 5.07:1, not
4.60:1). 27 translucent rgba washes reshaded to the new rgb so tints match the
colour they tint. .stat-orange #e67e22 -> #c66c1d.
Three findings the requirement's own audit did not have:
1. The reported pair does not occur. No element paints white text on --bg.
What a reader actually saw is the white label on the primary button
(3.48:1) and every link (3.20:1). Both real, both fixed.
2. Accent text sits on rgba(accent,.08/.12) washes in id chips, inline mono
tags and source-line highlights. Composited, three of those measured
4.08-4.42:1 and failed. A naive fg/bg audit cannot see them because the
background is translucent rather than a palette variable, so the test
composites alpha.
3. .stat-orange measured 2.85:1 on --surface, failing even the 3:1
large-text bar.
Removed the drift class behind (2) and (3) rather than fixing instances: five
hand-copied hex literals of the accent lived OUTSIDE the CSS -- in stats.rs,
source.rs, doc_linkage.rs and serve/layout.rs -- still painting the old blue
after the palette moved. styles::ACCENT_HEX is now the source of truth, one
test asserts the CSS agrees with it, and a static scan fails if the retired
value reappears anywhere.
The scan's first version listed only render/ modules and missed
serve/layout.rs, which was the one file still painting the retired tint under
new-accent text. Widened, and the scan is negative-controlled: injecting
#3a86ff into stats.rs turns it red and names the file; removing it turns it
green.
Verified in rendered output rather than in source alone: `rivet export --format
html` and `rivet serve` both emit --accent #2059b8 with zero retired
references. (One match remains in artifacts/REQ-276.html, where the
requirement's own prose quotes the old hex.)
Confirmed with cargo fmt --check, clippy --all-targets -D warnings on 1.97.0,
cargo test --workspace (exit 0, 65 ok), rivet validate and rivet docs check --
all exit 0.
Implements: REQ-276
Verifies: REQ-276
…e (REQ-274) Customer-reported: deep chains (the ASPICE sw-req <- sw-detail-design <- unit-verification shape) rendered as a flat table. REQ-001 produces 40 hops of undifferentiated rows. The data was already a tree and nobody was using it. trace_test_results walks breadth-first with a `seen` set, so every reached artifact has exactly one via_target -- it returns that tree flattened. rivet_core::result_trace::as_tree restores the shape; rendering goes through the existing collapsible_tree component, so this is a rendering change over an existing model plus an existing component. Native <details> means no JavaScript, so the same markup serves `rivet serve` AND the static export. Verified in both: REQ-001 renders 3 foldable branches plus 37 leaf lines, identically, with Expand/Collapse All as progressive enhancement. The load-bearing invariant is that a folded view must contain every hop -- a branch that silently stops rendering is indistinguishable from one that is merely collapsed. tree_preserves_every_node pins it including the orphan case (a node whose parent was never reached is re-homed to top level, not dropped), and it is negative-controlled: disabling the re-homing turns it red with "tree dropped nodes: 2 in, 1 out". Two presentation corrections after looking at real output rather than a synthetic case. Leaves render as plain lines instead of <details>, because a disclosure triangle over "no further hops" is noise and the first version was no more readable than the table it replaced. And a large trace now opens nothing by default -- 33 expanded siblings is the same wall -- while a small one still opens its first level, because folding three hops helps nobody. The existing serve_integration test asserted the old Hops/Via table and legitimately broke. Rewritten to assert the tree. Note that my first rewrite compared the rendered hop count against /api/v1/trace-results, guarded on a non-empty response -- that route does not exist, so it 404'd, the guard swallowed it, and the assertion never ran while reporting green. Removed; exact parity is asserted in a unit test against a synthetic trace where the expected count is known. Confirmed with cargo fmt --check, clippy --all-targets -D warnings on 1.97.0, cargo test --workspace (exit 0, 65 ok), rivet validate, rivet docs check -- all exit 0. Implements: REQ-274 Verifies: REQ-274
📐 Rivet artifact delta
Graphgraph LR
REQ_274["REQ-274"]:::modified
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
Modified
Posted by |
Codecov Report❌ Patch coverage is
📢 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: 4f6d189 | Previous: 6ab55ac | Ratio |
|---|---|---|---|
store_insert/100 |
84757 ns/iter (± 798) |
67629 ns/iter (± 2912) |
1.25 |
store_insert/1000 |
910221 ns/iter (± 5062) |
738715 ns/iter (± 2984) |
1.23 |
store_insert/10000 |
17322224 ns/iter (± 1049564) |
13024659 ns/iter (± 1380431) |
1.33 |
store_lookup/100 |
1928 ns/iter (± 12) |
1473 ns/iter (± 65) |
1.31 |
store_lookup/1000 |
24301 ns/iter (± 45) |
17970 ns/iter (± 661) |
1.35 |
store_lookup/10000 |
362680 ns/iter (± 2077) |
251833 ns/iter (± 12449) |
1.44 |
store_by_type/100 |
96 ns/iter (± 0) |
74 ns/iter (± 0) |
1.30 |
store_by_type/1000 |
96 ns/iter (± 0) |
74 ns/iter (± 0) |
1.30 |
store_by_type/10000 |
96 ns/iter (± 0) |
74 ns/iter (± 0) |
1.30 |
schema_load_and_merge |
1512034 ns/iter (± 16320) |
1167816 ns/iter (± 43304) |
1.29 |
link_graph_build/100 |
167020 ns/iter (± 5801) |
126478 ns/iter (± 885) |
1.32 |
link_graph_build/1000 |
2007772 ns/iter (± 10033) |
1486732 ns/iter (± 29512) |
1.35 |
link_graph_build/10000 |
38408486 ns/iter (± 1090795) |
30560062 ns/iter (± 1587461) |
1.26 |
validate/100 |
475624 ns/iter (± 2017) |
356229 ns/iter (± 29454) |
1.34 |
validate/1000 |
15950912 ns/iter (± 165206) |
11595257 ns/iter (± 127171) |
1.38 |
validate/10000 |
1157651527 ns/iter (± 15451059) |
828365146 ns/iter (± 8650155) |
1.40 |
traceability_matrix/100 |
4325 ns/iter (± 20) |
3283 ns/iter (± 44) |
1.32 |
traceability_matrix/1000 |
46784 ns/iter (± 168) |
35250 ns/iter (± 188) |
1.33 |
traceability_matrix/10000 |
844872 ns/iter (± 6593) |
566762 ns/iter (± 9543) |
1.49 |
diff/100 |
63982 ns/iter (± 321) |
47909 ns/iter (± 126) |
1.34 |
diff/1000 |
727938 ns/iter (± 10941) |
529057 ns/iter (± 2786) |
1.38 |
diff/10000 |
9081784 ns/iter (± 497615) |
6255974 ns/iter (± 250553) |
1.45 |
query/100 |
1157 ns/iter (± 2) |
859 ns/iter (± 2) |
1.35 |
query/1000 |
15019 ns/iter (± 119) |
11226 ns/iter (± 24) |
1.34 |
query/10000 |
225336 ns/iter (± 1257) |
184013 ns/iter (± 892) |
1.22 |
document_parse/10 |
21443 ns/iter (± 146) |
16510 ns/iter (± 33) |
1.30 |
document_parse/100 |
146352 ns/iter (± 357) |
112486 ns/iter (± 954) |
1.30 |
document_parse/1000 |
1367787 ns/iter (± 20821) |
1042829 ns/iter (± 6111) |
1.31 |
This comment was automatically generated by workflow using github-action-benchmark.
…ping nodes The rivet-core mutation gate flagged 4 survivors in as_tree, and acting on them found a real defect rather than just a test gap. The `depth > 64` guard survived mutation to both `==` and `>=`, and `depth + 1` survived mutation to `*`, meaning nothing exercised the guard at all. Writing that test showed why it mattered: past depth 64 the guard returned an empty Vec, silently dropping the entire subtree. That is precisely the node loss tree_preserves_every_node exists to forbid — a rendered tree smaller than the trace it claims to show — and it was invisible because the only inputs anyone tested were shallow. Replaced the depth cap with a current-path visited set. Cycles still terminate (a genuine revisit is skipped) but a legitimately deep chain renders in full; a 200-deep chain now nests to 200 and keeps all 200 nodes. Also generalised the preservation sweep. It previously re-homed only nodes whose parent was unknown; a set of nodes cycling among themselves with no path from the root still vanished. It now sweeps for any input node absent from the tree, so "nothing is lost" holds by construction rather than by special case. Three tests added: a 200-deep chain keeps every node and nests to full depth, a cycle terminates and keeps both nodes, and is_empty is pinned so it cannot become `true` and make a populated subtree read as empty. Confirmed with cargo fmt --check, clippy --all-targets -D warnings on 1.97.0, cargo test --workspace (exit 0, 65 ok), rivet validate and rivet docs check -- all exit 0. result_trace unit tests 11/11. Verifies: REQ-274
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.
Customer-reported: deep chains — the ASPICE
sw-req ← sw-detail-design ← unit-verificationshape — rendered as a flat table. REQ-001 produces 40 hopsof undifferentiated rows.
The data was already a tree
trace_test_resultswalks breadth-first with aseenset, so every reachedartifact has exactly one
via_target— it returns that tree flattened.result_trace::as_treerestores the shape, and rendering goes through theexisting
collapsible_treecomponent. A rendering change over an existing modelplus an existing component; no new infrastructure.
Native
<details>means no JavaScript, so the same markup servesrivet serveand the static export. Verified in both — REQ-001 renders 3 foldablebranches + 37 leaf lines, identically, with Expand/Collapse All as progressive
enhancement.
The invariant that matters
A folded view must contain every hop. A branch that silently stops rendering
looks identical to one that is merely collapsed — you cannot tell by looking,
which is the whole problem.
tree_preserves_every_nodepins it, including the orphan case (a node whoseparent was never reached is re-homed to top level, not dropped). Negative-
controlled:
Two corrections after looking at real output
My first version was not actually more readable than the table it replaced:
<details>with "no further hops" inside — a disclosuretriangle over nothing. Now plain lines.
chrome. Now a large trace opens nothing and a small one still opens its first
level, because folding three hops helps nobody.
Result for REQ-001: 40 rows → 37 compact lines + 3 collapsed branches with
"N more" counts.
A vacuous assertion I wrote and caught
The existing
serve_integrationtest asserted the old Hops/Via table andlegitimately broke. My first rewrite compared rendered hop count against
/api/v1/trace-results/REQ-001, guarded on a non-empty response.That route does not exist. It 404'd,
reported == 0, the guard swallowedit, and the parity assertion never ran — while the test reported green. Exactly
the defect class this session has been chasing, written by me into the test for
the feature about not hiding things.
Removed. Exact parity is asserted in a unit test against a synthetic trace where
the expected count is known; the integration test asserts the tree renders with
hops and that the flat table is gone.
Gates
cargo fmt --check·clippy --all-targets -D warningson 1.97.0 ·cargo test --workspace(exit 0, 65 ok, 0 failed) ·rivet validate·rivet docs check— all exit 0.REQ-274 →
verified, 8 test markers.