Skip to content

Fix #292: [Model] IntegralFlowHomologousArcs#739

Merged
GiggleLiu merged 6 commits intomainfrom
issue-292
Mar 22, 2026
Merged

Fix #292: [Model] IntegralFlowHomologousArcs#739
GiggleLiu merged 6 commits intomainfrom
issue-292

Conversation

@GiggleLiu
Copy link
Contributor

Summary

Add the implementation plan for the new IntegralFlowHomologousArcs model and execute it in follow-up commits.

Fixes #292

@codecov
Copy link

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 96.30996% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.57%. Comparing base (638c7f7) to head (1bdede0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/models/graph/integral_flow_homologous_arcs.rs 92.64% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #739      +/-   ##
==========================================
- Coverage   97.58%   97.57%   -0.01%     
==========================================
  Files         421      423       +2     
  Lines       52075    52346     +271     
==========================================
+ Hits        50815    51077     +262     
- Misses       1260     1269       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@GiggleLiu
Copy link
Contributor Author

Implementation Summary

Changes

Deviations from Plan

  • The issue text used the unit-capacity brute-force bound 2^num_arcs. The implemented metadata uses the more general search-space bound (max_capacity + 1)^num_arcs, and the paper explicitly notes 2^m as the unit-capacity special case.

Open Questions

  • None.

@GiggleLiu
Copy link
Contributor Author

Agentic Review Report

Structural Check

Structural Review: model IntegralFlowHomologousArcs

Structural Completeness

# Check Status
1 Model file exists PASS — src/models/graph/integral_flow_homologous_arcs.rs
2 inventory::submit! present PASS — schema + size-field registrations are present in src/models/graph/integral_flow_homologous_arcs.rs
3 Serialize / Deserialize derive on struct PASS — #[derive(Debug, Clone, Serialize, Deserialize)] on the model struct
4 Problem trait impl PASS — impl Problem for IntegralFlowHomologousArcs present
5 SatisfactionProblem impl PASS — impl SatisfactionProblem for IntegralFlowHomologousArcs present
6 Linked unit-test module from model file PASS — #[path = "../../unit_tests/models/graph/integral_flow_homologous_arcs.rs"] present
7 Dedicated model test file exists PASS — src/unit_tests/models/graph/integral_flow_homologous_arcs.rs exists
8 Dedicated model test file has >= 3 tests PASS — 11 test functions in src/unit_tests/models/graph/integral_flow_homologous_arcs.rs
9 Registered in src/models/graph/mod.rs PASS — module, re-export, and canonical example aggregation are present
10 Re-exported in src/models/mod.rs PASS
11 declare_variants! entry exists PASS — default sat IntegralFlowHomologousArcs => "(max_capacity + 1)^num_arcs"
12 CLI name resolution path works PASS — canonical-name lookup is registry-backed; pred show IntegralFlowHomologousArcs succeeds without a hardcoded alias entry
13 CLI create support PASS — create/help/example wiring is present in problemreductions-cli/src/cli.rs and problemreductions-cli/src/commands/create.rs
14 Canonical model example registered PASS — canonical_model_example_specs() added in the model file and aggregated via src/models/graph/mod.rs
15 Paper display-name entry PASS — docs/paper/reductions.typ includes "IntegralFlowHomologousArcs": [Integral Flow with Homologous Arcs]
16 Paper problem-def block PASS — docs/paper/reductions.typ adds a full #problem-def("IntegralFlowHomologousArcs") block
17 Deterministic whitelist check FAIL — files outside the model-PR whitelist: problemreductions-cli/src/cli.rs, problemreductions-cli/src/commands/create.rs, problemreductions-cli/tests/cli_tests.rs, src/lib.rs, src/unit_tests/example_db.rs, src/unit_tests/graph_models.rs
18 Blacklisted autogenerated files committed PASS — none of the forbidden generated artifacts are present in the diff
19 Deterministic completeness packet PASS — no completeness items missing

Build Status

  • make test equivalent (cargo test --features "ilp-highs example-db" -- --include-ignored, rerun offline after crates.io index timeouts): PASS
  • make clippy equivalent (cargo clippy --all-targets --features ilp-highs -- -D warnings, rerun offline): PASS

Semantic Review

  • evaluate() correctness: OK — reviewed against the added YES/NO instances; it enforces config length, homologous-pair equality, arc capacities, non-terminal flow conservation, and sink inflow threshold in the right places.
  • dims() correctness: OK — the model exposes one variable per arc with domain size capacity + 1.
  • Size getter / complexity consistency: OK — max_capacity, num_arcs, and num_vertices exist, and the exported catalog shows O((max_capacity + 1)^num_arcs) for the new model.
  • Example / paper consistency: OK — the canonical example JSON, unit tests, and paper example all describe the same 6-vertex YES instance.

Issue Compliance

# Check Status
1 Problem name matches issue OK — IntegralFlowHomologousArcs
2 Mathematical definition matches OK — directed graph, capacities, source/sink, homologous-pair equalities, and sink inflow requirement are all represented
3 Problem type matches OK — satisfaction problem (Metric = bool)
4 Type parameters match OK — no type parameters, matching the issue
5 Configuration space matches OK — one integer variable per arc with domain {0, ..., c(a)}
6 Feasibility check matches OK — capacity, conservation, homologous-pair equality, and required sink inflow are enforced
7 Objective / answer criterion matches OK — evaluate() returns whether the configuration is feasible
8 Complexity matches OK — implementation uses the more general exact-search bound (max_capacity + 1)^num_arcs, which specializes to the issue’s unit-capacity 2^num_arcs case

Summary

  • 18/19 structural checks passed.
  • 8/8 issue-compliance checks passed.
  • Failures / issues:
  • Deterministic review packet reports a whitelist failure because this model PR also changes CLI plumbing and auxiliary test/export files outside the current model-PR whitelist.

Quality Check

Quality Review

Design Principles

  • DRY: OK — the model logic is self-contained and follows the existing directed-flow model pattern without obvious copy-paste hazards beyond normal example duplication.
  • KISS: OK — both evaluate() and the CLI parsing branch are straightforward and match repo conventions.
  • HC/LC: OK — model semantics stay in src/models/..., while CLI behavior stays in problemreductions-cli/....

HCI (CLI changed)

  • Error messages: OK — e.g. missing --homologous-pairs yields a specific error plus a complete usage string.
  • Discoverability: OK — pred list, pred show IntegralFlowHomologousArcs, and CLI help all surface the new model and its flags.
  • Consistency: OK — the new flags reuse existing directed-flow naming conventions (--arcs, --capacities, --source, --sink, --requirement).
  • Least surprise: ISSUE — from a user’s perspective the model appears in the catalog but is disconnected from the reduction graph (pred show reports 0 incoming / 0 outgoing reductions, and pred path IntegralFlowHomologousArcs ILP fails).
  • Feedback: OK — pred create --example writes the instance file, and pred solve returns structured JSON with the satisfying assignment.

Test Quality

  • Naive test detection: ISSUE
    • problemreductions-cli/tests/cli_tests.rs:672 only checks that create wrote a file whose top-level type is IntegralFlowHomologousArcs; it does not assert the generated payload fields (graph, capacities, requirement, homologous_pairs) or round-trip the instance through a behavior check.
    • src/unit_tests/models/graph/integral_flow_homologous_arcs.rs:6 and the added CLI tests exercise only unit-capacity instances. The model’s key generalization is integral capacities, but there is no positive test covering capacities greater than 1 or domains larger than binary.

Issues

Critical (Must Fix)

None.

Important (Should Fix)

  • The user-facing reduction-graph workflow is currently incomplete for this model: pred show IntegralFlowHomologousArcs reports no reductions, and pred path IntegralFlowHomologousArcs ILP returns Error: No reduction path from IntegralFlowHomologousArcs to ILP.
  • The new CLI creation test is too shallow to catch field-wiring regressions (problemreductions-cli/tests/cli_tests.rs:672).
  • The test suite does not cover any valid non-unit-capacity instance, leaving the general integral-capacity path unexercised (src/unit_tests/models/graph/integral_flow_homologous_arcs.rs:6).

Minor (Nice to Have)

None.

Summary

  • User-facing brute-force flows work, but reduction-path workflows do not yet work for this model.
  • CLI creation coverage should assert the serialized payload, not only the top-level problem type.
  • Model coverage should include at least one valid capacity > 1 instance.

Agentic Feature Tests

Feature Test Report: problem-reductions

Project type: Rust library + CLI
Feature tested: IntegralFlowHomologousArcs
Profile: ephemeral read-only review run
Use Case: downstream CLI user trying to discover, inspect, create, and solve the newly added model from documented CLI flows
Expected Outcome: the model should be discoverable from the CLI, example generation should work, solving the example should work, and the model should participate in the reduction graph to the extent implied by the project’s reduction-centric workflow
Verdict: fail
Critical Issues: 0

Summary

Feature Discoverable Setup Works Expected Outcome Met Doc Quality
IntegralFlowHomologousArcs yes yes partial partial good

Per-Feature Details

IntegralFlowHomologousArcs

  • Role: downstream CLI user validating a newly added model.
  • Use Case: find the model, inspect its schema, generate an example instance, and solve it.
  • What was tried:
  • pred list — confirmed the model is listed.
  • pred show IntegralFlowHomologousArcs — confirmed schema/complexity rendering works.
  • pred create --example IntegralFlowHomologousArcs -o /tmp/ifha-example.pkJB49 — confirmed example generation works.
  • pred solve /tmp/ifha-example.pkJB49 --solver brute-force — confirmed solving works and returns a satisfying assignment.
  • pred create IntegralFlowHomologousArcs ... without --homologous-pairs — confirmed the error message is actionable.
  • pred path IntegralFlowHomologousArcs ILP — reproduced missing reduction-path support.
  • Discoverability: good; generic CLI help and catalog commands were enough.
  • Setup: no blocker after using the local workspace build offline.
  • Functionality: partial; model creation/show/solve work, but reduction-path usage does not.
  • Expected vs Actual: expected a reduction-graph-native model; actual behavior is a brute-force-solvable standalone model with no available reduction path.
  • Blocked steps: none.
  • Friction points: the model appears in a reduction-graph tool yet currently shows zero incoming/outgoing reductions.
  • Doc suggestions: if the disconnected state is intentional for now, note that explicitly in final review or land the corresponding rule PR(s) before treating the model as graph-ready.

Issues Found

  • Confirmed / Important: pred show IntegralFlowHomologousArcs reports Outgoing reductions (0) and Incoming reductions (0).
  • Confirmed / Important: pred path IntegralFlowHomologousArcs ILP fails with Error: No reduction path from IntegralFlowHomologousArcs to ILP.
  • Not reproducible in current worktree: none.

Suggestions

  • Merge or sequence the corresponding rule PR(s) so the model is connected before downstream users rely on reduction-path workflows.
  • Strengthen CLI tests to validate the actual serialized example payload.
  • Add at least one positive non-unit-capacity test case for the model.

Generated by review-pipeline

# Conflicts:
#	docs/paper/reductions.typ
#	problemreductions-cli/src/cli.rs
#	problemreductions-cli/tests/cli_tests.rs
#	src/lib.rs
#	src/models/graph/mod.rs
#	src/models/mod.rs
@GiggleLiu GiggleLiu mentioned this pull request Mar 22, 2026
3 tasks
@GiggleLiu GiggleLiu merged commit e770c59 into main Mar 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Model] IntegralFlowHomologousArcs

1 participant