Conversation
![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: d538157 | Previous: 2fafe1a | Ratio |
|---|---|---|---|
store_insert/1000 |
884773 ns/iter (± 4180) |
678487 ns/iter (± 6153) |
1.30 |
store_insert/10000 |
13502184 ns/iter (± 558793) |
10919758 ns/iter (± 667589) |
1.24 |
store_lookup/10000 |
369571 ns/iter (± 1899) |
269456 ns/iter (± 1380) |
1.37 |
link_graph_build/100 |
159041 ns/iter (± 396) |
127731 ns/iter (± 591) |
1.25 |
link_graph_build/1000 |
1876416 ns/iter (± 63595) |
1487602 ns/iter (± 11779) |
1.26 |
link_graph_build/10000 |
38447212 ns/iter (± 3273933) |
23876175 ns/iter (± 1828586) |
1.61 |
validate/100 |
107471 ns/iter (± 978) |
83361 ns/iter (± 401) |
1.29 |
validate/1000 |
963851 ns/iter (± 6929) |
736705 ns/iter (± 4109) |
1.31 |
validate/10000 |
16632445 ns/iter (± 1185784) |
9182847 ns/iter (± 548001) |
1.81 |
traceability_matrix/10000 |
772106 ns/iter (± 4509) |
564717 ns/iter (± 4408) |
1.37 |
diff/10000 |
7672581 ns/iter (± 447435) |
6107115 ns/iter (± 365890) |
1.26 |
query/10000 |
155171 ns/iter (± 526) |
69446 ns/iter (± 4127) |
2.23 |
document_parse/10 |
21282 ns/iter (± 82) |
16784 ns/iter (± 24) |
1.27 |
document_parse/100 |
151286 ns/iter (± 389) |
114429 ns/iter (± 307) |
1.32 |
document_parse/1000 |
1404910 ns/iter (± 27334) |
1056989 ns/iter (± 13193) |
1.33 |
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! |
avrabe
force-pushed
the
fix/search-url-persistence-and-embed-discovery
branch
3 times, most recently
from
baa3924 to
d09ff5f
Compare
The Cmd+K overlay used vanilla fetch() with no history write, so a page reload (Cmd+R) silently dropped any in-flight search. Now we sync the current query into the URL via history.replaceState as ?cmdk=...; on load, if ?cmdk= is present we re-open the overlay pre-filled with the saved term and re-run the search. Using cmdk (not q) avoids colliding with the existing /artifacts?q=... filter-bar state. The /artifacts filter-bar search input was already wired with hx-push-url="true" via components::search_input — this change adds a regression test asserting that typing into it updates the URL and the value survives a reload. Fixes: REQ-007
…t query
Design note only — no code changes. Covers four UX gaps reported from
dogfooding in a single note so the trade-offs are visible side-by-side:
1. Mermaid in artifact descriptions (one-function fix in markdown.rs).
2. {{query:<sexpr>}} embed — reuses existing sexpr_eval; read-only MVP.
3. rivet docs embeds + table-driven registry for resolve_embed.
4. rivet query CLI — thin mirror of MCP's rivet_query.
Each section cites exact file paths and line numbers where changes
would land. Ends with a priority-ordered recommendation table so the
implementation sequence is unambiguous.
Exempt type: docs (no trailer required).
avrabe
force-pushed
the
fix/search-url-persistence-and-embed-discovery
branch
from
d09ff5f to
d538157
Compare
avrabe
added a commit
that referenced
this pull request
Apr 22, 2026
The user report listed {{group:...}} as a missing embed with no stated
semantics. Neither the PR #159 design doc nor the codebase pinned down a
definition, so this commit picks the most useful reading:
{{group:FIELD}} renders a count-by-value table of the named artifact
field. Example outputs:
{{group:status}} → draft / approved / shipped counts
{{group:type}} → per-type counts (complement to {{stats:types}})
{{group:asil}} → per-ASIL counts from a custom YAML field
Missing / empty values bucket into "unset" so the totals equal the
project artifact count. List-valued fields (e.g. tags) render as
comma-joined keys — per-tag grouping is a future enhancement.
The renderer reuses the same html_escape / embed-table class set as the
rest of the embeds, and returns an EmbedError for an empty FIELD.
Five unit tests: status grouping with an "unset" row, type grouping,
custom YAML field (asil), empty-field rejection, and empty-store
no-data path.
Implements: REQ-007
Refs: FEAT-032
avrabe
added a commit
that referenced
this pull request
Apr 22, 2026
The user report listed {{group:...}} as a missing embed with no stated
semantics. Neither the PR #159 design doc nor the codebase pinned down a
definition, so this commit picks the most useful reading:
{{group:FIELD}} renders a count-by-value table of the named artifact
field. Example outputs:
{{group:status}} → draft / approved / shipped counts
{{group:type}} → per-type counts (complement to {{stats:types}})
{{group:asil}} → per-ASIL counts from a custom YAML field
Missing / empty values bucket into "unset" so the totals equal the
project artifact count. List-valued fields (e.g. tags) render as
comma-joined keys — per-tag grouping is a future enhancement.
The renderer reuses the same html_escape / embed-table class set as the
rest of the embeds, and returns an EmbedError for an empty FIELD.
Five unit tests: status grouping with an "unset" row, type grouping,
custom YAML field (asil), empty-field rejection, and empty-store
no-data path.
Implements: REQ-007
Refs: FEAT-032
avrabe
added a commit
that referenced
this pull request
Apr 22, 2026
…et docs embeds + rivet query (#180) * fix(markdown): render mermaid fences as <pre class="mermaid"> Artifact descriptions go through rivet-core's pulldown-cmark based render_markdown. That renderer was emitting pulldown's default <pre><code class="language-mermaid"> for fenced mermaid blocks, which the dashboard's mermaid.js (selector `.mermaid`) never matches — so diagrams in artifact descriptions rendered as literal source. Add a tiny event-mapping pass that replaces the Start/End code-block events for `mermaid` fences with synthetic HTML wrappers (NUL-byte sentinels that cannot appear in input, rewritten post-html-push). Other raw HTML events are still dropped for XSS defence, and the sanitize pass still runs. Non-mermaid fences keep their existing rendering. Covers the bug end-to-end: markdown unit tests verify the <pre class="mermaid"> shape and regression for rust fences; architecture.yaml ARCH-CORE-001 now carries a mermaid diagram as a live fixture, and a Playwright regression walks the artifact page and asserts mermaid.js actually produces an SVG. Fixes: REQ-032 Refs: FEAT-032 * feat(embed): {{query:(sexpr)}} renders live s-expression results Adds a read-only {{query:(...)}} embed backed by the existing sexpr_eval::parse_filter + matches_filter_with_store pair — the same path that powers `rivet list --filter` and MCP's `rivet_query`. The embed renders a compact `id | type | title | status` table, clamped to a default of 50 rows (hard max 500 via `limit=N`), with a visible "Showing N of M" footer on truncation so nothing disappears silently. Parser changes: - `EmbedRequest::parse` was previously splitting on `:` blindly, which corrupted any s-expression argument. For `name == "query"` it now expects a balanced-paren form `{{query:(...)}}` and captures the whole paren group as the single positional arg. Parens inside string literals are respected so `(= title "foo)bar")` parses correctly. - All other embed shapes (`stats`, `stats:types`, `table:T:F`, …) keep their existing colon-split behaviour — covered by regression tests. Tests: 12 new unit tests covering the paren scanner (simple, nested, string-literal, unbalanced), the `query` parse shape, an end-to-end `query_embed_matches_sexpr_filter` cross-check against the evaluator directly, truncation, `limit=` clamping, and error propagation from a malformed filter. Implements: REQ-007 Refs: FEAT-032 * feat(embed): {{stats:type:NAME}} for single-type counts {{stats:types}} already renders the full per-type count table; users asking for a single number — e.g. "how many requirements do we have?" — had to eyeball it out of the table. Add a granular form {{stats:type:requirement}} that renders a one-row table with just the count for the named type. Unknown types render count=0 rather than erroring, matching SC-EMBED-3: the rule is "visible output, never silent disappearance". An empty type name falls back to an `embed-error` span. Four unit tests: counts correctly, unknown type → zero, empty name → visible error, and a regression check that the existing {{stats:types}} form still produces the full multi-row table. Implements: REQ-007 Refs: FEAT-032 * feat(embed): {{group:FIELD}} count-by-value grouping The user report listed {{group:...}} as a missing embed with no stated semantics. Neither the PR #159 design doc nor the codebase pinned down a definition, so this commit picks the most useful reading: {{group:FIELD}} renders a count-by-value table of the named artifact field. Example outputs: {{group:status}} → draft / approved / shipped counts {{group:type}} → per-type counts (complement to {{stats:types}}) {{group:asil}} → per-ASIL counts from a custom YAML field Missing / empty values bucket into "unset" so the totals equal the project artifact count. List-valued fields (e.g. tags) render as comma-joined keys — per-tag grouping is a future enhancement. The renderer reuses the same html_escape / embed-table class set as the rest of the embeds, and returns an EmbedError for an empty FIELD. Five unit tests: status grouping with an "unset" row, type grouping, custom YAML field (asil), empty-field rejection, and empty-store no-data path. Implements: REQ-007 Refs: FEAT-032 * feat(cli): `rivet docs embeds` + embed registry Adds `rivet_core::embed::EMBED_REGISTRY` — a `&[EmbedSpec]` slice with one entry per known embed (`stats`, `coverage`, `diagnostics`, `matrix`, `query`, `group`, plus the legacy `artifact` / `links` / `table` inline embeds). Each spec carries the `name`, a compact `args` signature, a one-line `summary`, a runnable `example`, and a `legacy` flag so the inline embeds are still listed even though they dispatch from `document.rs` rather than `resolve_embed`. Surfaces the registry in three places so discoverability stays in sync: - `rivet docs embeds` (and `--format json`) — prints an aligned table or emits a machine-readable list; usage footer points at `rivet embed` and `rivet docs embed-syntax`. - Dashboard Help view — new "Document Embeds" card built from the same registry so users browsing at /help see the exact same set. - Unit tests assert that every name dispatched in `resolve_embed` also appears in `EMBED_REGISTRY`, and that every registry example parses via `EmbedRequest::parse` (catches copy-paste rot). Integration tests in `rivet-cli/tests/embeds_help.rs` walk the built binary end-to-end for both text and JSON output. While here, fix a latent order-dependent assertion in the earlier `query_embed_matches_sexpr_filter` test — store iteration is not stable, so compare as a sorted set instead. Implements: REQ-007 Refs: FEAT-032, FEAT-001 * feat(cli): `rivet query --sexpr ...` mirrors MCP rivet_query Lifts the shared s-expression evaluation path into `rivet_core::query::execute_sexpr` so MCP's `rivet_query` tool, the new `rivet query` CLI, and the `{{query:(...)}}` document embed all converge on one function. The result struct carries `matches`, `total`, and a `truncated` flag so callers can render the same "Showing N of M" footer without re-running the filter. CLI surface: rivet query --sexpr '(and (= type "requirement") (has-tag "stpa"))' rivet query --sexpr '...' --format json # MCP shape envelope rivet query --sexpr '...' --format ids # newline-separated IDs rivet query --sexpr '...' --limit 25 Three output formats — text (aligned columns), json (the MCP envelope `{filter, count, total, truncated, artifacts[]}`), and ids (for shell pipelines: `rivet query --format ids | xargs -n1 rivet show`). MCP's `tool_query` is refactored to use `execute_sexpr` directly and now returns the same envelope shape (adding `total` + `truncated` fields alongside the existing `filter`, `count`, `artifacts`), so scripts working against MCP and CLI read identical JSON. Five unit tests in `rivet_core::query::tests` (type filter, limit + truncation, empty-filter-matches-all, parse-error propagation, tag filter agreement with `rivet list --filter`). Three integration tests in `rivet-cli/tests/cli_commands.rs` exercise the binary end-to-end for `--format ids` vs. `rivet list --type`, the MCP-shape JSON envelope, and the error path for an unbalanced s-expression. Implements: REQ-007 Refs: FEAT-010, FEAT-032
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.
Two commits
A. Code fix — Cmd+K search URL persistence
`rivet-cli/src/serve/js.rs::SEARCH_JS` now calls `history.replaceState` with `?cmdk=
Note: `/artifacts` search input already had `hx-push-url="true"` in `components.rs:167` — only Cmd+K was broken.
Playwright regression tests added for both paths.
B. Design doc — `docs/design/embed-discoverability.md`
No code. Covers 4 UX gaps in priority order:
Sequencing: do #1 first (pure bug), then #3 before #2 so `{{query:...}}` registers itself in the registry.
Test plan
🤖 Generated with Claude Code