fix: use directed edges instead of bidirectional in reduction graph#42
fix: use directed edges instead of bidirectional in reduction graph#42
Conversation
The JSON export used a single edge with `bidirectional: true` for reversible reductions, but the source/target direction depended on non-deterministic inventory iteration order. This broke the Typst paper's label resolution in CI (Deploy Documentation workflow). Replace with two separate directed edges per reversible reduction. This makes the JSON deterministic and the paper's link resolution trivial. Add reduction-rule entries for the 5 newly-explicit reverse directions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the reduction-graph JSON schema to represent reversible reductions as two explicit directed edges rather than a single edge with a bidirectional flag, eliminating nondeterminism caused by inventory iteration order.
Changes:
- Remove
bidirectionalfromEdgeJsonand emit separate directed edges for each reduction. - Update mdBook visualization logic to infer bidirectionality by detecting reverse edge pairs.
- Update tests and generated JSON artifacts (mdBook + paper) to match the new schema and include newly explicit reverse directions.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/unit_tests/rules/graph.rs | Updates unit tests to assert presence/absence of directed edge pairs and removes reliance on bidirectional. |
| src/rules/graph.rs | Removes bidirectional from EdgeJson and changes JSON export to emit/sort directed edges deterministically. |
| docs/src/reductions/reduction_graph.json | Updates committed mdBook graph JSON to new schema and adds explicit reverse edges. |
| docs/src/introduction.md | Adjusts visualization JS to mark edges bidirectional when reverse pairs exist (derived, not stored). |
| docs/paper/reductions.typ | Adds missing reverse-direction reduction-rule entries and updates link rendering/coverage checks for directed edges. |
| docs/paper/reduction_graph.json | Updates committed paper graph JSON to new schema and adds explicit reverse edges. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert!(json_string.contains("\"nodes\"")); | ||
| assert!(json_string.contains("\"edges\"")); | ||
| assert!(json_string.contains("MaximumIndependentSet")); | ||
| assert!(json_string.contains("\"category\"")); | ||
| assert!(json_string.contains("\"bidirectional\"")); | ||
| assert!(json_string.contains("\"overhead\"")); |
There was a problem hiding this comment.
test_to_json_string no longer asserts that the legacy "bidirectional" field is absent, so a regression that accidentally re-introduces it (e.g., via a renamed/flattened field) wouldn’t be caught. Consider adding an explicit negative assertion like checking the JSON string does not contain "bidirectional" (keeping the existing "overhead" check if desired).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #42 +/- ##
=======================================
Coverage 97.37% 97.37%
=======================================
Files 176 176
Lines 26217 26219 +2
=======================================
+ Hits 25530 25532 +2
Misses 687 687 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
[fix] Address the Copilot review comment: add negative assertion for bidirectional field in test_to_json_string. |
Address Copilot review comment: assert that the legacy "bidirectional" field is absent in test_to_json_string to catch accidental regressions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review feedback addressedCopilot comment: Added negative assertion in CI: All checks were already passing; the new assertion also passes (1551/1551 tests green, clippy clean). Commit: |
Full audit of all rule files against source code: - adding-models.md: Fix Problem trait template (was missing NAME, variant(), Size, num_flavors(), energy_mode(), solution_size(); had non-existent is_valid_solution(); wrong import path for ProblemSize). Add specialized category. - testing.md: Fix closed-loop test pattern to use actual API (ReduceTo::<T>::reduce_to, solution_size().is_valid). Remove stale cargo-tarpaulin reference (project uses cargo-llvm-cov). - documentation.md: Fix problem-def signature (name, body not name, title). Fix reduction-rule params (example is bool, no overhead param). Note JSON-based examples, not Rust code. - adding-reductions.md: Fix make export-graph -> cargo run. - CLAUDE.md: Fix problem-def signature, add directed reduction note. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace verbose pseudo-code templates with references to actual files (kcoloring.rs, VC↔IS reduction, etc.) so rules stay in sync with code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
bidirectionalflag fromEdgeJson— each reversible reduction now emits two separate directed edgesreduction-ruleentries in the paper for the newly-explicit reverse directions (IS↔VC, IS↔SetPacking, SAT↔kSAT, SG↔MaxCut, SG↔QUBO)bidirectionalfield)Test plan
make test— all 1551 tests passmake clippy— no warningstypst compile docs/paper/reductions.typ— compiles without errors🤖 Generated with Claude Code