feat(planning): plan-version diff and I_DISPLACED #95

Merged

patrick merged 1 commit from feature/api-diff into main

2026-09-11 14:07:17 +00:00

patrick commented

2026-09-11 14:03:39 +00:00

Owner

Description

A planner can now compare any two plan versions and see exactly what's different: which jobs are newly assigned, which dropped out, and which moved to a different crew shift, site or time — the same information the plan board's version diff and "what changed since yesterday" view need. The system also has a new, ready-to-use check for flagging a previously confirmed job that quietly disappeared or moved in a later draft without anyone explicitly saying why (an override, a triage reject, and so on) — the kind of silent side effect a planner would otherwise only notice by accident.

Closes #37

PH5.5 in docs/12-implementation-plan.md.

Opened #92 as a seam request (blocks this PR's issue): wiring the new I_DISPLACED check into the shared validator registry and its shared test files touches files outside this ticket's owned paths (they belong to an already-merged ticket's row), so that one-line registration is left for a follow-up rather than widening this ticket's scope.

Type of Change

New feature
Bug fix
Refactor / cleanup (no behaviour change)
Documentation
Chore / build / CI
Breaking change

Breaking Changes

None.

Test Plan

Tests green: new unit tests for the diff logic (pure function and repository-backed wrapper), new unit tests for the displacement check against hand-built bad-plan fixtures (a dropped job, a moved job, the same drop with an explicit override reason attached so it's correctly not flagged, and an unchanged job also correctly not flagged), and a real-Postgres integration test exercising the new endpoint end to end (added/removed/moved, and a 404 for an unknown version). Full existing unit and integration suites pass; mypy --strict and ruff are clean.

Checklist

Conventional commit(s), one logical change per commit
Tests added/updated and passing
docs/ updated for any behavioural change (ADR added if a prior decision was reversed)
No hard deletes; no direct status updates outside polaris.domain
No business logic in routers/components
Migration included if the schema changed (one per PR max, reversible downgrade)
No new dependency without a reason in the commit body

(docs/ needed no changes — docs/09-api.md and docs/04-lifecycles.md already documented this endpoint and finding; no schema change, no new dependency.)

Additional Context

Two judgment calls worth a reviewer's eyes:

Matching assignments across versions: a to-version row is matched to a from-version row by its predecessor_assignment_id link when present, falling back to matching on job_id when it isn't (e.g. two drafts in the same override chain where neither has been published yet, so nothing has a confirmed predecessor to point at). Documented in domain/planning/diff.py's module docstring.
"Explicit override reason" for suppressing I_DISPLACED: treated as an active lock on the job that carries a non-empty reason. docs/04-lifecycles.md doesn't spell out what counts as "explicit", so this is an interpretation call, documented in validator/rules/displaced.py's module docstring (same kind of call validator/rules/booking.py already makes for its own vehicle-overlap split).

Also found (and worked around, entirely inside api/routers/plans/diff.py) a pre-existing mismatch: polaris.db.repositories.planning.assignments_for_version()'s row mapping carries location as a GeoPoint object, but the AssignmentRead DTO expects a WKT string. Not a blocker since it's a one-line conversion at the boundary, but flagged here for whoever next touches that shared repository file.

Also updated tests/unit/api/routers/plans_locks_assignments_stub_test.py to drop the now-real diff path from its still-501 list — that file's own docstring already documents this exact precedent from when an earlier ticket's routers went live, so this follows it rather than opening a seam request for something the file itself anticipates.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01LoNrSy7Reyp7evkfcdHeLX

## Description A planner can now compare any two plan versions and see exactly what's different: which jobs are newly assigned, which dropped out, and which moved to a different crew shift, site or time — the same information the plan board's version diff and "what changed since yesterday" view need. The system also has a new, ready-to-use check for flagging a previously confirmed job that quietly disappeared or moved in a later draft without anyone explicitly saying why (an override, a triage reject, and so on) — the kind of silent side effect a planner would otherwise only notice by accident. ## Related Issues Closes #37 PH5.5 in `docs/12-implementation-plan.md`. Opened #92 as a seam request (blocks this PR's issue): wiring the new `I_DISPLACED` check into the shared validator registry and its shared test files touches files outside this ticket's owned paths (they belong to an already-merged ticket's row), so that one-line registration is left for a follow-up rather than widening this ticket's scope. ## Type of Change - [x] New feature - [ ] Bug fix - [ ] Refactor / cleanup (no behaviour change) - [ ] Documentation - [ ] Chore / build / CI - [ ] Breaking change ## Breaking Changes None. ## Test Plan Tests green: new unit tests for the diff logic (pure function and repository-backed wrapper), new unit tests for the displacement check against hand-built bad-plan fixtures (a dropped job, a moved job, the same drop with an explicit override reason attached so it's correctly *not* flagged, and an unchanged job also correctly not flagged), and a real-Postgres integration test exercising the new endpoint end to end (added/removed/moved, and a 404 for an unknown version). Full existing unit and integration suites pass; `mypy --strict` and `ruff` are clean. ## Checklist - [x] Conventional commit(s), one logical change per commit - [x] Tests added/updated and passing - [x] `docs/` updated for any behavioural change (ADR added if a prior decision was reversed) - [x] No hard deletes; no direct status updates outside `polaris.domain` - [x] No business logic in routers/components - [x] Migration included if the schema changed (one per PR max, reversible downgrade) - [x] No new dependency without a reason in the commit body (`docs/` needed no changes — `docs/09-api.md` and `docs/04-lifecycles.md` already documented this endpoint and finding; no schema change, no new dependency.) ## Additional Context Two judgment calls worth a reviewer's eyes: - **Matching assignments across versions**: a `to`-version row is matched to a `from`-version row by its `predecessor_assignment_id` link when present, falling back to matching on `job_id` when it isn't (e.g. two drafts in the same override chain where neither has been published yet, so nothing has a *confirmed* predecessor to point at). Documented in `domain/planning/diff.py`'s module docstring. - **"Explicit override reason"** for suppressing `I_DISPLACED`: treated as an active lock on the job that carries a non-empty `reason`. `docs/04-lifecycles.md` doesn't spell out what counts as "explicit", so this is an interpretation call, documented in `validator/rules/displaced.py`'s module docstring (same kind of call `validator/rules/booking.py` already makes for its own vehicle-overlap split). Also found (and worked around, entirely inside `api/routers/plans/diff.py`) a pre-existing mismatch: `polaris.db.repositories.planning.assignments_for_version()`'s row mapping carries `location` as a `GeoPoint` object, but the `AssignmentRead` DTO expects a WKT string. Not a blocker since it's a one-line conversion at the boundary, but flagged here for whoever next touches that shared repository file. Also updated `tests/unit/api/routers/plans_locks_assignments_stub_test.py` to drop the now-real diff path from its still-`501` list — that file's own docstring already documents this exact precedent from when an earlier ticket's routers went live, so this follows it rather than opening a seam request for something the file itself anticipates. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LoNrSy7Reyp7evkfcdHeLX

patrick added 1 commit

2026-09-11 14:03:39 +00:00

feat(planning): plan-version diff and I_DISPLACED 3ed910811f

Implements GET /plans/versions/{id}/diff/{other} (replacing the 501
stub) and the I_DISPLACED validator finding, per PH5.5.

- domain/planning/diff.py: pure diff_assignments(from_rows, to_rows)
  matches assignments across two versions via predecessor_assignment_id
  first, falling back to job_id when no lineage link exists (e.g. two
  drafts in the same override chain, neither ever published). Reports
  added / removed / moved (different shift, site or time) / unchanged.
  diff_versions(repository, ...) is the thin PlanningRepository-backed
  wrapper the router calls.
- api/routers/plans/diff.py: real implementation of the endpoint. Maps
  PlanDiff.moved onto the existing PlanVersionDiffResponse's `displaced`
  bucket (that DTO only has three buckets; `unchanged` has no wire
  representation, matching docs/06-ux.md's "what changed" framing).
  Also works around a pre-existing mismatch in
  polaris.db.repositories.planning: assignments_for_version()'s row
  mapping carries `location` as a GeoPoint (what
  domain.planning.validate.rebuild_result wants), not the WKT string
  AssignmentRead expects — converted back to WKT locally in this
  router rather than editing that shared, unowned file.
- validator/rules/displaced.py: check_displaced(snapshot, result) flags
  a previously confirmed assignment (snapshot.published_assignments)
  that is missing or moved in the new result, unless an active lock on
  that job carries an explicit reason (interpretation call, documented
  in the module docstring). Not yet wired into polaris.validator.RULES
  or tests/unit/validator/{test_rules,test_validate}.py — those belong
  to T-D's already-merged polaris/validator/** row, outside this
  ticket's owned paths; a seam-request issue covers the one-line
  registration. check_displaced is fully unit-tested directly instead.

Also updates tests/unit/api/routers/plans_locks_assignments_stub_test.py
(PH1.7/issue #15) to move the diff path out of
STILL_STUB_PLANNER_READABLE_GETS, following the exact precedent that
file's own docstring already documents for T-E's earlier routers —
role-gating coverage is untouched, only the stale "still 501" assertion
moved.

Hand-built bad-plan fixtures (tests/fixtures/bad_plans/displaced.py)
cover dropped-without-reason, moved-without-reason, dropped-with-an-
explicit-lock-reason (not flagged) and unchanged (not flagged). Diff
matching is covered by unit tests against the pure function and against
FakePlanningRepository, plus a real-Postgres integration test for the
router.

Closes #37

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoNrSy7Reyp7evkfcdHeLX