fix(stats): clear stale backfill candidates before projection
This commit is contained in:
lingniu
207
.superpowers/sdd/task-5-report.md
Normal file
207
.superpowers/sdd/task-5-report.md
Normal file
@@ -0,0 +1,207 @@
|
||||
# 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 projector’s `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
|
||||
```
|
||||
Reference in New Issue
Block a user