Files

208 lines
7.9 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Task 5 Report: Backfill Candidate Rows And Final Projection
## Summary
Implemented Task 5 in `go/vehicle-gateway/cmd/stats-backfill` so backfill now writes source candidates into `vehicle_daily_mileage_source` and re-projects `vehicle_daily_mileage` through the shared projector instead of writing final rows directly.
## TDD Evidence
1. Added the failing regression in `go/vehicle-gateway/cmd/stats-backfill/main_test.go`:
- `TestDailySourceLastBuildsCandidateKeysBySourceIP`
- Updated the existing trusted-source test to the new `normalizedSourceKey(protocol, phone, deviceID, endpoint)` signature.
2. Verified the red state with:
```bash
go test ./cmd/stats-backfill -run 'TestChooseTrustedSource|TestDailySourceLastBuildsCandidateKeysBySourceIP' -count=1
```
Observed failure:
```text
too many arguments in call to normalizedSourceKey
have (string, string, string, string)
want (string, string)
```
3. Implemented the production changes.
4. Verified green with:
```bash
go test ./cmd/stats-backfill -run 'TestChooseTrustedSource|TestDailySourceLastBuildsCandidateKeysBySourceIP' -count=1
go test ./cmd/stats-backfill ./internal/stats -count=1
```
Both commands passed.
## Files Changed
- `go/vehicle-gateway/cmd/stats-backfill/main.go`
- `go/vehicle-gateway/cmd/stats-backfill/main_test.go`
## What Changed
### 1. Source-key normalization now matches runtime semantics
- Changed backfill `normalizedSourceKey` to call:
```go
stats.SourceKey(envelope.Protocol(protocol), phone, deviceID, stats.NormalizeSourceIP(endpoint))
```
- This means candidate keys now use:
- protocol
- phone or device ID
- normalized source IP
- Port changes on the same source endpoint no longer produce different source keys.
### 2. Scan-mode backfill now preserves per-source candidates
- Expanded raw-frame reads to include `phone`, `device_id`, and `source_endpoint`.
- Populated `envelope.FrameEnvelope` with those fields before calling `stats.SamplesFromEnvelope`.
- Changed scan-mode aggregation key from:
```text
vin + stat_date + protocol
```
to:
```text
vin + stat_date + protocol + source_key
```
- Extended `metricAgg` to carry:
- `SourceKey`
- `Phone`
- `DeviceID`
- `SourceEndpoint`
- `FirstEventTime`
- `LatestEventTime`
- `QualityStatus`
- `QualityReason`
This keeps scan-mode candidates source-separated instead of mixing sources together.
### 3. Backfill writes candidates + re-projects final rows
- Replaced the direct `vehicle_daily_mileage` batch upsert path.
- `writeAggregates` now, per aggregate:
1. Upserts `vehicle_data_source` using protocol + normalized source IP.
2. Upserts a `vehicle_daily_mileage_source` candidate row.
3. Calls `stats.ProjectDailyMileage` for the VIN/date/protocol.
- Quality handling:
- default `OK`
- `INVALID_DELTA` when daily delta is `< 0` or `> 1000`
- pre-set non-OK statuses from `last_diff` are preserved
### 4. `last_diff` now preserves every source candidate
- `buildLastDiffAggregates` no longer picks a single trusted source.
- It now creates one aggregate per current-source candidate using:
```text
vin + stat_date + protocol + source_key
```
- If a same-source previous-day baseline exists:
- `FirstKM = previous.TotalKM`
- `LatestKM = current.TotalKM`
- `QualityStatus = OK`
- `QualityReason = "same_source_previous_day"`
- If there is no same-source previous-day baseline:
- `FirstKM = current.TotalKM`
- `LatestKM = current.TotalKM`
- `QualityStatus = NO_PREVIOUS_BASELINE`
- `QualityReason = "missing_previous_source"`
This preserves candidates without projecting them by default, matching the projectors `quality_status = 'OK'` filter.
### 5. Reset now clears candidate rows too
- Updated `resetStats` to delete both:
- `vehicle_daily_mileage_source`
- `vehicle_daily_mileage`
within the requested date/protocol scope.
This prevents stale candidates from affecting a subsequent rebuild run.
## Test Results
```bash
go test ./cmd/stats-backfill -run 'TestChooseTrustedSource|TestDailySourceLastBuildsCandidateKeysBySourceIP' -count=1
ok lingniu-vehicle-ingest/go/vehicle-gateway/cmd/stats-backfill 0.572s
```
```bash
go test ./cmd/stats-backfill ./internal/stats -count=1
ok lingniu-vehicle-ingest/go/vehicle-gateway/cmd/stats-backfill 0.174s
ok lingniu-vehicle-ingest/go/vehicle-gateway/internal/stats 0.557s
```
## Concerns / Notes
- `chooseTrustedSource` remains in place for its existing unit test, but the `last_diff` aggregation path no longer relies on it.
- `ProjectDailyMileage` still follows the shared runtime semantics: it selects only `quality_status = 'OK'` candidates and does not synthesize cross-source deltas.
## Review Fix: Stale Final Rows
### Bug Addressed
When all candidates for a backfill target `(vin, stat_date, protocol)` are non-qualifying (`NO_PREVIOUS_BASELINE` / `INVALID_DELTA`), `ProjectDailyMileage` runs but inserts no final row, leaving any prior `vehicle_daily_mileage` row in place. That violated the expectation that a run with no selected candidate should not keep a historical final row for that key.
### Fix Applied
- Added helper in `cmd/stats-backfill/main.go`:
- `clearBackfillFinalMileage(ctx, db, vin, statDate, protocol)`
- Executes `DELETE FROM vehicle_daily_mileage WHERE vin = ? AND stat_date = ? AND protocol = ?`
- Guarded against empty key values.
- Updated `writeAggregates` to call the helper once per exact `(vin, stat_date, protocol)` target before calling `ProjectDailyMileage`.
- Kept existing runtime writer behavior unchanged; only backfill projection sequencing changed.
### Tests Added
- `TestClearBackfillFinalMileageClearsExactKey` in `cmd/stats-backfill/main_test.go`
- Verifies the SQL targets exactly the intended triplet.
- `TestWriteAggregatesClearsFinalBeforeProjection` in `cmd/stats-backfill/main_test.go`
- Verifies backfill writes still occur and final-row clear is executed before projection.
### Updated Evidence
```bash
go test ./cmd/stats-backfill -run 'TestChooseTrustedSource|TestDailySourceLastBuildsCandidateKeysBySourceIP|Test.*Reset|Test.*Projection|Test.*Final' -count=1
ok lingniu-vehicle-ingest/go/vehicle-gateway/cmd/stats-backfill 0.447s
go test ./cmd/stats-backfill ./internal/stats -count=1
ok lingniu-vehicle-ingest/go/vehicle-gateway/cmd/stats-backfill 0.188s
ok lingniu-vehicle-ingest/go/vehicle-gateway/internal/stats 0.547s
```
## Follow-up Fix: Non-Reset Backfill Idempotency
### Additional Bug
The prior fix only cleared `vehicle_daily_mileage` before projection. In non-reset backfills, stale `vehicle_daily_mileage_source` rows for the same `(vin, stat_date, protocol)` could still be selected by `ProjectDailyMileage`, causing old OK candidates to win against fresh backfill input.
### Additional Fix
- Added `clearBackfillTargetMileage` in `go/vehicle-gateway/cmd/stats-backfill/main.go` to delete both tables for the exact target key:
- `DELETE FROM vehicle_daily_mileage_source WHERE vin = ? AND stat_date = ? AND protocol = ?`
- `DELETE FROM vehicle_daily_mileage WHERE vin = ? AND stat_date = ? AND protocol = ?`
- Updated `writeAggregates` so this clear happens at most once per `(vin, stat_date, protocol)` and before the first candidate upsert for that target.
- Kept existing backfill runtime flow intact: still upserts sources/candidates and calls `ProjectDailyMileage` for each target aggregate.
### Tests Added/Updated
- `TestClearBackfillTargetMileageClearsExactKey`:
- Verifies both table deletes use the exact VIN/date/protocol tuple.
- `TestWriteAggregatesClearsTargetRowsBeforeStaleCandidateUpsert`:
- Verifies clear operations happen before candidate writes and projection, and that the stale-candidate flow is tested by using a non-qualifying candidate status.
### Verification
```bash
go test ./cmd/stats-backfill -run 'TestChooseTrustedSource|TestDailySourceLastBuildsCandidateKeysBySourceIP|Test.*Reset|Test.*Projection|Test.*Final|Test.*Candidate' -count=1
go test ./cmd/stats-backfill ./internal/stats -count=1
```