Skip to content

Fix #448: [Model] ConjunctiveQueryFoldability#687

Merged
zazabap merged 7 commits intomainfrom
issue-448-conjunctive-query-foldability
Mar 19, 2026
Merged

Fix #448: [Model] ConjunctiveQueryFoldability#687
zazabap merged 7 commits intomainfrom
issue-448-conjunctive-query-foldability

Conversation

@GiggleLiu
Copy link
Copy Markdown
Contributor

Summary

Add the ConjunctiveQueryFoldability problem model (Garey & Johnson A4 SR30). This is a classical NP-complete satisfaction problem from database theory that asks whether one conjunctive query can be folded into another via substitution of undistinguished variables.

Fixes #448

Add the Conjunctive Query Foldability problem (Garey & Johnson A4 SR30).
NP-complete satisfaction problem from database theory — determines if one
conjunctive query can be folded into another via substitution of
undistinguished variables.

- Model in src/models/misc/ with Term enum, validation, declare_variants!
- 7 unit tests (YES/NO instances, solver, serialization, constants)
- Paper entry with formal definition, CeTZ diagram, worked example
- CLI discovery via ProblemSchemaEntry, example-db canonical instance
@GiggleLiu
Copy link
Copy Markdown
Contributor Author

Implementation Summary

Changes

  • src/models/misc/conjunctive_query_foldability.rs — New model with Term enum, ConjunctiveQueryFoldability struct, Problem/SatisfactionProblem traits, validation, declare_variants!, and example-db builder
  • src/unit_tests/models/misc/conjunctive_query_foldability.rs — 7 tests: creation, YES/NO instances, solver, serialization, paper example, constants
  • src/models/misc/mod.rs — Module registration and example-db wiring
  • src/models/mod.rs — Re-exports for ConjunctiveQueryFoldability and Term
  • src/example_db/fixtures/examples.json — Regenerated (39→40 model examples)
  • docs/paper/reductions.typ — Problem definition entry with CeTZ diagram
  • problemreductions-cli/src/commands/create.rs — Usage hint and error message for complex input

Deviations from Plan

  • Skipped trait_consistency.rs entry — file doesn't exist on current origin/main (removed in refactor PR Refactor generated doc exports and remove centralized trait tests #676)
  • Paper example produces 4 satisfying configs (not 1) because σ(U2) is unconstrained — U2 doesn't appear in Q1
  • CLI pred create ConjunctiveQueryFoldability provides a helpful error directing users to --example rather than supporting flat flags for this complex input structure

Open Questions

  • None

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new NP-complete satisfaction model, ConjunctiveQueryFoldability, to the misc model suite, including schema registration and example/CLI/docs integration.

Changes:

  • Introduces ConjunctiveQueryFoldability model + Term representation, with schema registration, validation, evaluation, and variant complexity declaration.
  • Adds unit tests and an example-db fixture entry for the new model.
  • Updates CLI pred create messaging and the paper docs to include the new problem.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/models/misc/conjunctive_query_foldability.rs New model implementation, schema entry, evaluation logic, variants, and example-db spec hook.
src/models/misc/mod.rs Registers the new misc module and re-exports ConjunctiveQueryFoldability/Term; adds example-db spec extension.
src/models/mod.rs Re-exports the new misc model (and Term) at the top-level models module.
src/unit_tests/models/misc/conjunctive_query_foldability.rs Adds unit tests covering dims, evaluation semantics, brute-force solving, and serde roundtrip.
src/example_db/fixtures/examples.json Adds a canonical example instance and satisfying configs for the example DB.
problemreductions-cli/src/commands/create.rs Adds an example_for entry and an explicit pred create bail-out message for complex nested input.
docs/paper/reductions.typ Adds problem name mapping and a formal definition + example/figure to the paper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +46 to +49
/// distinguished variables `X`, and undistinguished variables `Y`, this problem
/// asks: do there exist two conjunctive queries Q1 and Q2 (over `X ∪ Y`) such
/// that a substitution `σ: Y → X ∪ Y ∪ D` maps every atom of Q1 to an atom
/// of Q2?
/// ],
/// vec![
/// (0, vec![Term::Distinguished(0), Term::Distinguished(0)]),
/// (0, vec![Term::Distinguished(0), Term::Distinguished(0)]),
Comment thread src/models/mod.rs
Comment on lines 23 to 27
pub use misc::{
BinPacking, Factoring, FlowShopScheduling, Knapsack, LongestCommonSubsequence,
MinimumTardinessSequencing, PaintShop, SequencingWithinIntervals, ShortestCommonSupersequence,
SubsetSum,
BinPacking, ConjunctiveQueryFoldability, Factoring, FlowShopScheduling, Knapsack,
LongestCommonSubsequence, MinimumTardinessSequencing, PaintShop, SequencingWithinIntervals,
ShortestCommonSupersequence, SubsetSum, Term,
};
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.39%. Comparing base (da983fa) to head (e0e9a02).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #687      +/-   ##
==========================================
+ Coverage   97.36%   97.39%   +0.03%     
==========================================
  Files         337      339       +2     
  Lines       43123    43413     +290     
==========================================
+ Hits        41988    42284     +296     
+ Misses       1135     1129       -6     

☔ 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
Copy Markdown
Contributor Author

Agentic Review Report

Note: CI is currently failing (1 failing check). Patch coverage is 84.73% (project: 96.97%).


Structural Check

# Check Status
1 Model file exists (src/models/misc/conjunctive_query_foldability.rs) PASS
2 inventory::submit! present PASS
3 #[derive(...Serialize, Deserialize)] on struct PASS
4 Problem trait impl PASS
5 SatisfactionProblem impl PASS
6 #[cfg(test)] + #[path = "..."] test link PASS
7 Test file exists (src/unit_tests/models/misc/conjunctive_query_foldability.rs) PASS
8 Test file has >= 3 test functions PASS — 7 test functions
9 Registered in misc/mod.rs PASS
10 Re-exported in models/mod.rs PASS (both ConjunctiveQueryFoldability and Term)
11 declare_variants! entry exists PASS (default sat)
12 CLI resolve_alias entry PASS — catalog-driven via ProblemSchemaEntry
13 CLI create support PASS — handled with bail directing to --example
14 Canonical model example registered PASS — present in examples.json fixture
15 Paper display-name entry PASS
16 Paper problem-def block PASS

Issue Compliance:

# Check Status
1 Problem name matches issue OK
2 Mathematical definition matches OK
3 Problem type (sat) matches OK
4 Type parameters match OK — no type parameters
5 Configuration space matches OK
6 Feasibility check matches OK
7 Objective function N/A — satisfaction problem
8 Complexity matches OK

Build: make test PASS, make clippy PASS, make fmt-check PASS

Semantic Review:

  • evaluate() correctness: OK — correctly applies substitution, collects atoms as HashSet, compares with Q2
  • dims() correctness: OK — vec![domain_size + num_distinguished + num_undistinguished; num_undistinguished]
  • Complexity string: OK — matches brute-force enumeration analysis
  • Validation: OK — new() checks all structural invariants
  • Paper entry: OK — precise definition, proper citations, CeTZ diagram

Issues found:

  1. [Important] Incorrect comment in doc example at src/models/misc/conjunctive_query_foldability.rs:71: The comment // sigma: u -> x (index = domain_size + 0 = 1) should read (index = domain_size + 0 = 0) since domain_size is 0 in the example. Code is correct; only the comment is wrong.

Quality Check

Design Principles: DRY OK, KISS OK, HC/LC OK

HCI (CLI changes): Error messages OK, discoverability OK, consistency OK

Issues found:

  1. [Important] Weak serialization test (src/unit_tests/models/misc/conjunctive_query_foldability.rs:115-120): The roundtrip test only asserts restored.dims() == problem.dims(). This depends on just 3 of 6 struct fields. It does not verify that relation_arities, query1_conjuncts, or query2_conjuncts survive the roundtrip. Peer models in misc/ check multiple individual fields.

  2. [Important] Missing tests for evaluate guard clauses (src/models/misc/conjunctive_query_foldability.rs:289-295): The evaluate function has explicit guards returning false for wrong-length configs and out-of-range values. These defensive branches are untested, impacting the 84.73% patch coverage.

  3. [Minor] Missing test for num_undistinguished == 0 boundary case: The dims() doc comment explicitly calls out this edge case but it is never exercised in tests.

  4. [Minor] Q2 set rebuilt on every evaluate call (src/models/misc/conjunctive_query_foldability.rs:311): The Q2 HashSet is reconstructed from scratch on every invocation. A cached Q2 set could improve solver performance. Not a correctness issue.

  5. [Minor] PartialEq not derived on ConjunctiveQueryFoldability: Deriving PartialEq would enable stronger serialization roundtrip assertions (assert_eq!(restored, problem)).


Agentic Feature Tests

# Test Result
1 pred list — CQF appears in catalog PASS
2 pred show ConjunctiveQueryFoldability — details display PASS
3 pred create --example ConjunctiveQueryFoldability — valid JSON PASS
4 pred solve (brute-force) — finds satisfying config [3,3,0] PASS
5 pred create (no flags) — graceful degradation with help PASS
6 Unit tests (cargo test --lib conjunctive_query_foldability) — 7/7 PASS
7 Example-db fixture consistency — 30/30 PASS

All tests pass. The model is correctly integrated: appears in catalog, displays proper details, produces valid example, solves correctly with brute-force. The example is mathematically sound (triangle+self-loop folds into lollipop, 4 satisfying configs confirmed).

Suggestions (pre-existing, not regressions):

  • Schema-driven help for pred create ConjunctiveQueryFoldability lists non-existent CLI flags — affects all complex-input problems
  • Build warning from duplicate "Vec<u64>" match arm in create.rs:236 (pre-existing)

Generated by review-pipeline

zazabap and others added 3 commits March 19, 2026 08:43
Resolve conflicts: keep ConjunctiveQueryFoldability alongside newly
added models (ConjunctiveBooleanQuery, PartiallyOrderedKnapsack, etc.).
Adapt ModelExampleSpec to the new API.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix struct doc: clarify Q1/Q2 are instance, σ is the decision
- Simplify doctest Q2 to single atom (duplicates irrelevant)
- Add ConjunctiveQueryFoldability and Term to lib.rs prelude
- Add getter tests, edge-case tests, and validation panic tests
  to improve coverage

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix incorrect comment in doctest (index = 0, not 1)
- Strengthen serialization roundtrip test (check all fields)
- Add num_undistinguished==0 edge case tests (trivial fold)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zazabap
Copy link
Copy Markdown
Collaborator

zazabap commented Mar 19, 2026

Final review notes:

  • Merged main, resolved conflicts (ConjunctiveBooleanQuery, PartiallyOrderedKnapsack, etc.)
  • Adapted ModelExampleSpec to the new API (instance/optimal_config/optimal_value fields)
  • Addressed all 3 inline review comments:
    1. Fixed struct doc: clarified Q1/Q2 are instance, σ is the decision
    2. Simplified doctest Q2 to single atom (duplicates irrelevant per set semantics)
    3. Added ConjunctiveQueryFoldability and Term to lib.rs prelude
  • Addressed agentic review items:
    • Fixed incorrect comment in doctest (index = 0, not 1)
    • Strengthened serialization roundtrip test (checks all fields + evaluation)
    • Added num_undistinguished == 0 edge case tests
    • Added evaluate guard clause tests (wrong length, out of range)
    • Added validation panic tests (bad relation index, arity, Distinguished, Constant, Undistinguished)
    • Added getter coverage tests

All local checks pass (make check green). 17 tests total (up from 7).

Copy link
Copy Markdown
Collaborator

@zazabap zazabap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final review passed. All inline comments and agentic review items addressed. Coverage improved with 10 additional tests. LGTM.

Resolve conflicts: keep ConjunctiveQueryFoldability alongside newly
added ResourceConstrainedScheduling and PartitionIntoPathsOfLength2.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zazabap zazabap merged commit 3920752 into main Mar 19, 2026
5 checks passed
@GiggleLiu GiggleLiu deleted the issue-448-conjunctive-query-foldability branch April 12, 2026 00:53
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] ConjunctiveQueryFoldability

3 participants