Skip to content

Fix #511: [Model] TimetableDesign#719

Merged
GiggleLiu merged 7 commits intomainfrom
issue-511
Mar 21, 2026
Merged

Fix #511: [Model] TimetableDesign#719
GiggleLiu merged 7 commits intomainfrom
issue-511

Conversation

@GiggleLiu
Copy link
Contributor

Summary

  • add the implementation plan for the TimetableDesign model from issue [Model] TimetableDesign #511
  • scope the worktree execution into model, CLI/example-db, and paper batches

Fixes #511

@codecov
Copy link

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 96.29630% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.59%. Comparing base (fc1cdf8) to head (abf52dc).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/models/misc/timetable_design.rs 93.83% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #719      +/-   ##
==========================================
- Coverage   97.60%   97.59%   -0.01%     
==========================================
  Files         389      393       +4     
  Lines       48478    49051     +573     
==========================================
+ Hits        47316    47873     +557     
- Misses       1162     1178      +16     

☔ 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

  • Added the Timetable Design paper entry in docs/paper/reductions.typ, including a worked timetable table derived from the canonical example DB instance.
  • Added a native Timetable Design satisfying-assignment search in src/models/misc/timetable_design.rs and exposed it through src/solvers/ilp/solver.rs so large canonical examples can be validated by the example-db optimality audit.
  • Strengthened src/unit_tests/models/misc/timetable_design.rs with constructor panic coverage plus direct/unsat solver-dispatch tests.
  • Fixed problemreductions-cli/src/commands/create.rs so malformed --craftsman-avail / --task-avail input reports the correct flag with the TimetableDesign usage string, and expanded the CLI JSON round-trip assertions to cover the structured matrices.

Deviations from Plan

  • Added a native solver hook for Timetable Design. The original plan kept solver scope to brute-force only, but the canonical 5x5x3 example is too large for the repo's small-instance brute-force audit, so this direct search was needed to keep the canonical example and satisfy make test.
  • Tightened model/CLI tests beyond the initial plan to cover constructor panics and the flag-specific CLI regression surfaced in review.

Open Questions

  • None.

@GiggleLiu
Copy link
Contributor Author

Agentic Review Report

Structural Check

Structural Review: model TimetableDesign

Structural Completeness

# Check Status
1 Model file exists PASS — src/models/misc/timetable_design.rs:1
2 inventory::submit! present PASS — src/models/misc/timetable_design.rs:11
3 Serialize / Deserialize derive PASS — src/models/misc/timetable_design.rs:35
4 Problem trait impl PASS — src/models/misc/timetable_design.rs:299
5 SatisfactionProblem impl PASS — src/models/misc/timetable_design.rs:349
6 #[cfg(test)] + #[path = ...] link PASS — src/models/misc/timetable_design.rs:416
7 Test file exists PASS — src/unit_tests/models/misc/timetable_design.rs:1
8 Test file has >= 3 test functions PASS — 18 test functions
9 Registered in misc/mod.rs PASS — src/models/misc/mod.rs:61
10 Re-exported in models/mod.rs PASS — src/models/mod.rs:32
11 declare_variants! entry exists PASS — src/models/misc/timetable_design.rs:351
12 CLI resolve_alias entry PASS — current CLI resolution is registry-backed in problemreductions-cli/src/problem_name.rs:16
13 CLI create support PASS — flags in problemreductions-cli/src/cli.rs:555, creation/validation in problemreductions-cli/src/commands/create.rs:2548
14 Canonical model example registered PASS — example spec in src/models/misc/timetable_design.rs:407, category aggregation in src/models/misc/mod.rs:93, generic builder in src/example_db/model_builders.rs:3
15 Paper display-name entry PASS — docs/paper/reductions.typ:146
16 Paper problem-def block PASS — docs/paper/reductions.typ:3490
17 Whitelist policy PASS — deterministic whitelist hits were direct TimetableDesign integration points, not unrelated churn
18 Blacklisted auto-generated files absent PASS

Build Status

  • make test: PASS
  • make clippy: PASS

Semantic Review

  • evaluate() correctness: OK — rejects wrong-length / non-binary configs, unavailable assignments, double bookings, and exact-count mismatches in src/models/misc/timetable_design.rs:307.
  • dims() correctness: OK — one binary variable per (craftsman, task, period) as documented in src/models/misc/timetable_design.rs:32 and implemented in src/models/misc/timetable_design.rs:303.
  • Size getter consistency: OK — num_periods, num_craftsmen, and num_tasks getters in src/models/misc/timetable_design.rs:123 match the registered complexity expression in src/models/misc/timetable_design.rs:351.
  • Weight handling: OK — unweighted satisfaction model.
  • Native solver hook: OK — direct backtracking solver in src/models/misc/timetable_design.rs:162 is isolated behind ILP solver dispatch in src/solvers/ilp/solver.rs:175 and covered by src/unit_tests/models/misc/timetable_design.rs:133.

Issue Compliance

# Check Status
1 Problem name matches issue OK
2 Mathematical definition matches OK
3 Problem type matches OK
4 Type parameters match OK
5 Configuration space matches OK
6 Feasibility check matches OK
7 Objective function matches OK
8 Complexity matches OK

Summary

  • 18/18 structural checks passed
  • 8/8 issue compliance checks passed
  • No FAIL/ISSUE items

Quality Check

Quality Review

Design Principles

  • DRY: OK — the new create-path parsing/validation is reasonably shared.
  • KISS: ISSUE — ILPSolver::solve_dyn now has a hidden native TimetableDesign branch, so “ILP solver” no longer means “solve ILP or reduce to ILP.” That extra path is bespoke backtracking inside the model. (src/solvers/ilp/solver.rs:178, src/models/misc/timetable_design.rs:162)
  • HC/LC: ISSUE — solver-specific search logic now lives inside the model and is reached via downcasting from the generic ILP solver, coupling model internals to solver dispatch instead of the reduction graph or a dedicated solver interface. (src/models/misc/timetable_design.rs:162, src/solvers/ilp/solver.rs:178)

HCI

  • Error messages: OK — pred create TimetableDesign validates required flags and matrix dimensions with concrete messages. (problemreductions-cli/src/commands/create.rs:2549, problemreductions-cli/src/commands/create.rs:4006)
  • Discoverability: ISSUE — help and inspect still define ILP support in terms of reduction-graph reachability, so TimetableDesign can be solvable with --solver ilp while pred inspect advertises only brute-force, and solve help still says non-ILP problems need an ILP reduction path. (problemreductions-cli/src/cli.rs:640, problemreductions-cli/src/commands/inspect.rs:43, problemreductions-cli/src/dispatch.rs:48)
  • Consistency: ISSUE — solve_with_ilp still documents itself as “auto-reduce to ILP,” but TimetableDesign now bypasses that route entirely. (problemreductions-cli/src/dispatch.rs:63, src/solvers/ilp/solver.rs:185)
  • Least surprise: ISSUE — pred solve --solver ilp prints ilp (via ILP) and emits reduced_to: "ILP" for TimetableDesign even though no reduction happened. The MCP solve tool emits the same misleading JSON. (problemreductions-cli/src/commands/solve.rs:99, problemreductions-cli/src/mcp/tools.rs:1372, src/solvers/ilp/solver.rs:185)
  • Feedback: ISSUE — the success output confirms the wrong thing: users are told they solved “via ILP” instead of through a model-specific native hook. (problemreductions-cli/src/commands/solve.rs:101, problemreductions-cli/src/mcp/tools.rs:1380)

Test Quality

  • Naive test detection: ISSUE
  • The new solver tests only prove “returns a satisfying config / returns None” for direct dispatch; they do not cover the user-facing solver contract that changed most here: advertised solver availability, reduced_to metadata, or help/inspect output. (src/unit_tests/models/misc/timetable_design.rs:135, src/unit_tests/models/misc/timetable_design.rs:146)
  • The added CLI tests only exercise pred create for TimetableDesign; there is no integration coverage for pred solve --solver ilp or pred inspect, which is why the solver-path mismatch escaped. (problemreductions-cli/src/commands/create.rs:4975, problemreductions-cli/src/commands/create.rs:5055)

Issues

Critical (Must Fix)

  • None.

Important (Should Fix)

  • The new TimetableDesign native hook bypasses the reduction graph, but CLI/MCP/help/inspect still behave as if --solver ilp always means “reduce to ILP, then solve.” That produces contradictory UX: pred inspect can hide ILP support, pred solve can claim via ILP, and JSON can say reduced_to: "ILP" when no reduction occurred. (src/solvers/ilp/solver.rs:178, problemreductions-cli/src/dispatch.rs:48, problemreductions-cli/src/commands/inspect.rs:43, problemreductions-cli/src/commands/solve.rs:99, problemreductions-cli/src/mcp/tools.rs:1372, problemreductions-cli/src/cli.rs:640)
  • There is no integration test for the new solver behavior at the CLI boundary, so the regression above is unguarded. Add coverage that TimetableDesign is surfaced consistently as a native ILP-backed path or reported honestly as a non-reduction path in solve, inspect, and JSON/MCP output. (src/unit_tests/models/misc/timetable_design.rs:135, problemreductions-cli/src/commands/create.rs:4975)

Minor (Nice to Have)

  • The model now owns a custom backtracking solver that is only callable through ILPSolver downcasting. Even if the UX mismatch is fixed, this remains a cohesion/coupling smell that will keep requiring special handling anywhere the codebase reasons about “ILP support.” (src/models/misc/timetable_design.rs:162, src/solvers/ilp/solver.rs:178)

Summary

  • Important: TimetableDesign’s new native ilp path is not represented in the reduction graph, but CLI/MCP/help/inspect still report it as a normal “via ILP” reduction.
  • Important: The review surface that changed most, solver semantics in solve/inspect, has no integration coverage.
  • Minor: The implementation reduces cohesion by embedding solver-specific search inside the model and discovering it through ILPSolver downcasting.

Agentic Feature Tests

Feature Test Report: problem-reductions CLI

Date: 2026-03-21
Project type: CLI
Features tested: TimetableDesign model CLI flow
Profile: ephemeral
Use Case: downstream user discovers, inspects, creates, and solves the new TimetableDesign model using CLI help and examples
Expected Outcome: TimetableDesign should appear in the catalog, show its schema, create a canonical example, solve it with the default solver, and report solver capability consistently
Verdict: fail
Critical Issues: 1

Summary

Feature Discoverable Setup Works Expected Outcome Met Doc Quality
TimetableDesign CLI flow yes yes partial partial partial

Per-Feature Details

TimetableDesign CLI flow

  • Role: downstream CLI user validating a newly added model
  • Use Case: discover the model, inspect its schema, materialize the canonical example, solve it, and sanity-check error UX
  • What they tried:
    • cargo run -q -p problemreductions-cli --bin pred -- list | rg TimetableDesign
    • cargo run -q -p problemreductions-cli --bin pred -- show TimetableDesign
    • cargo run -q -p problemreductions-cli --bin pred -- create --example TimetableDesign -o /tmp/...
    • target/debug/pred solve /tmp/... --json
    • malformed --craftsman-avail and --task-avail matrix inputs
    • target/debug/pred inspect /tmp/...
  • Discoverability: pred list and pred show TimetableDesign both worked.
  • Setup: no extra setup beyond building pred.
  • Functionality: pred create --example TimetableDesign wrote the canonical JSON instance; pred solve /tmp/... --json returned a satisfying solution with evaluation = true; malformed availability matrices now mention the correct flag and include the TimetableDesign usage string.
  • Expected vs Actual Outcome: the model is discoverable and solvable, but solver metadata/reporting is inconsistent.
  • Blocked steps: none.
  • Friction points: pred inspect /tmp/... reports only brute-force even though default pred solve succeeds with solver ilp; pred solve --json emits reduced_to: "ILP" although pred show TimetableDesign lists no outgoing reductions and the solve path is a native hook through ILPSolver::solve_dyn.
  • Doc suggestions: document native solver hooks distinctly from reduction-graph ILP support, or keep output aligned with the actual solver route.

Expected vs Actual Outcome

  • Partial: a user can solve the example successfully, but the CLI does not describe that solver route consistently.

Issues Found

  1. Confirmed — Important: target/debug/pred inspect /tmp/... reports "solvers":["brute-force"] even though target/debug/pred solve /tmp/... --json succeeds with "solver":"ilp". Root cause is problemreductions-cli/src/dispatch.rs:48 / problemreductions-cli/src/commands/inspect.rs:43, which only consider reduction-graph reachability to ILP and ignore the new native TimetableDesign hook in src/solvers/ilp/solver.rs:178.
  2. Confirmed — Important: target/debug/pred solve /tmp/... --json emits "reduced_to":"ILP" and text Solver: ilp (via ILP) even though no reduction exists for TimetableDesign and the native hook is used first in src/solvers/ilp/solver.rs:205. This is emitted from problemreductions-cli/src/commands/solve.rs:99.

Suggestions

  • Surface native solver hooks in supports_ilp_solver() / pred inspect.
  • Distinguish “solved natively” from “reduced to ILP” in CLI and MCP outputs.
  • Add CLI integration coverage for pred solve / pred inspect on TimetableDesign.

Generated by review-pipeline

@GiggleLiu
Copy link
Contributor Author

Follow-up items (recorded during final review):

  • The native solver hook in solve_dyn (src/solvers/ilp/solver.rs:185) bypasses the reduction graph for TimetableDesign. Once a TimetableDesign → ILP reduction rule is implemented (to be proposed via /propose), the hook can be removed.
  • pred inspect / pred solve metadata should be updated to properly distinguish native solver hooks from reduction-graph ILP paths (pre-existing architectural issue, not TimetableDesign-specific).

@GiggleLiu GiggleLiu mentioned this pull request Mar 21, 2026
3 tasks
@GiggleLiu GiggleLiu merged commit 2fec9f9 into main Mar 21, 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] TimetableDesign

1 participant