feat: Add configuration settings to mermaid render - #3207
nicolaassolini-qntm wants to merge 15 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3207 +/- ##
==========================================
+ Coverage 81.87% 81.97% +0.09%
==========================================
Files 242 242
Lines 46824 47335 +511
Branches 40417 40928 +511
==========================================
+ Hits 38336 38801 +465
- Misses 6594 6638 +44
- Partials 1894 1896 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This PR contains breaking changes to the public Rust API. cargo-semver-checks summary |
There was a problem hiding this comment.
Pull request overview
This PR introduces a configurable string-rendering pathway for HUGR operations and types, and wires that configuration into Mermaid (and partially into dot) graph rendering so callers can control whether node/edge labels include qualified names, extension versions, and type arguments.
Changes:
- Add
RenderStringConfigand newrender_str(...)APIs across ops/types to support configurable string rendering. - Update Mermaid rendering to use
render_str(...)for node and edge labels, and exposemermaid_string_with_config(...)for callers. - Update/add tests and snapshots to reflect the new rendering behavior.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| hugr-persistent/src/trait_impls.rs | Propagates Mermaid render-string configuration into persistent HUGR view config. |
| hugr-core/src/types/type_row.rs | Extends TypeRowLike with config-driven render_str(...) and implements it for rows. |
| hugr-core/src/types/type_param.rs | Adds config-driven Term::render_str(...) and tests for nested/composite term rendering. |
| hugr-core/src/types/poly_func.rs | Adds config-driven PolyFuncTypeBase::render_str(...) and a propagation test. |
| hugr-core/src/types/custom.rs | Changes CustomType Display formatting for type arguments. |
| hugr-core/src/types.rs | Adds Type::render_str(...) convenience wrapper. |
| hugr-core/src/ops/sum.rs | Adds render_str(...) for Tag op under the new rendering trait. |
| hugr-core/src/ops/module.rs | Adds render_str(...) implementations for module-related ops (Module, FuncDefn, etc.). |
| hugr-core/src/ops/dataflow.rs | Adds render_str(...) to DataflowOpTrait and forwards through OpTrait. |
| hugr-core/src/ops/custom.rs | Implements config-driven rendering for extension and opaque ops, including version/type-arg handling and tests. |
| hugr-core/src/ops/controlflow.rs | Adds render_str(...) implementations for controlflow ops and blocks. |
| hugr-core/src/ops/constant.rs | Adds render_str(...) implementation for constants. |
| hugr-core/src/ops.rs | Introduces RenderStringConfig and adds render_str(...) requirement to OpTrait. |
| hugr-core/src/hugr/views/render.rs | Wires RenderStringConfig into Mermaid node/edge label rendering; adds formatter option + test. |
| hugr-core/src/hugr/views.rs | Adds mermaid_string_with_config(...) to expose configurable Mermaid rendering. |
| hugr-core/src/hugr/views/snapshots/hugr_core__hugr__views__tests__mmd_dfg.snap | Updates Mermaid snapshot output to match new default label rendering. |
| hugr-core/src/hugr/views/snapshots/hugr_core__hugr__views__tests__dot_dfg.snap | Updates dot snapshot output to match new default node label rendering. |
| hugr-core/src/hugr/views/root_checked/snapshots/hugr_core__hugr__views__root_checked__dfg__test__map_io_cycle_3qb.snap | Updates root-checked Mermaid snapshot output for new node labels. |
| hugr-core/src/hugr/views/root_checked/snapshots/hugr_core__hugr__views__root_checked__dfg__test__map_io_cx_gate.snap | Updates root-checked Mermaid snapshot output for new node labels. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Merging this PR will improve performance by 8.17%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | serialization/big_hugr/1048576/sexpr/without_extensions/decode |
28.6 ms | 26.2 ms | +9.26% |
| ⚡ | serialization/big_hugr/1048576/sexpr/with_extensions/decode |
28.7 ms | 26.3 ms | +9.22% |
| ⚡ | insert_view_forest |
86.8 µs | 81.9 µs | +6.06% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing na/3001-render-ext-ver-mermaid (e9a9ef7) with main (cf16d6f)
aborgna-q
left a comment
There was a problem hiding this comment.
Can you add some ```mermaid examples to the main comment?
| /// Returns a string representation of this term. | ||
| /// | ||
| /// Composite terms recursively render each of their nested terms. | ||
| pub fn render_str(&self, config: RenderStringConfig) -> String { |
There was a problem hiding this comment.
Some of this should be dispatched to the internal structs instead.
We have Display impls for SumType, CustomType, etc. that do almost-but-not-quite the same logic.
Those could be either merged, or at least put the impls beside each other.
There was a problem hiding this comment.
changes done:
- Custom type logic moved in
CustomType.render_strand unified with the display logic - Sum type logic moved in
SumType.render_strand unified with the display logic - FuncTypeBase I didnt move out the logic since it is simple (potentially we can)
| /// Render the polymorphic function type using the supplied configuration. | ||
| pub fn render_str(&self, config: RenderStringConfig) -> String { | ||
| let params = if self.params.is_empty() { | ||
| Cow::Borrowed("") |
There was a problem hiding this comment.
Why the cow? Seems like a &str would be enough here.
There was a problem hiding this comment.
display_params use Cow too, should I change?
| "Opaque operation" | ||
| } | ||
|
|
||
| fn render_str(&self, config: RenderStringConfig) -> String { |
There was a problem hiding this comment.
Before we prepended OpaqueOp: to the name.
OpaqueOps shouldn't appear on user hugrs, so it's nice to have the explicit marker to differentiate them from ExtensionOp.
There was a problem hiding this comment.
Is ok without space?
|
|
||
| /// Configuration for rendering an operation as a string. | ||
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | ||
| pub struct RenderStringConfig { |
There was a problem hiding this comment.
I moved the class here, also refactor a bit the definition, now the fields are private
closes #3001
by updating the mermaid rendering.
This PR allows you to decide when to print the qualified name, extension version and type arguments of nodes and edges during mermaid rendering.
This PR also deprecate
fn dot_stringinHugrView