Final acceptance audit: fix all mypy defects, verify full journey matrix and degraded modes
Ran a dedicated post-M7 release-readiness audit. Found and fixed the one real gap: mypy was a declared dev dependency but had never been run in any milestone's validation loop. Fixed all 43 pre-existing type errors it surfaced, including two genuine defensive- programming gaps (unguarded Optional vehicle/customer lookups that could have crashed with unhandled 500s instead of clean 404/401 responses) rather than suppressing them. make lint now runs ruff + mypy; mypy reports zero errors across 44 source files. Re-verified end to end against a genuinely wiped-volumes clean checkout: automatic migrations, deterministic seed, 66/66 backend tests, and the full user-journey matrix (login, dashboard, vehicle/booking detail, return workflow, invalid-mileage rejection, data-quality review, duplicate-customer merge, audit trail, Knowledge Assistant, n8n, MCP Hub) via curl and Playwright. Live-verified both external-dependency degraded modes, not just unit tests: stopped n8n mid-flow and confirmed a return still commits with the outbox event staying pending and retrying with backoff, then self-healing to succeeded with zero manual intervention once n8n came back; verified RAGcore's unavailable-degradation path against an unreachable host. Added frontend/e2e/interactive-elements.spec.ts (11 tests covering every nav item, filter, tab, and role boundary) alongside the existing demo script test — 12/12 e2e tests passing. Verified no secrets are committed (.env never tracked, clean git history scan) and .env.example covers every operator-configurable setting. Confirmed no placeholders, TODOs, fake responses, hardcoded metrics, or dead routes anywhere in the codebase. Updated README.md with an honest integration-status section and PROJECT_STATE.md with the full audit findings. Added artifacts/final-acceptance/summary.md as the authoritative final evidence document (commands, results, URLs, demo access, integration status per external dependency, known limitations, deployment instructions, five-minute demo flow).
This commit is contained in:
+107
-8
@@ -2,7 +2,7 @@
|
||||
|
||||
## Current milestone
|
||||
|
||||
M7 — complete. All milestones (M0–M7) done. See `artifacts/evidence/final-summary.md` for the acceptance evidence.
|
||||
M7 — complete. All milestones (M0–M7) done, plus a full post-M7 final-acceptance audit (see below). See `artifacts/final-acceptance/summary.md` for the definitive acceptance evidence (supersedes `artifacts/evidence/final-summary.md`, which is kept as historical M7 evidence).
|
||||
|
||||
## Locked decisions
|
||||
|
||||
@@ -120,12 +120,6 @@ M7 — complete. All milestones (M0–M7) done. See `artifacts/evidence/final-su
|
||||
- `docker compose run --rm api ruff check .` — All checks passed.
|
||||
- Live `curl` verification against the running stack (no browser needed — these are service-to-service endpoints, not UI): missing header → 422; wrong token → 401; correct token → all four endpoints return correct data (`operations-summary` metrics match the dashboard; `attention-vehicles?minimum_severity=high` returned `MO-016`×2 and `MO-031`, all severity `high`; `vehicles/MO-016` returned the narrow read-only shape; `search-knowledge` with `max_sources=2` returned exactly 2 grounded sources for the S6 question). Confirmed via `GET /api/v1/audit?action=mcp_tool_request` that all four calls were recorded with correct `actor_type=service`, tool name, and status.
|
||||
|
||||
## Known blockers
|
||||
|
||||
None. External service credentials may be absent; use the documented demo/degraded providers. The n8n workflow-activation steps from M4 are a one-time manual setup requirement in this environment, not a blocker — but not yet scripted; M7 should either automate it (e.g. a bootstrap script CI/compose can run) or document it clearly enough for `docs/14-testing-and-acceptance.md`'s clean-checkout criteria. RAGcore and the ITWorx MCP Hub itself were never reachable this session — `RAGcoreKnowledgeProvider` and the M6 MCP provider endpoints are implemented and directly tested/curl-verified against MobilityOps's own API, but neither a real RAGcore instance nor a real Hub round trip was available to confirm end-to-end; M5's demo provider and M6's direct endpoint tests are what actually satisfy those milestones' acceptance criteria in this environment.
|
||||
|
||||
## Exact next action
|
||||
|
||||
### M7 — Portfolio polish and final acceptance
|
||||
- **Automated clean-checkout migrations**: `backend/entrypoint.sh` now runs `alembic upgrade head` before starting uvicorn (Dockerfile `CMD` changed from `uvicorn ...` to `./entrypoint.sh`). Verified with a true `docker compose down -v` (all volumes wiped) → `docker compose up --build -d` → all 11 tables present, `/health` and web both green, all 66 backend tests pass, with zero manual migration step.
|
||||
- **n8n one-time setup scripted where it can be**: `make n8n-setup` runs the import/publish/restart sequence (previously three manual commands discovered ad hoc in M4). The owner-account creation itself cannot be scripted safely (it's an interactive one-time step in n8n 2.x's own onboarding, not a MobilityOps concern) — documented precisely in the rewritten `docs/17-runbook.md`, including the exact URL and that no email verification is required. Re-ran this full sequence from the wiped-volumes state this session and confirmed the S1 return → outbox → live n8n → callback → `succeeded` round trip works on a genuinely clean checkout, not just the already-provisioned stack from M0–M6.
|
||||
@@ -140,6 +134,111 @@ None. External service credentials may be absent; use the documented demo/degrad
|
||||
- `cd frontend && npx playwright test` — **1 passed** (full demo script, live stack).
|
||||
- Full clean-checkout drill: `docker compose down -v` → `docker compose up --build -d` → `docker compose exec api python -m app.cli seed --reset` → `docker compose run --rm api pytest -q` (66 passed) → n8n owner setup + `make n8n-setup` → live S1 return round-tripped through the real n8n instance to `succeeded`.
|
||||
|
||||
### Final acceptance audit (post-M7)
|
||||
|
||||
A dedicated release-readiness audit was run after M7 claimed completion, specifically to
|
||||
catch anything the milestone-by-milestone build might have missed by only ever validating
|
||||
each piece in isolation.
|
||||
|
||||
- **Real gap found: `mypy` had never been run.** `mypy` is a declared dev dependency
|
||||
(`backend/pyproject.toml`) but was never wired into any milestone's validation loop —
|
||||
only `ruff` was. Running it cold surfaced **43 real type errors across 10 files**, all
|
||||
pre-existing (not introduced by this audit). Triaged and fixed all of them rather than
|
||||
suppressing:
|
||||
- `services/returns.py`: the vehicle lookup after acquiring `FOR UPDATE` could type as
|
||||
`Vehicle | None` with no runtime guard — added an explicit `if vehicle is None: raise
|
||||
AppError(..., 404)`. This was a genuine defensive-programming gap (a dangling FK would
|
||||
have crashed with an unhandled `AttributeError`/500 instead of a clean 404), not just
|
||||
a type annotation issue.
|
||||
- `api/routers/bookings.py`: same pattern for `db.get(Customer, ...)` /
|
||||
`db.get(Vehicle, ...)` in `get_booking` — added a guard raising 500 with a clear
|
||||
message instead of crashing on `None.public_ref`.
|
||||
- `api/deps.py` + `api/routers/demo.py`: `CurrentUser.role` is a `Literal[...]`, but
|
||||
`SessionPayload.role` (decoded from an HMAC-signed cookie) and `User.role` (a DB
|
||||
column) are both plain `str`. Pydantic validates this at runtime already (so it was
|
||||
never exploitable), but `get_current_user` now explicitly checks membership before
|
||||
constructing `CurrentUser`, turning a would-be unhandled `ValidationError` (500) into
|
||||
a clean 401 for a corrupted/tampered cookie — another real defensive improvement, not
|
||||
just a type-checker appeasement.
|
||||
- `api/routers/dashboard.py`, `api/routers/data_quality.py`: two instances of reusing
|
||||
one variable name for both a `Vehicle` and a `Customer` across an if/else branch,
|
||||
which is genuinely confusing to read regardless of what mypy thinks — renamed to
|
||||
distinct variables (`entity`/typed union in dashboard, `customer`/`vehicle` in the
|
||||
data-quality snapshot helper).
|
||||
- `services/data_quality.py`, `seed_loader.py`: `Booking.__table__.update()` /
|
||||
`Customer.__table__.update()` don't typecheck against SQLAlchemy 2.0's stubs (the
|
||||
`.__table__` accessor is typed as the more general `FromClause`, which doesn't
|
||||
declare `.update()`) — switched to the idiomatic `sqlalchemy.update(Model)` construct,
|
||||
which is both correctly typed and the more modern SQLAlchemy 2.0 style anyway.
|
||||
- Remaining handful (schemas.py's deprecated `conint()` → `Annotated[int, Field(...)]`,
|
||||
a `Sequence` vs `list` `.sort()` call, an `assert`-guarded None-narrowing after a
|
||||
`WHERE ... IS NOT NULL` filter mypy can't see through, `Result.rowcount` typing gaps)
|
||||
were either latent pydantic-v1-style API usage or genuine SQLAlchemy stub limitations
|
||||
— fixed with the idiomatic modern equivalent or a narrowly-scoped, commented
|
||||
`# type: ignore[...]` at the exact line, never a blanket suppression.
|
||||
- `make lint` now runs both `ruff check .` and `mypy app`; `mypy app` reports
|
||||
**zero errors across 44 source files**.
|
||||
- **No other defects found.** Re-ran the full journey matrix end-to-end against a
|
||||
genuinely wiped-volumes (`docker compose down -v`) clean checkout: all 66 backend
|
||||
tests, ruff, ✅; ran the demo login → dashboard → vehicle/booking detail → return
|
||||
workflow → invalid-mileage rejection (422, both a negative value and a non-numeric
|
||||
string) → data-quality issue review → duplicate-customer merge → audit trail →
|
||||
Knowledge Assistant → live n8n round trip → MCP Hub endpoint journeys directly via
|
||||
`curl` against the running stack, all correct.
|
||||
- **Degraded-mode behavior explicitly re-verified live** (not just unit-tested):
|
||||
stopped n8n with `docker compose stop n8n`, registered a return — it committed
|
||||
(`201`, booking flipped to `returned`) exactly as required; the outbox event stayed
|
||||
`pending` with real `ConnectError`s logged and exponential backoff (2 attempts over
|
||||
~8s); restarted n8n and the dispatcher **self-healed** without any manual
|
||||
intervention, delivering the event to `succeeded` on attempt 5. RAGcore unavailable
|
||||
mode re-verified live against an unreachable host (`ConnectError` → `evidence_state:
|
||||
"unavailable"`, empty answer, no fabrication). MCP Hub unavailability is
|
||||
architecturally moot for MobilityOps — the Hub only ever calls *into* MobilityOps, so
|
||||
there is nothing on the MobilityOps side that can degrade if the Hub is down (only the
|
||||
reverse, "does an unavailable Hub break MobilityOps," which is trivially no since
|
||||
nothing here calls out to it).
|
||||
- **New test coverage added, no existing tests weakened**: `frontend/e2e/interactive-elements.spec.ts`
|
||||
(11 Playwright tests — all seven nav items, every filter on every list page, vehicle
|
||||
detail tabs, defer/reject, automation retry, knowledge form, role-switching, and
|
||||
role-based page restriction) plus the existing `demo.spec.ts` — **12/12 e2e tests
|
||||
passing** against the live stack.
|
||||
- Verified `.env` is `.gitignore`d and was never committed (`git ls-files` /
|
||||
`git log --all -p -- '*.env'` both empty); scanned full git history for AWS keys,
|
||||
private-key headers, and `sk-...`-style tokens — none found. Every `Settings` field in
|
||||
`backend/app/core/config.py` has a corresponding entry either directly in
|
||||
`.env.example` or is derived/wired through `compose.yaml` (a few purely-internal
|
||||
container-path constants like `SEED_DIR`/`KNOWLEDGE_DIR` are intentionally not
|
||||
operator-configurable and correctly absent from `.env.example`).
|
||||
- Grepped the full `frontend/src` and `backend/app` trees for TODO/FIXME/placeholder/
|
||||
fake/stub/mock/"not implemented" markers — zero real hits (the two `placeholder=`
|
||||
matches are legitimate HTML input placeholder attributes). Confirmed dashboard metrics
|
||||
and all list-page data are 100% DB-backed (`compute_metrics` in
|
||||
`services/operations.py`, never a literal in frontend JSX). Confirmed every frontend
|
||||
route in `App.tsx` maps to an implemented page and every nav item maps to a real route
|
||||
— no dead routes.
|
||||
- Commands run and verified from this audit:
|
||||
- `docker compose run --rm api pytest -q` — **66 passed**.
|
||||
- `docker compose run --rm api ruff check .` — All checks passed.
|
||||
- `docker compose run --rm api mypy app` — **Success: no issues found in 44 source files** (0 errors, down from 43).
|
||||
- `cd frontend && npm run build` — clean (`tsc -b && vite build`).
|
||||
- `cd frontend && npx playwright test` — **12 passed** (`demo.spec.ts` + `interactive-elements.spec.ts`).
|
||||
- Full clean-checkout drill repeated from a fresh `docker compose down -v`: automatic migrations, seed, 66/66 tests, n8n owner setup + `make n8n-setup`, live return round-tripped through n8n to `succeeded`.
|
||||
- See `artifacts/final-acceptance/summary.md` for the complete evidence write-up (commands, exact outputs, demo access, deployment instructions, five-minute demo flow).
|
||||
|
||||
## Definition of done
|
||||
|
||||
All eight milestones (M0–M7) are complete. `docs/14-testing-and-acceptance.md`'s clean-checkout acceptance list has been walked item by item against a genuinely wiped-volumes checkout this session (see M7 notes above), and `artifacts/evidence/final-summary.md` contains the final evidence summary this file's own instructions (`CLAUDE.md`) require. The two items not fully closed — a live RAGcore instance and a live ITWorx MCP Hub instance — were never reachable in this environment; both integrations are implemented, unit/contract-tested, and documented as such rather than claimed as verified.
|
||||
All eight milestones (M0–M7) are complete, and a dedicated post-M7 final-acceptance audit
|
||||
found and fixed one real category of gap (`mypy` never having been run) with zero
|
||||
regressions. `docs/14-testing-and-acceptance.md`'s clean-checkout acceptance list has been
|
||||
walked item by item against a genuinely wiped-volumes checkout, twice (once in M7, once in
|
||||
this audit), and `artifacts/final-acceptance/summary.md` is the authoritative final
|
||||
evidence document. The two items not fully closed — a live RAGcore instance and a live
|
||||
ITWorx MCP Hub instance — were never reachable in this environment; both integrations are
|
||||
implemented, unit/contract-tested, directly verified against MobilityOps's own API, and
|
||||
their unavailable-degradation paths are live-verified, but an actual round trip against
|
||||
real RAGcore/Hub instances remains unconfirmed and is documented as such rather than
|
||||
claimed.
|
||||
|
||||
## Known blockers
|
||||
|
||||
None. External service credentials may be absent; use the documented demo/degraded providers. The n8n workflow-activation steps are a one-time manual setup requirement in this environment (owner-account creation via n8n's own `/setup` UI cannot be scripted safely), fully documented in `docs/17-runbook.md` and scripted where possible (`make n8n-setup`). RAGcore and the ITWorx MCP Hub itself were never reachable in this environment — both integrations are implemented and directly tested/curl-verified against MobilityOps's own API, but neither a real RAGcore instance nor a real Hub round trip was available to confirm end-to-end.
|
||||
|
||||
Reference in New Issue
Block a user