Improves nextcheck result and fixes nextcheck bug#123
Improves nextcheck result and fixes nextcheck bug#123merschformann wants to merge 15 commits intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the nextcheck output schema to distinguish between executable vs plannable best moves, fixes an inverted planning_makes_objective_worse interpretation, and updates Python test scaffolding for nextmv>=1.0.0.
Changes:
- Add
has_executable_best_move/was_executableflags to report moves that satisfy constraints even if they don’t improve the objective. - Add
objective_deltas(high verbosity) to show per-objective-term estimated deltas for candidate moves. - Update Python golden-test runner and dependency versions to align with
nextmvAPI changes.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
check/check.go |
Computes new executable/plannable flags and emits objective_deltas; adjusts when planning_makes_objective_worse is set. |
check/schema/schema.go |
Extends JSON schema with new fields for executability and per-term objective deltas. |
tests/check/input.json.golden |
Updates expected nextcheck JSON output to include the new fields. |
src/tests/solve_golden/main.py |
Updates test harness to new nextmv API (Option, load, write). |
requirements.txt |
Bumps nextmv dependency to >=1.0.0. |
.github/workflows/python-test.yml |
Updates the Python CI matrix (3.9 → 3.10). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
check/check.go:369
- End-of-loop aggregation doesn’t appear to handle the “executable but not plannable” case described in the PR:
PlanningMakesObjectiveWorseis only set whenhasWorseMovebecomes true (which currently requires executing an estimated-improving move that then doesn’t plan). If all executable best moves are non-improving (bestMove.IsImprovement() == false), this flag stays false even though the best executable move worsens the objective.- The summary path treats
HasPlannableBestMove == falseas “no move” and incrementsPlanUnitsHaveNoMove, even whenHasExecutableBestMoveis true.
Consider basing the “no move” summary onHasExecutableBestMove(or introducing a separate counter), and settingPlanningMakesObjectiveWorsewhenever an executable move exists but no plannable/improving move exists (using the best executable move’s value/verification logic).
if !hasImprovingMove && hasWorseMove {
m.output.PlanUnits[solutionPlanUnitIdx].PlanningMakesObjectiveWorse = true
m.output.Summary.NumberOfPlanUnitsMakingObjectiveWorse++
}
if m.output.PlanUnits[solutionPlanUnitIdx].HasPlannableBestMove {
m.output.Summary.PlanUnitsBestMoveFound++
} else {
m.output.Summary.PlanUnitsHaveNoMove++
constraints := make(map[string]int, len(m.solution.Model().Constraints()))
m.output.PlanUnits[solutionPlanUnitIdx].Constraints = constraints
for _, constraint := range planUnitsObserver.Constraints() {
name := fmt.Sprintf("%v", constraint)
if _, ok := constraints[name]; !ok {
constraints[name] = 0
}
constraints[name]++
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
This PR means to improve nextcheck output by distinguishing plannable and executable moves, i.e., a plannable move can be executed AND improves the solution, while an executable move can be executed but does not improve the solution.
Furthermore, fixes a bug in nextcheck where
planning_makes_objective_worsewas flipped. It now accurately expresses whether the best move for a plan unit improves the objective or not.At last, we add
objective_deltasthat list the delta / improvement of each objective term defined for all best moves found.Changes
has_executable_best_movefor a plan unit indicating whether the best move for a plan unit could be executed, i.e., did not violate any constraint.was_executablefor a move on a vehicle indicating whether the respective move was executable, i.e., did not violate any constraint.planning_makes_objective_worsewas incorrectly stating whether a move made the objective worse.objective_deltasshowing the delta of each objective term for all best moves found (only available inhighverbosity).nextmvPython dependency for tests to work again.Preview
The new features can to some extent already be seen in the golden file test expectation update (
tests/check/input.json.golden) part of this PR.