-
Notifications
You must be signed in to change notification settings - Fork 1
Renamed merc_preorder to merc_refinement and extended the implementation with counter examples #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… FIxed the strategy
…empty element to do nothing
…but not yet usable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR renames the merc_preorder crate to merc_refinement and extends the implementation with counter example generation for failed refinement checks. The renaming better reflects the crate's purpose of checking refinement relations between labelled transition systems.
Changes:
- Renamed
merc_preordercrate tomerc_refinementwith updated dependencies across the codebase - Added counter example generation infrastructure including a
CounterExampleConstructorandCounterExampleTreetrait - Enhanced refinement checking with new parameters for counter examples and preprocessing control
- Integrated timing measurements for rewrite operations and refinement checks
- Added JSON key-value formatting for structured logging in the vpg tool
- Fixed dependabot.yml configuration to enable Cargo ecosystem monitoring
- Minor documentation improvements and spelling corrections
Reviewed changes
Copilot reviewed 19 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Updated workspace member from crates/preorder to crates/refinement and enabled kv features for logging crates |
| Cargo.lock | Updated dependency references from merc_preorder to merc_refinement |
| tools/lts/Cargo.toml | Updated dependency from merc_preorder to merc_refinement |
| tools/lts/src/main.rs | Added CLI arguments for counter examples and preprocessing control, implemented counter example display |
| crates/refinement/Cargo.toml | Renamed package from merc_preorder to merc_refinement |
| crates/refinement/README.md | Updated documentation to reflect package rename and improved formatting |
| crates/refinement/src/lib.rs | Added exports for new counter example constructor module |
| crates/refinement/src/preorder.rs | Moved and updated from old location with new API for counter examples and preprocessing |
| crates/refinement/src/failures_refinement.rs | Extended refinement algorithm to support counter example generation with exploration strategy support |
| crates/refinement/src/counterexample_constructor.rs | New file implementing counter example tree construction and trace reconstruction |
| crates/refinement/src/antichain.rs | New file implementing antichain data structure for efficient state space exploration |
| tools/vpg/src/main.rs | Added JSON formatting for structured logging of key-value pairs |
| tools/rewrite/src/main.rs | Added timing infrastructure initialization and printing |
| tools/rewrite/src/lib.rs | Integrated timing measurements for rewrite operations |
| crates/vpg/src/variability_zielonka.rs | Enhanced debug logging with structured key-value pairs |
| crates/ldd/README.md | Reformatted citation for consistency |
| crates/aterm/README.md | Reformatted citations for consistency |
| SECURITY.md | Fixed grammatical errors ("featured" -> "feature", "send" -> "sent") |
| README.md | Fixed spelling ("MERC" -> "merc", "discussion" -> "discussions") |
| .github/dependabot.yml | Fixed configuration by setting package-ecosystem to "cargo" and removing comments |
Comments suppressed due to low confidence (5)
crates/refinement/src/failures_refinement.rs:222
- The counter example functionality introduced in this PR lacks test coverage. The test only checks the case where counter_example is false. Consider adding tests that verify counter example generation works correctly, including both successful refinement (where no counter example is generated) and failed refinement (where a counter example should be returned).
crates/refinement/src/failures_refinement.rs:45 - There is a spelling error in the documentation. "Brreadth-first" should be "Breadth-first".
crates/refinement/src/failures_refinement.rs:171 - The doc comment for the LtsCache struct is empty (just "///"). This should either have actual documentation or be removed.
crates/refinement/README.md:6 - There is a grammatical error. The sentence reads "which the preorder relation" but should be "which is the preorder relation" (missing "is").
crates/refinement/src/failures_refinement.rs:178 - The LtsCache struct and its implementation appear to be unused code that was likely left in accidentally. The struct is empty, and the method returns an empty iterator. This dead code should be removed.
| /// Returns the index of the root of the counterexample tree. | ||
| fn root_index(&self) -> Self::Index; | ||
|
|
||
| /// Adds a edge to the counterexample tree. |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The article should be "an" before "edge" (which starts with a vowel sound), not "a".
| /// Adds a edge to the counterexample tree. | |
| /// Adds an edge to the counterexample tree. |
| // If there are key-values, format them as a JSON object. | ||
| formatter.write("\n{ ".as_bytes())?; | ||
| let mut json_printer = JsonPrinter { formatter }; | ||
| source.visit(&mut json_printer).expect("Visiting log key-values failed"); |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message "Visiting log key-values failed" should be used in a proper error context. Using expect() with a panic message suggests this might be an unrecoverable error, but it's unclear if this is the intended behavior. Consider using a more graceful error handling approach or clarifying the failure scenario in the message.
| source.visit(&mut json_printer).expect("Visiting log key-values failed"); | |
| if let Err(e) = source.visit(&mut json_printer) { | |
| return Err(std::io::Error::new( | |
| std::io::ErrorKind::Other, | |
| format!("visiting log key-values failed: {e}"), | |
| )); | |
| } |
| } else { | ||
| if let Some(trace) = counter_example { | ||
| println!("Counter-example trace:"); | ||
| for label in trace { | ||
| println!(" {}", label); | ||
| } | ||
| } | ||
|
|
||
| println!("false"); |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The if-let block should be combined with the else clause in a more idiomatic way. Consider printing the counter-example trace and then "false" together without the nested structure, or use a single println! for clarity.
| } else { | |
| if let Some(trace) = counter_example { | |
| println!("Counter-example trace:"); | |
| for label in trace { | |
| println!(" {}", label); | |
| } | |
| } | |
| println!("false"); | |
| } else if let Some(trace) = counter_example { | |
| println!("Counter-example trace:"); | |
| for label in trace { | |
| println!(" {}", label); | |
| } | |
| println!("false"); | |
| } else { | |
| println!("false"); |
| /// A class that can be used to store a counter example tree from which a | ||
| /// counter example trace can be extracted. | ||
| pub struct CounterExampleConstructor { | ||
| /// The backward three is stored in a deque. |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a spelling error in the comment. "three" should be "tree".
| /// The backward three is stored in a deque. | |
| /// The backward tree is stored in a deque. |
| use merc_utilities::Timing; | ||
|
|
||
| use crate::is_failures_refinement; | ||
| use crate::CounterExampleConstructor; |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unused import CounterExampleConstructor should be removed. It's imported but never used in this file.
| use crate::CounterExampleConstructor; |
See the title, this is mostly to run the tests.