feat: v3.3.0 tire mount periods, tire analytics, and movement-based drive sessions - #159
Merged
Conversation
The upgrade note added for the LiveLink odometer fix could not be followed
by anyone, on any instance.
- `backend/tools/` was built and then discarded by the runtime stage, so
every documented command failed with `can't open file`. Verified against
the running container: `/app/tools` did not exist.
- Three of the four tools hardcoded `create_engine(f"sqlite:///{args.db}")`.
On PostgreSQL that creates an empty SQLite file and then fails with
`no such table`, so PostgreSQL instances had no repair path at all.
`--db` now takes a path (the published form), a URL, or nothing, and the
async-to-sync conversion is extracted from `init_db` rather than written
a second time. All tool SQL verified against PostgreSQL 17, including a
full read/write run of the normalize tool.
- The note said to run the tools "before the upgraded instance records new
readings", which was impossible: migrations run inside the app's lifespan
and the MQTT toggle is a database row. `MYGARAGE_MAINTENANCE_MODE=1` now
starts the instance for migrations only, with ingest answering 503 and
neither the scheduler nor the MQTT subscriber running.
- A dry run of `normalize_telemetry_odometer_units.py` that refused a
mixed-unit device returned 0, so a script gating `--apply` on it would
apply what the dry run had just refused. Its sibling already returned 2.
The upgrade note is rewritten around a backup taken with the SQLite backup
API, and now states plainly that this release cannot be downgraded and that
the repair only reaches as far back as telemetry retention allows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ostgreSQL Two defects in 96313be, found by adversarial review of the upgrade procedure. backfill_livelink_odometer.py died on the first telemetry row on PostgreSQL. It called date.fromisoformat() on three DATE columns; psycopg2 adapts DATE to datetime.date, and fromisoformat rejects a date with TypeError. This was unreachable before 96313be, because the tool hardcoded sqlite:///{path} and could never open a PostgreSQL database at all. Making it reach PostgreSQL moved the failure from "connects to the wrong database" to "cannot read", so the documented repair was still unexecutable there. Adds tools._tool_db.as_date. Maintenance mode did not stop telemetry. It closed two prefixes, chosen by looking at the ingest routers, and missed POST /api/livelink/devices/{id}/ backfill, which lives under the admin prefix and reaches bulk_backfill. A manual call during the repair window recreates the mixed-unit state the tools refuse to run against. is_maintenance_closed() now matches that route exactly, leaving the rest of the admin API open so the operator can watch the repair. The new dialect test lives in tests/integration/ because ci.yml:25 runs only tests/migrations/ and tests/integration/ against the PostgreSQL sidecar. In tests/unit/tools/ it would pass with the bug fully present, forever, since SQLite returns every DATE as a string. Verified by mutation: reverting the fix fails both tests on PostgreSQL with the predicted TypeError and passes both on SQLite. The maintenance guard derives its route list from the code by AST rather than from a hand-written list, so a new ingest route fails it without anyone remembering to update it. Resolution is transitive within the module: the first version looked only at direct calls and missed the torque route, whose writer call sits in a helper. Verified: bin/ci-check --backend green (3960 passed); PostgreSQL tests/migrations/ + tests/integration/ green (1636 passed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he writer set Two gaps found while independently verifying the last commit's own claims. Nothing ran the upgrade procedure the CHANGELOG prescribes. The previous commit's tests covered URL resolution, exit codes, and one tool's date handling; none ran all four tools in the documented order against seeded data. test_upgrade_procedure.py does, as subprocesses with no --db, which is the real `docker exec` invocation: it exercises sys.path.insert, argparse, the settings.database_url fallback, and the driver's actual return types. Verified by mutation on PostgreSQL: reverting as_date fails it with a traceback. Its first test asserts the seeded rows are visible to a separate connection. Without that, an uncommitted fixture would leave every tool reading an empty table, exiting 0, and proving nothing while looking green. TELEMETRY_WRITERS was itself an unchecked inventory. The route scan assumes it names every function that can write a telemetry row, and nothing verified that. TestTelemetryWriterSet now derives the builders from the AST and requires each to be a declared writer or provably unreachable. It found store_value (telemetry_service.py:1052), which builds VehicleTelemetry and has zero callers anywhere in app/ -- harmless today, and listed in KNOWN_UNREACHABLE rather than silently skipped, so giving it a caller fails the test. KNOWN_UNREACHABLE is an escape hatch, so it has its own guard asserting the listed names really have no callers. Otherwise listing a function there would be a way to silence the check rather than a statement of fact. Both new guards mutation-verified: dropping store_telemetry from the set fails 2 tests; claiming bulk_backfill is unreachable fails 1. Verified: bin/ci-check --backend green (3968 passed); PostgreSQL tests/migrations/ + tests/integration/ green (1641 passed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…utes Third round of review found a third open path into the repair window. The gate started as two URL prefixes; review found the admin SD backfill route; that was fixed by matching it exactly; review then found POST /api/livelink/mqtt/restart, which writes no telemetry itself but starts the MQTT subscriber, which does. Two rounds of the same defect is enough to say the shape out loud: any gate that enumerates entry points is a floor. MQTT is not a route. The scheduler is not a route. The AST route-inventory test added last commit cannot see a cross-module background path, and no route-level check can. So the gate moves to the choke point every path passes through. store_telemetry, store_torque_telemetry and bulk_backfill each raise MaintenanceModeError while maintenance mode is on, and start_mqtt_subscriber refuses, which closes the restart route without naming it. The route gate stays: a 503 at the edge is a better answer than a 500 from the middle, but it is no longer what is being relied on. One test deliberately calls bulk_backfill with a non-empty row list, because the natural implementation short-circuits on empty input and would make the empty-list test pass for the wrong reason while leaving a real backfill open. Mutation-verified: no-oping the writer guard fails 4 tests; removing the MQTT guard fails 2. A test asserting normal operation is unaffected keeps an always-raise mutant from passing. Verified: bin/ci-check --backend green (3975 passed); PostgreSQL tests/migrations/ + tests/integration/ green (1641 passed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Spec D, items 1-3. Three record types were broken on both sides of the round trip, and the third was found only by enumerating mechanically. Export read attributes the models do not have: WarrantyRecord.coverage (it is coverage_details) plus cost/deductible/max_claims/terms, which do not exist at all, and InsurancePolicy.premium (it is premium_amount). Any vehicle with such a record returned 500. Import constructed with the same nonexistent kwargs, and TaxRecord with four more (year, paid_date, due_date, jurisdiction). The TypeError was caught per row and reported as "Invalid record data", so the endpoint returned 200 while blaming the user's file for an application bug. No tax record has ever imported successfully. Tax EXPORT was fine, which is why two earlier passes that enumerated from the export bug never found it; a script matching every Model(...) call against its mapper did, 3 broken of 20. The tax importer also read Year/Paid Date/Due Date/Jurisdiction while the exporter wrote Date/Renewal Date, so the file it produced could not be read back even before the constructor raised. The two halves now share one vocabulary, and the old headers are still accepted so a pre-fix file imports. mileage_limit_km is exported for the first time. It is unit-bearing, so it goes through build_csv/EMITTED_COLUMNS rather than the plain path, and the importer reads it through _read_csv_with_units: a bare DictReader cannot see a tokenised header and would have stored miles as km. test_csv_emission's existing guard requires every emitted unit-bearing column to have a matching importer spec, which is the round-trip property stated as a test. Fixing the kwargs exposed the real error path underneath: db.commit() is outside the per-row try, so a CHECK violation escaped as a 500 and discarded every valid row in the file. Rows now write inside a savepoint, so one bad warranty type is one reported row. Folded in rather than deferred because leaving it would ship an importer that 500s on a typo, which is worse than the bug being fixed. The two shipped tests asserting pytest.raises(AttributeError) on these exports are inverted, not deleted: they passed BECAUSE the export was broken. Noted in their docstring so the change does not read as removing failing tests. Reminder notifications (item 3): last_notified_at is a naive column and was assigned an aware datetime. SQLite accepts it silently; PostgreSQL raises asyncpg DataError, so no PostgreSQL instance has ever delivered a reminder notification. The read path was already guarded, the write was not. The test lives in tests/integration/ because ci.yml:25 runs only that path against the PG sidecar; in tests/unit/ it passes with the bug present. Verified: it fails on PostgreSQL with the predicted DataError before the fix. Verified: bin/ci-check green (3991 backend, 2098 frontend); PostgreSQL migrations + integration green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…disabling them Spec D item 4. `<form onSubmit>` had no noValidate while the inputs inside it carried required, min and step. A constraint failure makes the browser abort submit and try to focus the offending control; in a collapsed or scrolled-away line-item section it cannot, so nothing is shown and Save appears to do nothing. The constraints span three files, not one: ServiceVisitForm renders LineItemEditor, which renders SupplyUsedPicker, all inside the same form. The per-line-item fields are exactly the collapsed case, so they are the most likely to fail invisibly and were the ones two earlier inventories excluded. noValidate ALONE would be worse than the bug. `required` on the visit date (:553) was the only thing stopping a blank date from reaching the API, and handleSubmit checked only line-item descriptions and inspection results, so disabling native validation would turn a silent no-op into a silent bad write. Every removed constraint has an equivalent in validateFields/validateLineItems, rendering inline through the existing fieldErrors path. Two of those equivalents had to be written in the right unit space, which the units manifest is what caught: - The reminder interval's min="1" sat on the DISPLAY value while due_mileage_km is canonical km, so comparing canonical km against a bare 1 would have loosened an imperial account's floor from 1 mi to 1 km. The threshold is converted with u.distance.toCanonical(1). - SupplyUsedPicker's step was '1' for count-type supplies, so a count could not take a fraction. The backend enforces only gt=0 (schemas/supply.py:75), so dropping the check rather than moving it would have allowed "2.5 oil filters". The parent already holds the supplies index, so it checks there. The new test enumerates by ATTRIBUTE over every control in the rendered form rather than by a list of fields, and asserts the set is empty. A bare JSX boolean has no `=`, which is how `required` survived two hand-written inventories of this same form. Each validation test asserts both a visible error and that no request was sent; asserting only the error would pass against a form that complains and posts anyway. The guard-the-guard test asserts a valid form is NOT rejected by the new field-level checks, so a validator that rejects everything cannot pass. Verified: bin/ci-check green (3991 backend, 2098 frontend); units manifest re-stamped after re-review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Spec A, first three pieces: migration 097, the models, and the service
contracts with their API.
THE DEFECT. `_project_wear` computed `newer.odometer_km - older.odometer_km`
and treated it as distance driven ON THAT TIRE. For anyone running a second
seasonal set that counts the distance driven on the OTHER set, and the reported
remaining life came out at 648,000 km against a 2.0 mm threshold. Erring high,
on a tire. Distance is now summed over the tire's own mount periods, and where
the history cannot support a figure it is withheld rather than published with
an "estimate" badge -- a label does not communicate that a number is
structurally invalid rather than imprecise.
A second, quieter one: `_sync_low_tread_reminder` passed readings UNSORTED, so
`newer` was the oldest, the tread delta came out negative, and the projection
has been silently missing from every low-tread reminder ever raised. Selection
moved inside the function so both surfaces quote the same number.
MIGRATION 097 runs steps 1-7 in ONE transaction, because the runner executes
and stamps separately (runner.py:226) and a crash between non-transactional
steps would leave a schema matching neither re-entrancy branch. The
discriminator is vehicle_reminders.source, the LAST thing it writes: keying on
tire_mount_periods would be wrong because create_all makes that table before
migrations run, and keying on tires.installed_date would be wrong because a
fresh database never had it.
PRAGMA foreign_keys = OFF is a no-op inside a transaction and SQLite reports no
error -- the read still returns 1. test_the_cascade_hazard_is_real demonstrates
that DROP TABLE tires then empties tire_readings, silently. 097 follows
migration 070: raw DB-API connection, pragma outside any transaction, read
back and assert. Do not copy 092, which sets it and never checks.
Verified against a backup-API copy of the production database: 2 periods
created, 2 readings and 10 reminders preserved, 3/3 CHECKs installed.
The PostgreSQL branch had never executed under any test -- on PG the suite's
own database comes from create_all, so 097 sees its discriminator and returns.
It now has three tests that build a legacy PG schema and run it.
TYPED RESULTS replace `Decimal | None`. Six DistanceStatus members and eight
WearStatus, both exhaustive, with a test asserting every member is reachable so
a caller can be written against it. WearResult carries km_remaining AND
wear_date: a single-value type would have silently deleted projected_wear_date
from the API, which the tire card renders. `nothing_bounded` exists because the
migrated shape (an assumed period with a null start odometer) would otherwise
report "0 km since an unknown date" -- the state of every tire on upgrade day.
BREAKING: POST /api/vehicles/{vin}/tires no longer accepts `position` and no
longer upserts by it. A tire is a thing you own; mounting is a separate
operation with its own conflict semantics. TireBase sets extra="forbid" so a
stale client gets a 422 naming the field rather than silently creating a
second, unmounted tire. New: create-and-mount, mount, dismount.
Two things the tests found that the design did not:
- `tires` has ONE constraint and no CHECKs. The spec said "reproduce all three
CHECKs", carrying the count over from vehicle_reminders where they live.
- The ORM declared NONE of the three reminder CHECKs, so create_all databases
had zero while migrated ones had three. Declaring them exposed three test
fixtures using status="completed", a value production has always rejected
and nothing in the app writes.
test_tires.py now takes a vehicle per test, with teardown. Positions are
claimable once per vehicle now, so tests collided on the shared VIN -- and
without teardown the extra vehicles pushed a paginated assertion in
test_vehicle.py off its page.
Verified: bin/ci-check --backend green (4023 passed); PostgreSQL
tests/migrations/ + tests/integration/ green (1672 passed).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Spec A, D18. Replacing a worn tire was a hard DELETE that cascaded through every reading and every mount period. Shipping the mount-period model beside an unchanged delete would mean the first thing a user does after collecting a season of data is erase it -- and this release is the one that makes that history worth keeping. Retire closes the open period, frees the corner so the replacement can go where the old one was, sets retired_on, and keeps everything. DELETE remains, for a tire entered by mistake, and now says in its docstring what it destroys. A retired tire leaves the default listing (it is history, not inventory, and nothing more can be recorded about it) and is available via include_retired. Analytics will still count it: its final distance and wear are the most complete data the app will ever have about that tire. Delete now detaches its reminders first, in the same transaction. The composite FK (tire_id, vin) -> tires(id, vin) carries no ON DELETE action on purpose: a referential action applies to every column in the FK, so SET NULL would try to null `vin` too, and vehicle_reminders.vin is NOT NULL. Verified by mutation -- removing the detach makes deleting a tire with a reminder fail with "FOREIGN KEY constraint failed", which is how SET NULL would have made retiring a worn tire impossible rather than merely lossy. The low-tread sync now stamps tire_id, source='low_tread', and the tread and projection it saw. Without those the delete test above passed vacuously (every reminder already had a null tire_id), and the sync could adopt a reminder a human wrote with the same title. Verified: bin/ci-check --backend green (4032 passed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Spec A, D16. `uq_tires_vin_position` is an IMMEDIATE unique index on both dialects, so assigning one tire at a time fails the moment a destination is still occupied -- which for a rotation is always. An X-pattern swap collides on the very first move even though the requested FINAL arrangement is legal, and SQLite has no DEFERRABLE INITIALLY DEFERRED to fall back on. So the write is split: clear every affected position and close every affected period, FLUSH, then assign the new positions and open the new periods. Verified by mutation -- removing the vacate phase fails with the predicted "UNIQUE constraint failed: tires.vin, tires.position". All or nothing. A destination held by a tire not in the rotation is a 409, a missing tire is a 404, and neither writes anything; a rotation that applied its valid moves and rejected the rest would leave the vehicle in an arrangement nobody asked for, which for something done four tires at a time is worse than a refusal. There is a test asserting nothing moves on a refusal, because the conflict tests alone would pass against a service that corrupted the vehicle. Duplicate destinations are caught in the schema rather than by the index: the index fires mid-write and its IntegrityError cannot say which pair of moves conflicted. A rotation is a dismount and a remount, so each corner's distance stays attributable -- the closing period takes the rotation odometer as its upper bound and the opening one takes it as its lower. Verified: bin/ci-check --backend green (4037 passed); PostgreSQL tests/migrations/ + tests/integration/ green (1686 passed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Spec A's UI. TireList was built on "position is a tire's whole identity" -- its own comment said so -- which is exactly the assumption this release removes, so this is a rewrite of that premise rather than a patch. Regenerating the OpenAPI types found every break: `position` is nullable now, and `positionLabels[tire.position]` no longer type-checks anywhere. Rather than casting past it, `MountedPosition = NonNullable<TirePosition>` keeps the two ideas apart -- a tire's CURRENT position, which may be null, versus a corner being named, which may not -- and one `labelFor()` helper gives a stored tire the wording it needs in list rows, drawer titles and aria-labels. Mounted and stored tires are separate sections, not one list sorted by position: a stored tire mixed into the corner list reads as a corner whose label failed to render. The storage heading only appears when there is something in it, so a single-set owner sees what they saw before. Save is split. Create-and-mount for a new tire, PUT for an existing one -- the same payload no longer covers both, because a POST to an occupied corner is a 409 and `position` is not a writable field at all. distanceSummary() renders all six DistanceStatus members with their own wording. `nothing_bounded` matters most: it is the state of EVERY tire immediately after upgrading, since migration 097 gives each existing tire an assumed period with an unknown start odometer. Rendering it as "0 km" would tell every user their tires had never been driven on. Mount and dismount are drawers rather than confirm dialogs because both carry an odometer reading, and that reading is what makes the tire's distance computable at all. A confirm with no field would produce an unbounded period that reports "not recorded yet" forever -- the dead end this release exists to get out of. The units manifest earned its place again: re-reviewing before re-stamping found the two drawers sharing ONE odometer state, so a value typed into Mount and abandoned reappeared in Dismount and silently became that period's closing bound -- the number a tire's whole distance is computed from. Split, and pinned by a test verified by mutation. Verified: bin/ci-check green (4037 backend, 2101 frontend); units manifest re-stamped after re-review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tire flows had no end-to-end coverage at all, which is how POST /tires could stop accepting a `position` and only one incidental settings test noticed. That test failed on the first run of this branch, with exactly the 422 the breaking change is designed to produce -- the change working, and the seed needing to move to create-and-mount. Three specs, chosen for what only a browser run can prove: - create-and-mount then dismount through the drawer, asserting the tire is still listed afterwards under "In storage". A dismounted tire rendering as a blank corner, or vanishing entirely, is the failure the component tests cannot see because they mock the hooks. - The upgrade-day shape: a tire with no odometer bounds. Asserts distance_status on the wire AND that no "0 km" appears anywhere on the page. Every tire on every instance looks like this the moment migration 097 runs, so a zero here would be the most visible wrong number in the release. - A stale client POSTing a position gets a 422 naming the field. The breaking change as an executable contract rather than a changelog sentence: this is the request a browser tab left open across the upgrade will send. The seed tolerates 409 because create-and-mount is not an upsert -- a re-run against a surviving database finds the corner taken, which is correct behaviour rather than a broken fixture. Verified: 64 e2e passed (was 60 passed / 1 failed on this branch before the seed fix). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… look at
Found by running the branch against a backup-API copy of the production
database. The mounted tire card -- the one every user sees -- rendered
"Wear Estimate —" and no distance row at all. The whole point of the release
was invisible on real data, replaced by exactly the uninformative dash the
typed statuses exist to eliminate.
Both cards now carry a distance row, and wearSummary() renders all eight
WearStatus members with their own wording instead of collapsing seven of them
into "—". On the maintainer's own two tires that turns
Wear Estimate —
into
Wear Estimate Needs a second tread reading
Distance on tire Not yet known: add an odometer to this tire's mount
which reads as a next step rather than as a broken feature.
An unknown or absent wear_status falls back to the number when one is present:
a response cached by a pre-v3.3.0 client should not have a figure the server
sent hidden from it. An unknown status is a reason to stop explaining, not a
reason to withhold.
The e2e assertion took three attempts to become real, which is worth recording:
1. Asserting no "0 km" appears passed while the card had no distance row at
all -- "no zero" is also true of "nothing rendered".
2. Adding getByText('Distance on tire').first() ALSO passed under mutation:
the previous test in the file leaves a dismounted tire on the page whose
storage card carries the same label, so .first() matched a card the test is
not about.
3. Scoping to the card containing this test's own tire finally fails when the
row is deleted. Verified by mutation both ways.
Two em-dashes removed from user-facing copy.
Verified: bin/ci-check green (4037 backend, 2101 frontend); 64 e2e passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…un-safe Rotation and retirement -- the two most complex operations in spec A -- had no end-to-end coverage. Both now do, and both were mutation-verified: collapsing the rotation to a single phase fails with the unique-index violation, and making retire keep the position fails on the freed corner. The rotation test asserts `incomplete`, not `complete`, and that is the code being right rather than the test being wrong. The closed period is bounded but the new one is OPEN, and an open period's upper bound is the vehicle's latest OdometerRecord -- which a rotation's own `odometer_km` does not create. So a user who rotates and supplies an odometer still sees "incomplete" until they log an odometer reading separately. Written down in the test because it is the clearest statement of a real product gap. The rest of this commit is making these specs honest, which took several passes: - `test.skip` on a 409 meant the retire test proved nothing at all. Corners are claimable once now, so position availability became an ordering dependency between tests that did not exist before. - Sharing the seeded TEST_VEHICLE does not work for the same reason. Each group takes its own rig, keyed by label, because five positions cannot cover a rotation test claiming four and an unbounded test holding SPARE. - The first rig reused TEST_VEHICLE's make/model/year, which put a second identical card on the dashboard and broke two vehicle.spec.ts assertions with strict-mode violations. Distinctive make/model now. - And the rigs are deleted in afterAll. Without that they accumulate on the dashboard and break vehicle.spec.ts's single-"View Details" assertion -- a spec this file does not touch, failing on data this file created. The backend suite hit the identical problem with a paginated listing. - A tolerance list of [201, 400, 409, 422] on the vehicle seed hid a rejected VIN (I/O/Q are invalid) and surfaced it four requests later as a confusing 404. Narrowed to [201, 409]. Verified: two consecutive full e2e runs green against a reused server (66 passed), which is what rerun-safety actually means here; bin/ci-check green (4037 backend, 2101 frontend). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every tire write that takes an odometer stored it on the mount period and
nowhere else. `distance_on_tire` bounds an OPEN period with the vehicle's
latest OdometerRecord, so rotating four tires and dutifully entering the
odometer left the distance `incomplete` until the same number was logged a
second time somewhere else. The previous commit wrote that gap into an e2e
assertion; this closes it and the assertion now reads `complete`.
All six writers publish through `sync_odometer_from_record`, the helper fuel
and service visits have used since v2.26.2, composed into the caller's
transaction. `commit=False` is fixed inside `_publish_odometer` rather than
passed at each call site, because a commit in the middle of a mount, a
rotation or a retire splits the operation in half.
The six were enumerated from the request schemas, not from memory: the five
that carry an odometer field are TireMountRequest, TireDismountRequest (which
serves BOTH dismount and retire), TireCreateAndMountRequest,
TireRotationRequest and TireReadingCreate.
Readings are in deliberately. Without them the other five make things WORSE
for someone who only records readings: mounting gives the open period an
upper bound equal to its own start, and the card reports a confident "0 km"
instead of admitting it does not know.
The undo half: deleting a tire removes the readings it published, matched on
the exact marker so a manual row is never touched and a row a later sync took
ownership of stays with that source. Nothing cascades these rows on their own
- odometer_records carries an FK for fuel-sourced rows only - so a tire
entered with a typo'd odometer and then deleted would otherwise leave the typo
behind as the vehicle's latest reading, where it poisons every mileage
reminder. A ROTATION's reading is marked separately and deliberately survives:
it is a reading of the vehicle taken while several tires were on it, and
cascading it would break the distance figure for every other tire that moved.
Two things found on the way:
- Three of the six recomputed `utc_now().date()` twice, so a mount, dismount
or retire running across midnight could stamp the period and the retirement
on different days. Hoisted to one local each.
- The marker string was built in odometer_sync and matched by hand in the
cleanup paths, one edit apart from silently disagreeing. A mismatch does not
fail loudly, it orphans the row. Extracted as `auto_sync_marker`.
`OdometerSource` is corrected as part of the same vocabulary change. It listed
three values, invented one the backend never writes ('import' - the CSV
importer creates the row without a source, so those are 'manual') and omitted
five it does. Re-derived by enumerating the writers, which have three
syntaxes: the column default, a direct OdometerRecord(source=...), and
sync_odometer_from_record whose source_type is keyword at some call sites and
positional at others. Grepping only the keyword form is how the old list went
wrong.
Verified: 4047 backend (10 new), 1696 on PostgreSQL via the compose sidecar,
66 e2e, bin/ci-check green. The three assertions that were satisfied by
initial state - the two refusals and the cleanup - were mutation-tested
afterwards, and each of four mutants killed exactly its intended test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All three endpoints shipped with the mount-period model. All three had a query
hook. None of them had a caller: `useRotateTires`, `useRetireTire` and
`useCreateTire` each had exactly zero consumers in `src/`, and there was not
one `tireList.rotate*` or `tireList.retire*` translation key. So v3.3.0's two
headline tire features were reachable only with curl, and retiring a worn tire
-- the whole reason the mount-period model is worth collecting data for -- left
users with nothing but the Delete button it exists to replace.
Nothing caught it because the e2e suite drove all three through
`request.post`. That proves the endpoints work; it does not prove anyone can
reach them. The reachability gate cannot see it either: it flags unimported
FILES, and these were unused EXPORTS in a file with seven other consumers.
Found by listing every hook against its consumers instead of checking the one I
suspected -- one of the three would have been missed by the shorter check.
Rotate is a header control, because it is one action on four tires rather than
four actions. It offers the four standard patterns by DRIVETRAIN rather than as
a diagram, since the user knows what they drive and the arrows are the part of
every rotation chart people misread, and it shows the resulting moves in full
so the choice is checkable without trusting the name. Disabled unless all four
corners are mounted: a partial pattern comes back as a 404 or a 409 naming a
corner, neither of which tells the user they need a fourth tire.
Retire sits beside Dismount on the card, not beside Delete in the edit drawer.
The mis-tap worth guarding against is the one that actually happens -- reaching
for Delete when you mean "I replaced this tire" -- so the discoverable control
is the safe one and the destructive one stays buried. The drawer is the
confirmation step and is the only place the app says out loud that retiring and
deleting differ. It reaches stored tires too: a set can wear out and be
replaced without going back on the vehicle.
Storage is a sixth destination in the Add drawer's position picker, which also
fixes a bug in its own right: Add was DISABLED once every slot was taken, so
the single moment a second seasonal set most obviously needs entering was the
one moment nothing could be entered at all. It now opens on storage in that
state instead of preselecting an occupied corner the form then refused to
submit.
Two things found while re-reviewing under the units manifest:
- Only the reading drawer's odometer carried `step={u.distance.step}`; mount
and dismount fell back to the HTML default. That default equals the step for
every distance unit in the vocabulary, both whole-unit, so all four now carry
it as a guard against a future decimal distance unit rather than as a fix.
- `openTires` in the e2e matched `heading name: 'Tires'` non-exactly, which
also matches the empty state's "No tires tracked yet". Latent because every
other test in that file seeded a tire through the API first, so the state a
new user is actually in had never been rendered there.
Verified: 2113 frontend (12 new), 11 tire e2e including four new browser-driven
ones, bin/ci-check --frontend green with both manifest digests re-reviewed and
re-stamped. Mutation-tested twice over. Six mutants against the unit tests --
a pattern that leaves a tire put, a pattern with a duplicate target, retire
calling delete, storage calling create-and-mount, canRotate pinned true, and
the Add fallback restored to a corner -- each killed exactly its intended test.
Then, against the e2e, deleting the Retire button and deleting the Rotate
button: both killed, which is the proof these tests would have caught the
original defect.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ight
D6 of the mount-period design shipped as a table and a column and stopped
there. `tire_sets` and `tires.set_id` were created by migration 097; there was
no schema to name a set, no way to put a tire in one, and no endpoint to fit
one. Same shape as the three unreachable flows in the previous commit, one
layer further down: the data model was there and the surface was not.
Sets stay UX grouping only, as D6 requires. Nothing here computes distance,
wear or position; `tire_mount_periods` remains the single source for all three.
**The fit is the whole point.** `POST /tire-sets/{id}/mount` takes an odometer
and nothing else. Where each tire goes is REMEMBERED, read from that tire's own
mount history, because the periods already record which corner it sat on and
asking again would be asking the user to retype something the app knows. The
corner comes from the highest-id period rather than the latest `mounted_on`:
periods are append-only, so someone entering last winter's history after the
fact must not have that backfill outrank the mount they did this morning.
It refuses rather than guesses. A tire that has never been fitted has no corner
to go back to and the fit fails naming it; two tires last seen on the same
corner fail naming the corner. All or nothing either way, because an
arrangement that applied three of four moves is one nobody asked for and one
the user has to read back corner by corner to discover. Everything the incoming
set displaces comes off in the same transaction, bounded by the same odometer,
so the outgoing periods are closed rather than left open at a corner someone
else now holds. A member already sitting on its own destination is left alone:
taking it off and putting it straight back would split its history at a moment
when nothing happened to it.
`apply_mount_moves` is extracted rather than copied. The vacate/flush/assign
dance is the subtlest thing in the tire service -- `uq_tires_vin_position` is
IMMEDIATE on both dialects and SQLite has no DEFERRABLE -- and a second copy of
it does not fail loudly, it corrupts an arrangement. Rotation and set-fitting
now run the same code.
Two bugs found on the way, neither of them in the sets work:
- `update_tire` answered with `_to_response(tire)` and no odometer, so a PUT
that changed a brand came back reporting the tire's distance as unknown. It
never rendered because the client refetches, which is exactly why it lived.
- `useUpdateTire` invalidated the tire list but not the set list. Set
membership is DERIVED from `tires.set_id`, so filing a tire into a set left
the set reading "Tires: 0" until something else forced a refetch. Found by
the browser test and invisible to the component tests, which mock the hooks
away. Every tire write now invalidates the set key too.
And a guard removed rather than kept: `delete_set` explicitly nulled its tires'
`set_id` before deleting. No mutation could kill it, because SQLAlchemy
de-associates a loaded one-to-many before the DELETE regardless of dialect or
pragma. Deleting it and watching the suite stay green is what proved that.
Verified: 4062 backend (14 new), 1710 on PostgreSQL, 2120 frontend (7 new), 72
e2e (2 new), bin/ci-check green with six manifest digests re-reviewed and
re-stamped. Mutation-tested at all three layers. Eleven service mutants, and
the first pass left three ALIVE, each a real gap now closed: the
newest-period-wins rule was claimed in a docstring no test exercised, the
retired-member filter was unasserted, and the delete guard above. Seven UI
mutants. Two e2e mutants -- removing the Sets button, and reverting the
invalidation fix -- both killed, which is the proof these tests would catch the
original defect and the one they found.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six Added bullets, one BREAKING note for `POST /tires` dropping `position`, two Fixed, and an upgrade note for the one thing that changes for every existing user: the wear estimate goes quiet until a mount odometer is recorded. Stated plainly, including that a single-set owner loses a figure that was correct for them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`GET /api/analytics/vehicles/{vin}/tires`, the backend half of spec B. #152's
reporter noted tires appear nowhere in Analytics, and a reply on the issue made
it a public commitment.
Read-only, no migration. The tires come from `TireService.list_tires` with
retired ones included, so this endpoint and the tire card cannot disagree about
a distance or a projection: a second serialisation that can drift from the
card is worse than no analytics page. The tread trend is derivable from each
tire's own readings, which are already on the wire, so it is not a new field
either.
What analytics adds is readiness, and it leads with it because of a
measurement. On the instance that asked for this there were two tires, two
readings and ZERO readings carrying an odometer, so every analytical block
would have rendered empty. A page whose job is to display tire data has to
first help you produce some.
The three requirements are counted INDEPENDENTLY, which is the whole reason
this is not read off `wear_status`. `project_wear` short-circuits in a fixed
order, so a tire missing both a minimum tread and its reading odometers reports
only `no_minimum_set`; prompts built from that status would name one problem,
hide the other, and leave the user fixing the first only to find the block
still empty with no new advice.
Two distance statuses are deliberately NOT prompted for: `spare_only` is a
state rather than a gap (the tire has never rolled) and `odometer_rollback` is
bad data whose repair is to correct a number, not supply a missing one.
Retired tires are in the response and in none of the counts (B10). Their final
figures are the most complete data the app will ever hold about them; telling
someone to add an odometer reading to a tire in a landfill is noise, and on a
vehicle that has replaced three sets that advice would never go away.
Verified: 4085 backend (23 new), bin/ci-check --backend green including the
authz tripwire. Ten mutants against the readiness counts, all killed. One of
them -- a check that read only the newest reading's odometer instead of both --
is why there are now two tests for that pair rather than one: the figure is a
difference, so a one-sided check passes half the property.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The frontend half of spec B, and the last of #152. Tread over time, projected life and distance on tire, with readiness first. Readiness leads because of a measurement, not a preference. On the instance that asked for this there were two tires, two readings and zero readings carrying an odometer, so every analytical block would have rendered empty. The block names the one thing to record next, ranked by how many tires it unblocks, except that a tire at or below its minimum outranks any amount of missing data: that is something to do today, not something to write down. It disappears on its own once everything is answerable. Every figure arrives already computed by `TireService`, so the page and the tire card cannot drift. Every `DistanceStatus` and `WearStatus` member has its own wording, because collapsing five of them into "unknown" is the defect the typed results exist to prevent, and the legacy raw-delta projection is SUPPRESSED rather than labelled: an "estimate" badge does not communicate that 648,000 km is structurally invalid rather than imprecise. Gated on `tires.length` and nothing else (B9). Deliberately not `isMotorized`: that constant is `['Trailer', 'FifthWheel', 'TravelTrailer']`, so gating on it would exclude exactly the vehicles that have tires and have blowouts. Retired tires are in the table and out of readiness (B10). On a vehicle that has replaced three sets, counting them would make the advice permanent. Page-only for v3.3.0, and that is asserted rather than assumed: a test reads both PDF modules' section headers out of the AST and fails if one is about tires, tread, wear or rotation. On upgrade day most tires answer `nothing_bounded`, so an exported tire section would be a page of blanks in a document people archive and re-read months later. The changelog says so too, because it is a user-visible omission. Two things this turned up: - The help copy I wrote said "the kilometres driven on the other one". A fixed unit word in help text is wrong for every imperial reader, and neither the expression gate nor the hardcoded-strings gate can see it, because it lives in a locale JSON as a properly translated string. Caught in the manifest re-review, which is what that ceremony is for. - The section first used react-query and took the whole Analytics page down in four test files that render it with no `QueryClientProvider`. The provider exists in the real tree, so this only ever appeared in tests. Switched to the `useEffect` + `api.get` shape Analytics.tsx uses for its own six payloads, which is the local convention anyway. Verified: 4088 backend, 1716 on PostgreSQL, 2139 frontend (22 new), 72 e2e, bin/ci-check green with five manifest digests re-reviewed and one new row dispositioned. Twenty-three mutants across the readiness counts, the section and the PDF guard. Two survived the first pass and both were my tests being wrong: one asserted a CANONICAL figure was absent when the reader unit renders it as a different number entirely, and one was masked by a second guard downstream, so it took a mixed fixture to see it at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ttributes The last piece of the v3.3.0 bug batch. The three importers and two exporters were repaired in f7faaf5 and their round-trips are covered; this is the guard the design asked for and that commit did not land, and it is the part that catches the next one rather than the three already known. The bug class hides at every other layer. An export reading `record.premium` on a model whose column is `premium_amount` raises AttributeError at request time, so the route 500s only for a vehicle that HAS such a record. An importer constructing `WarrantyRecord(coverage=...)` raises TypeError, which the enclosing `except Exception` turns into "Invalid record data" and the endpoint answers HTTP 200 blaming the user's CSV for an application bug. Nobody reports that, which is why it survived to v3.3.0. Tax is why this is a walker and not a list: tax EXPORT was always fine, so enumerating outward from the export bug could not reach the four nonexistent kwargs on the tax IMPORTER, and two hand-written revisions of the spec missed it. No tax record had ever imported. Writing it turned up three defects in my own walker, all of which the failure output named: - It bound the QUERY variable to the model, so every `result.scalars()` read as a missing attribute. - It kept one binding table per function, and `export_vehicle_json` reuses the name `r` across three sibling comprehensions, so fuel and DEF rows resolved to HoursRecord. - It checked membership of the MAPPER, which calls `ServiceVisit.calculated_total_cost` broken. The export defect is an AttributeError, so the predicate is `hasattr`: a hybrid or plain property is a perfectly good read. `export_vehicle_json` is the reason comprehensions are handled at all. It selects six models and contains no `for` STATEMENT, so a walker built on `ast.For` alone would skip the largest handler in the file and report a clean run. That has its own test, and killing it is one of the mutations below. Verified: 4097 backend, bin/ci-check --backend green. Six mutants, all killed: both original export bugs reintroduced verbatim, a bogus attribute read inside a comprehension, bogus kwargs on the tax and warranty importers, and the guard-the-guard -- stopping the walker from seeing comprehensions must fail a test rather than quietly reduce this file to an assertion about nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…onstraints `validateLineItems` is what stands in for `min="0"`, `step="0.01"`, `min="1"` and SupplyUsedPicker's unit-dependent `step` after 01a7ca3 removed them. It had no test of any kind. Six branches, and the only thing between a removed browser constraint and a bad write. The existing validation suite could not have covered it: it MOCKS `LineItemEditor`, so both its behavioural cases and its structural sweep for leftover native attributes enumerate a tree the per-line-item inputs are not in. Those are the inputs that matter most -- they sit in sections that can be collapsed or scrolled away, which is exactly where a native constraint aborts a submit with nothing shown, and they were missed by two hand-written revisions of the fix for the same reason. Each case asserts a visible message AND that nothing was posted, because a form that complains and posts anyway is the failure the change exists to avoid. There is a positive control, without which every one of them is satisfied by a form that refuses everything. One branch has no test and says so in the file: `Number.isNaN(item.cost)` has no reachable input, because `type="number"` sanitises unparseable text to '' and the handler maps '' to undefined before the validator sees it. Kept as cheap defence for paste and programmatic paths rather than deleted to satisfy a coverage rule, and written down so that is a recorded decision rather than something the next reader has to re-derive. What IS pinned is the real behaviour: no throw, and no NaN on the wire. Verified: 2146 frontend (7 new), bin/ci-check --frontend green. Seven mutants, all killed: skipping the validator entirely, dropping each of its four testable checks, and putting `min` and `step` back on the two nested inputs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four Fixed bullets the batch had landed without: the warranty, insurance and tax CSV repair in both directions, the warranty mileage limit column, the PostgreSQL reminder-notification write, and the service visit that saved nothing without saying why. All four are user-visible and none of them would have been reported as described. The CSV import blamed the user's file, the PostgreSQL failure looked like notifications simply not being a feature, and the form looked like a dead button. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…on 098) Sessions open on contact today, so a parked WiCAN's 95-minute battery heartbeat records a drive: 2,975 of 3,238 sessions on this instance were phantom, while real drives out of broker range were missed entirely. Deciding on movement instead needs state that outlives a request, because the MQTT subscriber, the HTTPS route and the scheduler are three execution contexts and an in-memory candidate is invisible to two of them. - movement state on livelink_devices, provenance on drive_sessions - livelink_reconstruction_runs, so a run's refusals survive a log rotation - one open session per device, as a partial unique index The preflight is an inventory of every open row rather than a duplicate scan: the pointer can name the OLDER row in the race, and unlink_device leaves singleton orphans that violate nothing today and would reject every future session start once the index exists. Also collapses the two hand-written speed/RPM key lists onto one helper, so a key that can open a session is always a key the aggregates can read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A session opened whenever the dongle could reach the broker, so a parked WiCAN's ~95-minute battery heartbeat recorded a drive. Measured on this instance: 2,975 of 3,238 sessions were phantom (83%), and real drives out of broker range were missed entirely. The predicate is movement, at every live site. `handle_ecu_status_change`'s online branch no longer opens a session; the observer hangs off `store_telemetry`, which all three live paths funnel through. Movement is confirmed by two consecutive samples at or above the existing 5 km/h idle threshold, or by an odometer increase across the same window. RPM alone opens a *pending* drive, so an idling vehicle keeps its warm-up samples without being credited a trip. Two clocks, because the 5-minute setting is a connection-loss detector and must not double as a drive-splitter: contact loss measures from `last_seen` and retains the session for reopening; the new 15-minute drive gap measures from `last_movement_at` and is final. A six-minute stop no longer splits a drive, a twenty-minute one still does. Neither clock stamps `ended_at` from its own cutoff -- both close at the last movement, so a drive's tail is not padded by parked heartbeats. Also fixes the grace-period finalizer, which had never closed a session: the routes persist ecu_status='offline' on arrival, so `handle_ecu_offline` saw offline->offline and no-opped, and the finalizer then advanced `last_seen` itself. Its tests mocked that call and asserted only that it happened, so they passed throughout. Mutations run, each killing tests (recorded so the next reader need not re-derive them): M1 unconditional handle_ecu_online .......... 5, all three sites M2 movement floor -> speed > 0 ............... 6 M3 engine-on opens a session ................ 14 M4 started_at at first movement sample ....... 4 M5 the pre-v3.3.0 finalizer .................. 4 Test-only: `test_location_service` used a literal "dev1" for every test in a suite that shares one database, which the new partial unique index correctly refuses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bulk_backfill called only _refresh_sessions_in_span, which selects `ended_at IS NOT NULL` -- sessions that already exist and are already closed. It had never created one, and the SD card is the only path for anything driven out of broker range: off home WiFi the WiCAN reaches no broker at all. On 2026-09-01 the Ram drove 16.0 km and was credited 3.0; the Mirage drove 10.0 and was credited 0.0 across fifteen sessions. group_drives applies the same predicate and the same gap as the live path, so a journey is cut identically whether it arrived over MQTT or off a card. The debounce carries over: a lone above-floor sample is not a drive, and here the stakes are higher, because a replay path that made a session per contact burst would invent thousands of phantom drives out of history and no later upgrade undoes that. A month of parked heartbeats stays nothing. Never touches a Torque session (the phone's id is better evidence than a gap threshold), never writes overlapping windows, and leaves an ambiguous window alone rather than guessing during an ingest. Runs once per call, and from PARSED rows: the retry after a crash between the batch commit and the watermark save inserts zero rows and must still refresh, which a span built from inserts would skip permanently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
livelink_session_gap_minutes (15) is its own setting, not the session timeout. The two answer different questions -- "has this device gone quiet?" is not "was that the same drive?" -- and sharing one would mean an admin cannot fix trip grouping without also changing failure detection. livelink_session_boundary_mode (movement | contact) reverses the change. Not hedging: for a device whose signals nothing here recognises, contact produces REAL drives where movement produces none, and it lets an operator bisect a bad upgrade on an instance that cannot be downgraded. Which is only defensible with the diagnostic beside it, since a silent zero is the failure this whole change exists to remove. A device publishing engine telemetry but nothing recognisable as speed, RPM or odometer is named once per process, with the keys it did send, and told which of the two fixes applies. A parked vehicle publishing only its battery heartbeat is not flagged, or the warning would fire everywhere and mean nothing. Both settings are wired through the admin schema and route: a setting the UI cannot reach is a setting that does not exist, and every behavioural test passes without that wiring. Test-only: the settings tests write GLOBAL rows into a suite that shares one database with no rollback, and the route under test commits. Leaving `contact` behind put 28 tests in files that never mention settings into contact mode without knowing it, so the restore fixture commits too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…default An opt-in tool, dry run unless --apply, exit 2 on refusal. It rebounds pre-098 sessions onto the drives their surviving telemetry describes, deletes proven phantoms, and splits a session that spans several drives. The tests that matter are the ones proving it REFUSES, because the live fix is reversible with a setting and deleted history is not. "There is telemetry in the window" is not proof of anything: retention prunes by timestamp, so a session straddling the horizon keeps its later rows and loses its movement rows, and a drive taken out of broker range has no live samples at all -- both read as "telemetry present, no movement", which is exactly what a phantom looks like. So deletion needs positive evidence on three counts: inside the horizon with a margin, samples at both boundaries, and a plausible cadence throughout. Torque sessions never enter the candidate set (the phone's id is better evidence than a gap threshold). GPS points are reassigned by timestamp on a split and asserted to still have a home, because drive_session_id is ON DELETE CASCADE and the dev instance has zero rows in that table -- which is why the risk was invisible when this was first proposed. Deletion goes through db.delete() on loaded objects so the dry run's rollback is meaningful, and the dry-run test asserts location_point counts too. refresh_aggregates gains clear_first, because narrowing a window otherwise leaves the wider window's figures -- the defect PR #157 fixed, reintroduced by the tool meant to consolidate it. It defaults to False: the scheduled refresh must never blank a session whose telemetry has been pruned. Each run writes a livelink_reconstruction_runs row, including refusals with their reasons. A log rotates and a container restart loses it, and refusal is routine here, so in a quiet log a safe refusal and a broken tool look identical. Test-only note: the first version of the clear-list completeness guard could not fail -- it enumerated the very list it was checking, so deleting avg_coolant_temp from that list left all five tests green. It now enumerates from the ORM, and the mutation kills it by name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…the UI Two settings in LiveLink settings (drive gap, detection mode) and a History Rebuilds table listing each reconstruction run with what it changed and what it left alone. The tool is a CLI, so this table is the only way its result reaches a person, and refusal is its routine outcome: without the reasons, "40 drives left alone" and "the tool is broken" look identical. Refusal reasons cross the wire as keys, and the component's label map is checked against the backend's own ALL_REFUSAL_REASONS tuple, read from the Python source at test time. Dropping one label fails that test by name. Test-only note: the first version of that guard rendered the refusals and asserted no raw snake_case reached the DOM. It could not fail -- the global `t` stub is `(key) => key` and never interpolates, so the reason never reached the DOM at all and it passed against an empty map. Also three pre-existing PostgreSQL-only test failures, all fixture debt in files CI runs only under SQLite (ci.yml scopes PG to tests/migrations and tests/integration): - an 18-character VIN in a VARCHAR(17); PG enforces length, SQLite does not - an aware datetime seeded into a naive column, relying on SQLite's bind processor to drop the tzinfo - fuel source ids with no fuel_records row behind a real foreign key Each poisoned the shared session on failure, so the tests that ran next failed with PendingRollbackError and pointed at themselves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
check_session_timeouts has no `kind` filter, and the new drive-gap clock
reads last_movement_at / movement_ended_at / movement_started_at. A Torque
session has none of them: resolve_torque_session never calls the movement
observer, deliberately, because the phone supplies an authoritative session
id. So the fallback chain landed on started_at and the gap closed an
actively-uploading trip fifteen minutes after it BEGAN, cutting a one-hour
drive into a quarter-hour session plus forty-five minutes belonging to
nothing -- on a source that was working correctly.
A session this algorithm did not cut now gets the old rule whole: close on
contact loss, at the last contact. Keyed on boundary_algorithm_version, so
it says which algorithm owns the session's boundaries rather than testing
for Torque by name.
The spec named this test ("a Torque device's timeout behaviour is
byte-identical before and after, and check_session_timeouts has no kind
filter today") and it was the last one outstanding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every other test here exercises one rule; this replays the pattern that motivated the rework and asserts the outcome a user would describe, because a set of individually correct rules can still compose into a wrong day. The pattern is Diamond's 2026-09-01, not invented: a heartbeat every ~95 minutes, fifteen recorded sessions, thirteen of them holding no telemetry, and 10.0 km driven and credited as 0.0. It should be one drive with its distance and nothing else. Its first version fed the observer directly, so restoring the unconditional handle_ecu_online left it green -- it tested the state machine and skipped the wiring, which is where the previous design revision hooked one of three paths. It now goes through store_telemetry, the entry point MQTT and HTTPS both funnel into. Also two real pyright errors caught by bin/ci-check (the host binary exits 0 silently without libatomic1, which is why that script exists): an Optional timestamp reaching a chained comparison, and a duration computed from two nullable columns narrowed only by assignments a few lines above. Removes PENDING_SOURCE_SPEED, which nothing wrote and nothing could: a pending drive IS "engine on, nothing moving yet", so a sample above the movement floor confirms movement outright rather than opening one. Documents that merging is not implemented, rather than shipping a sessions_merged counter that is always 0 and reads as "we looked and found none". Two old sessions that are really one drive stay two; both report real movement and neither overlaps the other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Host ruff is 0.15.10 and the container's is 0.16.1, and they disagree about these line breaks. The container is what CI runs, so it is the authority. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…manifest The class docstring predated most of the file, which is now 1400 lines. The natural cut is the aggregate computation: self-contained and orthogonal to the boundary rules. Left in place deliberately rather than moved in the same change as the rules themselves, and the docstring now says so. Manifest re-stamp for exporting REFUSAL_LABEL_KEYS to its test. Re-scanned under the corrected (no word boundary) scan: two hits, both incidental substrings -- "length" in Array.prototype.length, "mi" in "emit" and "semibold". Disposition holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
C12's diagnostic had only its log half. A device whose speed and odometer
arrive under names this codebase does not recognise records no drives at
all, and "no drives" is indistinguishable from "the vehicle was parked" --
a silent zero, which is the exact failure the boundary rework exists to
remove, so leaving one for this cohort would be absurd.
The log names it once per process with the keys it sends; this is the half
that survives a rotation and reaches someone not reading logs. Derived from
last_movement_at rather than a new column.
A dongle in a drawer also reports no movement, so the notice waits for the
device to be keeping up with the fleet. "Recently" is measured against the
newest check-in among the devices shown, not the wall clock: Date.now() in
render is impure and eslint refuses it, and the relative form answers the
better question anyway -- an instance whose whole fleet has been offline for
a month raises nothing rather than raising everything.
The CHANGELOG already promised this ("Settings names the device"), which is
how the gap was found.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The upgrade note promised Settings would name the device and list its readings. Settings names the device; the readings are in the container log. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both are new on this branch and therefore not yet public. The hostname is a private-LAN server nickname that means nothing to an outside reader, and the device id is derived from a dongle's MAC. Seven other references to that hostname are already on main and untouched here: scrubbing them now would not un-publish them, and folding a seven-file rename into a feature branch would muddy its review. Worth doing on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
homelabforge
changed the title
homelabforge
marked this pull request as draft
September 3, 2026 23:47
…e rebuild tool Three changes, all from testing v3.3.0 against a real prod copy. Cut `tools/reconstruct_session_boundaries.py` and its admin surface. It only ever deleted, narrowed and split existing sessions, so `--apply` took recorded distance from 3,896.9 km to 182.4 km with `created 0`: the drives it should have recovered live in the gaps BETWEEN the old contact-sessions, and it never looked there. A tool that removes 2,700 km of history is not shippable even opt-in and dry-run-by-default. Telemetry is retained and every session is stamped `boundary_algorithm_version = 0`, so a corrected pass can still find them. Read distance from the finest distance signal in the window, not the odometer alone. Measured on a 2019 Mirage over 165 days: `ODOMETER` changes 149 times (~24 km per step) while `31-DISTANCESINCECODECLEAR` changes 1,712 times (~1 km), and both agree with recorded mileage in aggregate. Its average trip is shorter than one odometer step, so 2,781 of its 2,998 sessions computed to zero. Over the same session windows this takes it from 17.9 km to 103.9 km and from 12 sessions with a distance to 73; a Ram, whose odometer already resolves to 2 km, is byte-identical because an odometer wins its own ties. Distance is the sum of positive steps rather than a span, so a code clear mid-window reads 15 km and not 806. PID 0x21 is excluded despite being standard, metric and 1 km: it counts only distance driven with the MIL lit, so on a coarse-odometer car it would win and report 5 km as the length of a 12 km trip. Decide the no-movement notice on the backend. It asked `last_movement_at IS NULL`, which migration 098 makes true for every device that exists, so on the first boot after upgrading it named the whole fleet and pointed at the setting that reverts the boundary fix. The missing half is whether the device is OPERATING, which needs its parameter keys. An EXISTS per candidate, short circuiting at the first non-heartbeat row: 0.01 ms against 20.3 ms for a grouped scan that was most expensive for exactly the devices it flags. `recompute_session_aggregates.py` is already in the published upgrade sequence and will lower old drives sharply. That is the window-scoping fix, not this one, and the CHANGELOG now says so along with the fact that vehicle mileage lives in a separate table and is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…oved yet Rebuilding the dev instance against a real prod copy flagged BOTH healthy WiCAN devices as movement-unreadable, which is the false alarm the previous commit claimed to fix arriving by a different road. The two halves of the predicate measured different time bases. `last_movement_at` is a column migration 098 CREATES, so it can only be written by telemetry arriving after the migration, while the operating-evidence query looked at seven days of telemetry HISTORY, which is almost entirely older. Every device driven in the last week but not since the upgrade came out flagged. So the question is asked without reference to time: a device is unreadable when it publishes something beyond the parked heartbeat and NONE of what it publishes is a speed or an odometer this codebase knows. That is what the notice claims, and it is true or false regardless of when the column appeared. RPM does not count as readable, though it is a movement signal. It opens a pending drive and never confirms one, so a device with legible RPM and an unrecognised speed key still records no sessions at all -- exactly the cohort being named. Three existing tests caught that when the first version counted it. `recompute_session_aggregates.py` reported only the distance sessions LOSE. `_movers` drops rows where either side is None, so a session going from no distance to a real one was invisible by construction, which on the instance tested is 256 of one vehicle's 264 sessions. It now reports both directions. Correcting the CHANGELOG with the measured figure: applying the recompute takes that vehicle from 3,890 km to 340 km, not the 104 km stated. 104 km is recomputed from telemetry still on disk; the other 236 km is older figures left untouched on 18 sessions whose telemetry has been pruned, because for a drive past the retention horizon that stored figure is the only record left of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The drive list opened a session every time a parked dongle checked in, so on the instance this was built against 2,921 of 3,262 recorded sessions never moved at all. They are not drives, they cannot be rebuilt into drives (the telemetry that would prove where one began was never captured), and they must not be deleted: the reconstruction tool cut from this release removed 2,700 km of real distance trying. So the list narrows and every row stays where it is. FILTERS ON MOVEMENT, NOT ON WHICH RULE RECORDED THE SESSION The first version of this asked `boundary_algorithm_version = 0`, which is the column added by migration 098 to mark pre-upgrade rows, and it looked right for the same reason it was wrong: the marker CORRELATES with worthlessness without being it. 341 of those pre-098 sessions record a vehicle that demonstrably moved, and hiding them buries real journeys to tidy a display. On one vehicle that is the difference between an empty page and 232 drives. "Moved" is `IDLE_THRESHOLD_KMH`, the constant the boundary predicate already uses, so the list hides exactly what the recorder calls stationary. A NULL speed counts as stationary: a session whose telemetry has been pruned cannot prove it was a drive, and showing every unprovable row defeats the filter. On the measured data 2,816 sessions have no speed on record and 5 of them carry any distance, so that costs about 5 rows to hide 2,921. `stationary_total` is reported whether or not the rows are included, so a filtered list that shows nothing can name what it is holding back instead of looking broken. `boundary_algorithm_version` is exposed per session so a real old drive is tagged rather than silently odd: its distance may be zero because the odometer only steps every 24 km, not because anything failed. Server defaults to including everything, so no existing caller loses history; the UI is what opts out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
OIL CAPACITY RODE THE FUEL ADAPTER Engine oil capacity shared the volume formatter fuel uses, so a reader on US gallons was asked for gallons of engine oil. Entering `12` for a twelve-quart engine stored 45.42 L, and the card then read `12 gal` straight back: the round trip is symmetric, so the error is invisible from the screen and only an assertion on the canonical value can catch it. Two vehicles on the instance this was built against were wrong by 3.785x before this landed, a 6.7 Cummins at 45.42 L against a true 11.36 and a Mirage at 12.11 against 3.03. `UnitSet.volume` is `L | gal_us | gal_uk` and holds no quart token, which is why this is derived rather than added to the vocabulary: `oilCapacityUnit.ts` takes the quart as a quarter of the gallon the reader already resolved to. It is a Record keyed by the volume token rather than a branch on it, so a volume unit added later fails tsc here instead of falling silently into the wrong leg. A `gal_uk` reader gets the Imperial quart on day one. `supplyUnits.ts` still hardcodes the US quart and keeps its documented 20.1 percent defect, because that factor re-interprets quantities already stored and no column records which quart a row was written in. That is the D8 amendment's data decision, not this one's. Precision for this quantity drops from 2 to 1, so a metric reader sees 4.7 L where they saw 4.70 L. Deliberate, and pinned by the litres-with-lb-ft test. FUEL FILTER ON EVERY VEHICLE, NOT ONLY DIESELS Migration 099 adds `vehicles.fuel_filter_part_number`. FATAL, because the ORM maps the column and a skipped migration would 500 every vehicle read. Gating it on fuel type would wrongly exclude an older petrol vehicle whose inline filter is a real service part, and a vehicle without one leaves it blank and the card omits the row. Existing rows stay NULL: a backfilled default would assert a filter this instance was never told about. The Fluids & Torque card and its editor are Title Cased and laid out two up, which is the work that surfaced both of the above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…casing SEVEN HAND-ROLLED HEADERS BECOME ONE CARD Settings was skipped by the v3.0.0 reskin: all five tabs still wrap their sections in `bg-garage-surface rounded-lg border border-garage-border p-6` around a bare `<h2>`, which is why this tab reads as unfinished beside every card next to it. The seven copies here become one `IntegrationCard`. Its title row is written out rather than delegated to `CardHeader`, which would otherwise be the obvious reuse: `CardHeader` renders its icon on the RIGHT beside the actions and has no slot for a subtitle, while this page wants the icon leading the title with the description stacked under it, which is what `settings/WidgetKeysPanel.tsx` does at the top of the same screen. Taking `CardHeader` put two header shapes on one page. The `h3` typography is copied from it so the two cannot drift. Promoting a leading-icon-plus-description variant into `CardHeader` is the follow-up, and that is a change to a primitive with roughly 40 call sites rather than to this tab. THE GRID LEFT A QUARTER OF A ROW EMPTY A fixed `lg:grid-cols-2` with `items-start` paired the ~530px NHTSA card against ~280px of stacked cards, so the bottom right of that row was blank. The four narrow sections now flow as a CSS-columns masonry, NHTSA first: columns balance by height, and leading with a short card is what left the hole. Source order is preserved, so the one-column mobile reading order still groups. LLM Features and Shop Finder stay full width, one wanting a three-field row and the other a table. A COLUMN NO SCREEN READER COULD READ The provider table's state column rendered a bare lucide Check or X with no accessible name, so a screen reader announced an empty cell for every provider, and five red X glyphs read as five errors rather than as five switched-off providers. It is a labelled Chip now, and the header says Status, which was wrong as "Active" sitting above an Inactive row. Edit and Remove become IconButtons: a red Remove on every row made a routine table look destructive. CASING Title Case names things, sentence case says things. The tab carried both spellings on one screen: "Recall Check Interval" beside "Webhook ingest token", "Enable NHTSA Integration" beside "Enable receipt draft parsing". Names go up, toggle labels come down. The other six locales fall back to English and do not block the translation gate. TESTS WHERE THERE WERE NONE Both written before the refactor, because reshaping 771 lines of JSX with nothing asserting the sections still render is how a card vanishes behind a mispaired closing tag. It nearly did: the conversion left one stray `</div>` and a duplicated guard close, caught by tsc. Three mutations killed: reverting one Title Case value, swapping the chip back to a bare icon, and deleting the CarComplaints card. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three workstreams for v3.3.0, plus a batch of standing bug fixes. 35 commits,
two migrations, and one behaviour change that will visibly alter existing data.
Read this first if you are upgrading
Drive sessions are now decided by movement, not by the device connecting.
A session used to start whenever the dongle could reach the broker. A parked
WiCAN checks in roughly every 95 minutes, so most recorded "drives" were a
parked vehicle: on the instance this was measured on, 2,975 of 3,238 (83%).
Meanwhile drives taken out of broker range were recorded as nothing at all,
because the SD-card path could update an existing session but never create one.
After this, your drive count drops sharply and the remaining drives are real.
History is left exactly as it was: nothing is deleted, merged or rewritten. Each
session records which rule cut it, so a later release can revisit them. The full
note is in CHANGELOG.md.
Distance is also no longer read from the odometer alone. On a 2019 Mirage the
odometer changes 149 times over 165 days (~24 km per step) while
31-DISTANCESINCECODECLEARchanges 1,712 times (~1 km), and both agree withrecorded mileage in aggregate. Its average trip is shorter than one odometer
step, so 2,781 of its 2,998 sessions computed to zero. Over the same session
windows the finest-source rule takes it from 17.9 km to 103.9 km and from 12
sessions with a distance to 73. A Ram, whose odometer already resolves to 2 km,
is byte-identical, because an odometer wins its own ties.
Back up before deploying. Migration 097 is FATAL and 098 mutates rows: its
preflight closes orphaned open sessions and repairs device pointers so a partial
unique index can be created. Use the backup API, not
cp-- SQLite runs in WALmode and a file copy is torn but plausible.
What is in here
Tires (#153). Mount periods, so a tire's distance is summed over the times
it was actually fitted rather than taken from the vehicle's odometer. Retire,
rotate, tire sets, and storage. Migration 097.
Tire analytics (#152). A Tires section on the Analytics page: tread over
time, projected life, distance on tire, and a readiness block naming the one
reading to record next. Page only for now; not in the PDF or the garage export.
Session boundaries. The movement predicate above, two clocks instead of one
(connection loss and drive gap are different questions), the SD-card path
creating the sessions it describes, one open session per device as a constraint,
and a reversal switch for a device whose signals nothing here recognises.
Migration 098.
Bug batch. Warranty/insurance/tax CSV in both directions (no tax record had
ever imported), reminder notifications on PostgreSQL, service-visit saves that
failed silently, and the tire wear over-estimate for anyone running two sets.
Breaking
POST /api/vehicles/{vin}/tiresno longer acceptspositionand creates astored tire. Mount afterwards, or use
.../tires/create-and-mount. A payloadcarrying
positionis rejected with 422 naming the field.Verification
Local
bin/ci-check: all gates green. Backend 4,280 passed; frontend 2,162passed; pyright 0 errors; units manifest 393 modules dispositioned; API types
freshness clean.
Additionally, and outside what CI covers for these paths: the full backend suite
run against PostgreSQL (4,418 passed). CI scopes PostgreSQL to
tests/migrations/andtests/integration/, which is how a previous migrationdefect reached it. That run also surfaced three pre-existing PostgreSQL-only
test failures, fixed here: an 18-character VIN in a
VARCHAR(17), an awaredatetime seeded into a naive column, and fuel ids with no row behind a real
foreign key. Each poisoned the shared session on failure, so the tests after it
failed pointing at themselves.
Migration 098 was timed at the measured production scale (3,242 sessions):
under 0.01s, and its preflight correctly retains the pointed-at session even
when that is the older of a duplicate pair.
Every new guard was mutation-tested. The mutations and what each killed are
recorded in the commit messages rather than here, so they stay next to the code
they justify.
Known limitations, stated rather than discovered
only deleted, narrowed and split sessions that already existed, so
--applyon a prod copy took recorded distance from 3,896.9 km to 182.4 km with
created 0. The drives it should have recovered are in the gaps BETWEEN theold contact-sessions, and it never looked there. Telemetry is retained and
every old session is stamped
boundary_algorithm_version = 0, so a correctedpass can still find them later.
movement-based regroup of all telemetry finds 461 real drives worth 2,316 km
against 10,375 km actually driven, because the dongle batches every PID once
per ~60s while moving and samples every ~3s while parked. Integrating speed
lands 35-40% low on both vehicles and is not a usable fallback.
now measure a drive but not prove one happened, which matters most for devices
whose speed arrives under an unrecognised key. Follow-up.
all, and adding one needs a seeded device. The new settings have component
tests.