Allow arbitrary class data #23
Merged
Conversation
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
This PR extended the serialized spec to allow additional arbitrary string key value mappings on each class. This would be helpful for inlining some values into class level attributes (egraphs-good/egraph-visualizer#4) and also in egglog to serialize let bindings on the class as metadata (egraphs-good/egglog#376) TODO before merging: - [ ] Add support in graphviz visualizer and test it locally - [ ] Add support in egglog for emitting let bindings here - [ ] Add support in the javascript visualizer to display these TODO after merging: - [ ] Add support for turning some values automatically into extra class properties, in line with egraphs-good/egraph-visualizer#4
saulshanabrook
added a commit
to egraphs-good/egraph-visualizer
that referenced
this pull request
Oct 9, 2025
saulshanabrook
added a commit
to egraphs-good/egraph-visualizer
that referenced
this pull request
Oct 9, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR extends the serialized specification to allow additional arbitrary string key-value mappings on each class, enabling inlining of values into class-level attributes and supporting metadata serialization in egglog.
- Extended
ClassDatastruct to include a flattenedextraHashMap for arbitrary string key-value pairs - Updated graphviz visualizer to display extra class data as HTML table labels in subgraphs
- Added test data demonstrating the new arbitrary class data functionality
Reviewed Changes
Copilot reviewed 4 out of 7 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/lib.rs | Added HashMap import and extra field to ClassData struct with serde flattening |
| src/graphviz.rs | Updated visualization logic to extract and display extra class data as HTML labels |
| tests/tiny.json | Added test data with example arbitrary class data mappings |
| tests/round_trip.rs | Changed debug output from file path to dot format content |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
yihozhang
approved these changes
Oct 15, 2025
Contributor
|
I'm merging this PR since I'm cutting a release for egglog and it seems this one is ready to go. |
saulshanabrook
added a commit
to egraphs-good/egraph-visualizer
that referenced
this pull request
Oct 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR extended the serialized spec to allow additional arbitrary string key value mappings on each class.
This would be helpful for inlining some values into class level attributes (egraphs-good/egraph-visualizer#4) and also in egglog to serialize let bindings on the class as metadata (egraphs-good/egglog#376)
TODO before merging:
TODO after merging: