Jens/MobilityOps
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

M3: implement Data Quality Workbench

Browse Source 
Five rule scanners (duplicate customers, missing fields, odometer regression, booking overlap, status conflict) run automatically after seed and via an explicit scan endpoint. Issue defer/reject/merge-customers endpoints with transactional customer merge (booking rewiring, tombstone, audit). Data Quality nav + workbench UI with two-column duplicate comparison and inline (non-native) confirm. Dashboard attention items now link to issues. 35 backend tests passing, ruff clean. Fixed a real false-positive bug in odometer-regression detection found through iteration on seed data, and two TS narrowing errors. Verified end-to-end via browser: S2 merge and S4 overlap scenarios.
This commit is contained in:
NuklearRabbit
2026-08-01 22:11:06 +02:00
parent 0091c57c7f
commit a7cbeaae3b
17 changed files with 1127 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
PROJECT_STATE.md
+15 -2
View File
@@ -2,7 +2,7 @@
## Current milestone
M2 — complete. Starting M3 next.
M3 — complete. Starting M4 next.
## Locked decisions
@@ -65,10 +65,23 @@ M2 — complete. Starting M3 next.
- `docker compose up -d --build` (all services) then `docker compose exec api python -m app.cli seed --reset`, then a full browser run of the S1 demo scenario against `BK-DEMO-RETURN`/`MO-024`: submitted 53000 km (below canonical 54820) → result panel showed `INSP-0076`, `resulting_vehicle_status: maintenance` (correctly derived, since canonical 54820 ≥ `next_service_km` 40000), `DQ-RET-0076` created, automation event queued with a real UUID, "no upcoming booking" risk; vehicle detail page confirmed odometer unchanged at 54,820 km and a "Needs attention" badge.
- Both real defects listed above (form disappearing before showing its result; `event_id` reading as `None`) were **found via the browser run, not by pytest** — the test suite asserted on API response shape/values, not on what the UI actually rendered after a status transition. Worth remembering for M3+: UI state-after-mutation bugs need a browser check, not just API tests.
### M3 — Data Quality Workbench
- `app/services/data_quality.py`: `run_scan()` implements all five rules and is called automatically at the end of `seed_loader.reset_and_seed()` (after `db.commit()` of the base seed), plus exposed as `POST /api/v1/data-quality/scan` (Operations Manager only). Idempotency is simplified from the doc's literal `(rule_type, entity_type, entity_id, evidence fingerprint)` to just `(rule_type, entity_type, entity_id)` while an issue is open — see rationale below.
- Router `app/api/routers/data_quality.py`: `GET /issues` (filters status/rule_type/severity), `GET /issues/{ref}` (adds `entity_snapshot`/`related_snapshots` for the UI), `POST /issues/{ref}/defer`, `/reject`, `/merge-customers` (Operations Manager only — enforced via `require_operations_manager`), `POST /scan`.
- **Real bug found and fixed during this milestone, before any browser check**: the first cut of DQ-03 (odometer regression) compared every historical *returned* booking's `end_odometer_km` against the vehicle's *current* `odometer_km`. Since the seed generator assigns `vehicle.odometer_km` independently of booking history (see `seed/generate_seed.py`), this is true for nearly every historical booking by construction (odometer is monotonically increasing over time, so all-but-the-latest reading is "below current") — it produced 51 false-positive issues out of 50 vehicles on first run. Fixed twice: first attempt (compare only the single most-recent booking against canonical) still produced the same problem because canonical itself is disconnected from booking history in this dataset; the working fix compares each vehicle's *own returned-booking sequence* against itself (each booking's end reading vs. the immediately preceding one, chronologically) — a self-consistency check that doesn't depend on the unrelated `vehicle.odometer_km` field at all. Final deterministic seed+scan totals: 15 CSV-seeded + 11 scan-discovered = **26** open/resolved `data_quality_issues` (breakdown: 14 vehicle_status_conflict, 5 missing_required_field, 3 possible_duplicate_customer, 3 odometer_regression, 1 booking_overlap). `tests/test_seed.py`'s exact-count assertion was updated from 15 to 26 accordingly — if the scan logic changes again, update that count.
- Idempotency simplification rationale: the doc's fingerprint-based key would make the scan blind to issues it structurally can't compute a matching fingerprint for against the CSV-seeded rows (which don't carry a fingerprint field), producing duplicate issues for the same real-world problem (e.g. a second `MO-016` overlap issue next to the seeded `DQ-DEMO-OVERLAP`). Using `(rule_type, entity_type, entity_id)` alone while open is a stricter, safe simplification: it can never falsely suppress an issue for a *different* entity, and per-entity there's realistically only one meaningful open issue of a given rule type at a time for this PoC's scope.
- Merge UI intentionally does **not** use `window.confirm()` — a native dialog blocks further automation/testing and isn't screen-reader-distinguishable from page content the same way a rendered `role="alertdialog"` panel is. Built an inline two-step confirm instead (`ReturnForm`-style pattern reused).
- `AttentionItem` gained an `issue_ref` field (dashboard now links attention items straight to `/data-quality/{issue_ref}` instead of only to vehicles); dashboard attention list capped at 8 items (was unbounded, would have shown up to 26 with the richer scan).
- Commands run and verified from this checkout:
- `docker compose run --rm api pytest -q` — **35 passed** (new `tests/test_data_quality.py`: all five rule types present, scan idempotent on rerun, scan requires Operations Manager, S2/S4 issue-detail snapshots correct, defer→reject-on-closed 409, merge requires Operations Manager, merge rejects an unrelated survivor ref, full S2 merge scenario asserting rewiring + audit + replay-is-409).
- `docker compose run --rm api ruff check .` — All checks passed.
- `npm run build` — clean (had to fix two `possibly 'null'` TS errors from a closure-narrowing limitation — TS doesn't narrow `const` captured-by-closure across nested function boundaries when the value comes from an index/property expression; fixed by re-binding to explicitly-typed local consts right after the guard).
- Full browser run: Data Quality list (26 open issues, filterable) → `DQ-DEMO-DUPLICATE` two-column compare (CUS-0012 vs CUS-0178, per-field diff highlighting only where they differ) → merge with inline confirm → issue flips to `resolved` → confirmed `customer_merged` audit event with correct actor/entity/correlation → `DQ-DEMO-OVERLAP` (non-duplicate type) renders evidence JSON + defer/reject, no dead compare UI shown for a rule type it doesn't apply to.
## Known blockers
None. External service credentials may be absent; use the documented demo/degraded providers.
## Exact next action
Start M3 (Data Quality Workbench): read `docs/07-data-quality.md`. Implement the five rule scanners (possible_duplicate_customer with weighted scoring, missing_required_field, odometer_regression, booking_overlap, vehicle_status_conflict) as an explicit scan service (idempotent by `(rule_type, entity_type, entity_id, evidence fingerprint)` while open, per the doc's lifecycle section), issue defer/reject/merge-customers endpoints, a transactional customer-merge (rewires bookings, tombstones the loser, audits before/after), the Data Quality nav item + Workbench UI (two-column duplicate comparison, evidence/proposed-resolution for other types), and wire "open quality issues" into the dashboard attention section it already reads from. Seed data already has 15 `data_quality_issues` including the named `DQ-DEMO-*` scenarios (S2 duplicate CUS-0012/CUS-0178, S4 overlap MO-016) to scan/resolve against — remember M2's return workflow also creates ad-hoc `odometer_regression` issues (`DQ-RET-*`) directly, so the scan service's idempotent-fingerprint rule must not double-report those.
Start M4 (n8n automation): read `docs/11-n8n-integration.md`, `n8n/README.md`. Implement the outbox dispatcher (poll pending `outbox_events`, claim with `FOR UPDATE SKIP LOCKED`, POST to n8n's webhook with timeout + exponential backoff + small max-attempt cap, never hold a DB transaction open during the HTTP call), correct/import `n8n/mobilityops-return-processing.json` (credential + the callback route — M2's `vehicle.returned.v1` outbox payload shape is already `contracts/events.schema.json`-compliant, so the workflow should be able to consume it as-is), a narrow MobilityOps callback endpoint the workflow calls back into, an Automation nav item + UI (pending/succeeded/failed list, manual retry button, matches `docs/06-ui-ux.md`'s "Recent automation" concept already on the dashboard), and manual-retry wiring for `POST /api/v1/workflows/{event_id}/retry` per `docs/05-api-contract.md`. The seeded `workflow_runs.csv` already includes one deterministic `failed` row (`event_id 00000000-...-0020`, "Synthetic connection timeout to n8n") for the S5 demo scenario — dashboard/Automation UI must surface it and allow retry. Remember the compose `n8n` service is already up (`http://localhost:5678`, basic auth `admin`/`change-me` from `.env.example`) but the workflow itself hasn't been imported/activated yet this session.