Skip to content

feat(breaks): trace vehicle breaks between problem and solution by id - #19

Merged
bayangan1991 merged 2 commits into
masterfrom
feat/vehicle-break-id-trace
Sep 9, 2026
Merged

bayangan1991 merged 2 commits into
masterfrom
feat/vehicle-break-id-trace

Conversation

@bayangan1991

Copy link
Copy Markdown

Problem

Vehicle breaks cannot be identified in the solution output.

  • Optional breaks become conditional jobs with a synthetic id, but the writer emits jobId: "break" for every break activity. The only trace available is the per-place tag, which is optional, sits on the place rather than the break, and is matched by location and time.
  • Required breaks never become jobs at all — they are flattened into reserved time spans keyed by actor, which drops every link back to the break in the problem. A tour with two of them gives no way to tell which is which.
  • Break violations name only the vehicle and shift index, so "which break was skipped" was unanswerable.

Change

Both VehicleBreak variants accept an optional id, propagated back to the solution as breakId on the break activity and on the break violation.

  • vrp-core: ReservedTimeSpan / ReservedTimeWindow carry an optional id — the only way a required break can keep its identity, since it never becomes a job.
  • Optional breaks store the id in a new BreakId dimension. The synthetic {vehicle}_break_{shift}_{idx} job id is left alone, because initial-solution reading keys off that naming.
  • breakId is a new optional field rather than an overload of jobId, so consumers filtering on jobId == "break" and the round-trip reader keep working.
  • The activity matcher and the checker prefer the id over time-window intersection when resolving a break, and the checker asserts the emitted id matches the problem.
  • New E1309 validation: break ids must be unique within a shift.

Existing problem JSON is unaffected — id is optional and skipped on serialization, and the untagged enum still disambiguates on places / duration.

Compatibility

Adding a public field to ReservedTimeSpan breaks external struct literals, hence the version bump to 1.25.7. All crates share workspace.package.version, so the whole workspace moves together.

Tests

New tests/features/breaks/break_id_test.rs covers optional breaks, required breaks, two breaks distinguished by id, violation propagation, and the id-omitted case; parameterized E1309 cases added to the validation tests. Full suite passes.

The second commit is a standalone cargo fmt --all over formatting drift that predates this work, so it does not muddy the feature diff. cargo fmt --all -- --check is now clean.

🤖 Generated with Claude Code

bayangan1991 and others added 2 commits September 8, 2026 10:24
Vehicle breaks were impossible to identify in the output. Optional breaks
surfaced as an activity with `jobId: "break"`, traceable only through the
per-place `tag`; required breaks are flattened into reserved time spans and
lost every link back to the problem, so a tour with two of them gave no way
to tell which was which. Break violations named only the vehicle and shift.

Both break variants now accept an optional `id` which is propagated back to
the solution as `breakId` on the break activity and on the break violation.

- vrp-core: `ReservedTimeSpan`/`ReservedTimeWindow` carry an optional id, the
  only way a required break can keep its identity, since it never becomes a job
- optional breaks store the id in a new `BreakId` dimension; the synthetic
  `{vehicle}_break_{shift}_{idx}` job id is left alone because initial solution
  reading keys off that naming
- `breakId` is a new field rather than an overload of `jobId`, so consumers
  filtering on `jobId == "break"` and the round trip reader keep working
- the activity matcher and checker prefer the id over time window intersection
  when resolving a break, and the checker asserts the emitted id matches
- new E1309 validation: break ids must be unique within a shift

Adding a public field to `ReservedTimeSpan` breaks external struct literals,
hence the version bump to 1.25.7.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Formatting drift left over from earlier changes, unrelated to any behaviour.
`cargo fmt --all -- --check` is now clean, so a future edit to one of these
files no longer drags unrelated reformatting into its diff.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@hutchinsp01 hutchinsp01 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Cool, lots of plumbing but can't see anything out of place

Comment on lines +264 to +274
fn check_e1309_vehicle_break_ids_are_unique(ctx: &ValidationContext) -> Result<(), FormatError> {
let duplicates = ctx
.vehicles()
.flat_map(|vehicle| vehicle.shifts.iter())
.filter_map(|shift| {
get_duplicates(shift.breaks.iter().flat_map(|breaks| breaks.iter()).filter_map(|br| br.id()))
})
.flatten()
.collect::<HashSet<_>>();

if duplicates.is_empty() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we ever send up the same taskSession to two seperate shifts for multiple techs?

Or are you going to handle that as 2 seperate break id's on the WF side?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I reckon we probably do but I think I'll burn that bridge when we get to it. I expect we will just send it as two separate breaks with a guid or something.

@bayangan1991
bayangan1991 merged commit 631a8eb into master Sep 9, 2026
5 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.

2 participants