Skip to content

Fix #510: [Model] JobShopScheduling#760

Open
GiggleLiu wants to merge 3 commits intomainfrom
issue-510
Open

Fix #510: [Model] JobShopScheduling#760
GiggleLiu wants to merge 3 commits intomainfrom
issue-510

Conversation

@GiggleLiu
Copy link
Contributor

Summary

  • Add the execution plan for issue [Model] JobShopScheduling #510 ([Model] JobShopScheduling)
  • Track the work needed for the model, CLI support, example-db coverage, and paper entry

Fixes #510

@codecov
Copy link

codecov bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.65%. Comparing base (5441368) to head (162ff2a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #760   +/-   ##
=======================================
  Coverage   97.65%   97.65%           
=======================================
  Files         455      455           
  Lines       55706    55706           
=======================================
  Hits        54401    54401           
  Misses       1305     1305           

☔ 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 JobShopScheduling in src/models/misc/job_shop_scheduling.rs with validation, Lehmer-code machine-order dimensions, schedule reconstruction, and canonical example support.
  • Registered the model through the misc/model/prelude exports and extended example-db plus model tests, including brute-force and schedule-specific checks.
  • Extended the CLI create flow to support JobShopScheduling with --job-tasks, --deadline, and optional --num-processors.
  • Added the paper entry and worked example for Job-Shop Scheduling in docs/paper/reductions.typ.

Deviations from Plan

  • Used machine-order permutation encoding instead of direct start-time variables. The issue discussion clarified the canonical example expects enumeration over per-machine task orders, which also matches the cited 6! * 6! = 518400 search space.
  • Set the default complexity metadata to factorial(num_tasks) rather than factorial(num_jobs), because the encoded search space is over machine task orders.

Open Questions

  • None.

@GiggleLiu
Copy link
Contributor Author

Agentic Review Report

Structural Check

Structural Review: model JobShopScheduling

Structural Completeness

# Check Status
1 Model file exists PASS — src/models/misc/job_shop_scheduling.rs added
2 inventory::submit! present PASS — src/models/misc/job_shop_scheduling.rs:13
3 Serialize / Deserialize derive PASS — src/models/misc/job_shop_scheduling.rs:29
4 Problem trait impl PASS — src/models/misc/job_shop_scheduling.rs:216
5 Aggregate metric is present PASS — type Metric = bool at src/models/misc/job_shop_scheduling.rs:218
6 Test link present PASS — src/models/misc/job_shop_scheduling.rs:265
7 Test file exists PASS — src/unit_tests/models/misc/job_shop_scheduling.rs
8 Test file has >= 3 test functions PASS — 7 tests in src/unit_tests/models/misc/job_shop_scheduling.rs
9 Registered in misc/mod.rs PASS — src/models/misc/mod.rs:48, src/models/misc/mod.rs:87, src/models/misc/mod.rs:145
10 Re-exported in models/mod.rs PASS — src/models/mod.rs:40
11 Variant registration exists PASS — src/models/misc/job_shop_scheduling.rs:239
12 CLI name resolution entry PASS — problemreductions-cli/src/problem_name.rs:23
13 CLI create support PASS — problemreductions-cli/src/cli.rs:301, problemreductions-cli/src/commands/create.rs:3614
14 Canonical model example registered PASS — src/models/misc/mod.rs:145 feeds src/example_db/model_builders.rs
15 Paper display-name entry PASS — docs/paper/reductions.typ adds JobShopScheduling to display-name
16 Paper problem-def block PASS — docs/paper/reductions.typ adds a dedicated problem-def("JobShopScheduling") block
17 Deterministic file whitelist FAIL — model PR touched files outside the model whitelist: problemreductions-cli/src/cli.rs, problemreductions-cli/src/commands/create.rs, problemreductions-cli/src/problem_name.rs, src/lib.rs, src/models/mod.rs, src/unit_tests/example_db.rs, src/unit_tests/trait_consistency.rs
18 Blacklisted auto-generated files committed PASS — none found

Build Status

  • make test: PASS
  • make clippy: PASS

Semantic Review

  • evaluate() correctness: OK — schedule_from_config() decodes per-machine Lehmer orders, builds the combined precedence DAG, rejects cyclic orders, and enforces the global deadline before returning true.
  • dims() correctness: OK — src/models/misc/job_shop_scheduling.rs:224 returns per-machine Lehmer radices, so the search space matches the chosen machine-order encoding.
  • Size getter consistency: OK — num_tasks() exists and matches the declare_variants! complexity string at src/models/misc/job_shop_scheduling.rs:239.
  • Weight handling: N/A.
  • Validation path: ISSUE — the CLI validates processor bounds, but it does not validate the Garey-Johnson consecutive-processor constraint before calling JobShopScheduling::new(). Invalid input therefore panics inside src/models/misc/job_shop_scheduling.rs:60 instead of returning a normal CLI error.

Issue Compliance

# Check Status
1 Problem name matches issue OK
2 Mathematical definition matches OK — processor routes, precedence, machine capacity, and deadline are all represented
3 Problem framing matches OK — implemented as a satisfaction problem with Metric = bool
4 Type parameters match OK — none
5 Configuration space matches ISSUE — the linked issue body still describes start-time variables, but src/models/misc/job_shop_scheduling.rs:224 implements per-machine Lehmer-order variables instead
6 Feasibility check matches OK
7 Objective function matches OK
8 Complexity matches ISSUE — the linked issue body says factorial(num_jobs), while src/models/misc/job_shop_scheduling.rs:239 registers factorial(num_tasks)

Summary

  • 17/18 structural checks passed
  • 6/8 issue compliance checks passed
  • Deterministic whitelist failed because the PR also touches CLI / re-export / support files outside the model-only whitelist
  • CLI validation is incomplete for the consecutive-processor constraint, so invalid --job-tasks can panic the process
  • The implementation deliberately deviates from the linked issue on configuration space and complexity (num_tasks machine-order encoding instead of start-time variables / factorial(num_jobs))

Quality Check

Quality Review

Design Principles

  • DRY: OK — the model logic is centralized in src/models/misc/job_shop_scheduling.rs; the only duplicated checks are lightweight CLI guards around constructor preconditions.
  • KISS: OK — the machine-order encoding plus longest-path schedule reconstruction is a direct fit for this satisfaction model.
  • HC/LC: OK — scheduling semantics stay in the model file, while argument parsing and user messaging stay in the CLI crate.

HCI (CLI changed)

  • Error messages: ISSUE — pred create JobShopScheduling --job-tasks "0:1,0:1" --deadline 2 exits with code 101 and a Rust panic from src/models/misc/job_shop_scheduling.rs:60 instead of an actionable CLI validation error. The missing guard is in problemreductions-cli/src/commands/create.rs:3623.
  • Discoverability: OK — pred list, pred show JobShopScheduling, and bare pred create JobShopScheduling all expose the new model and show a working example.
  • Consistency: OK — --job-tasks, --deadline, and --num-processors/--m follow the existing scheduling command patterns.
  • Least surprise: OK — pred solve <instance> without --solver brute-force fails, but the error explicitly tells the user to retry with --solver brute-force.
  • Feedback: OK — pred create writes the JSON instance and pred solve / pred evaluate return clear machine-readable output.

Test Quality

  • Naive test detection: ISSUE
    • problemreductions-cli/src/commands/create.rs:7820 adds happy-path and malformed-token coverage, but it misses the regression where structurally invalid job routes (0:1,0:1) crash the CLI instead of producing a handled error.

Issues

Critical (Must Fix)

  • None.

Important (Should Fix)

  • Missing CLI validation for consecutive same-processor tasks causes a user-triggerable panic on invalid --job-tasks input. Reproduced with pred create JobShopScheduling --job-tasks "0:1,0:1" --deadline 2.

Minor (Nice to Have)

  • The linked-issue spec and implementation disagree on the chosen encoding / complexity string, which should be reconciled before merge so the issue, PR body, and catalog metadata say the same thing.

Summary

  • Valid JobShopScheduling flows are discoverable and work end-to-end.
  • Invalid job-route input currently crashes the CLI instead of returning a normal validation error.
  • The new tests are generally solid, but they do not cover the reproduced panic path.

Agentic Feature Tests

Feature Test Report: problem-reductions

Date: 2026-03-23
Project type: CLI + library
Features tested: JobShopScheduling
Profile: ephemeral
Use Case: Act as a downstream CLI user who wants to discover the new model, inspect it, create an example instance, create a manual instance, and solve/evaluate it from the command line.
Expected Outcome: The model is discoverable from CLI docs, valid instances can be created and solved with the documented brute-force flow, and invalid inputs fail gracefully with actionable feedback.
Verdict: pass
Critical Issues: 0

Summary

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

Per-Feature Details

JobShopScheduling

  • Role: downstream CLI user building and exercising pred from source.
  • Use Case: discover the new model, inspect its schema, create valid example/manual instances, solve them, and check how invalid route input is handled.
  • What they tried:
    • pred list
    • pred show JobShopScheduling
    • pred create JobShopScheduling with no flags
    • pred create --example JobShopScheduling
    • pred solve <instance> --solver brute-force
    • pred evaluate <instance> --config 0,0,0,0,0,0,1,3,0,1,1,0
    • pred create JobShopScheduling --job-tasks "0:3,1:4;1:2,0:3,1:2;0:4,1:3;1:5,0:2;0:2,1:3,0:1" --deadline 20 --num-processors 2
    • pred create JobShopScheduling --job-tasks "0:1,0:1" --deadline 2
  • Discoverability: Good. The model appears in pred list, pred show explains the fields and complexity, and bare pred create JobShopScheduling prints parameter help plus a concrete example command.
  • Setup: Good. target/debug/pred built successfully from the branch.
  • Functionality: Valid example and manual instances both serialize correctly and solve with --solver brute-force. pred evaluate on the paper witness returns true.
  • Expected vs Actual Outcome: The documented valid path works. Invalid route input does not fail gracefully: instead of a handled CLI error, the command panics.
  • Blocked steps: None.
  • Friction points: The invalid-input panic is the only reproduced friction point. Running pred solve <instance> without --solver brute-force fails, but the message is actionable and points to the right fix.
  • Doc suggestions: None required for the happy path; the main fix is implementation-side input validation.

Expected vs Actual Outcome

  • Expected: discoverable model with working create/show/solve flow and graceful handling of invalid input.
  • Actual: discoverability and valid create/show/solve flow are good; invalid consecutive same-processor tasks trigger a panic instead of a normal validation error.

Issues Found

  • Confirmed / Should Fix: pred create JobShopScheduling --job-tasks "0:1,0:1" --deadline 2 exits with code 101 and panics in src/models/misc/job_shop_scheduling.rs:60 because the CLI path does not validate the consecutive-processor constraint before calling the constructor.
  • Not a bug: pred solve <instance> without --solver brute-force fails with No reduction path ... Try --solver brute-force. This is expected and documented by the error message.

Suggestions

  • Validate the consecutive-processor constraint in problemreductions-cli/src/commands/create.rs before constructing JobShopScheduling, and return an anyhow error instead of letting the constructor panic.
  • Add a CLI regression test for the reproduced invalid-input path so this stays handled.

Generated by review-pipeline

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] JobShopScheduling

1 participant