diff --git a/docs/superpowers/plans/2026-09-11-ingestion-restructure.md b/docs/superpowers/plans/2026-09-11-ingestion-restructure.md index 3a4b921..cf29a39 100644 --- a/docs/superpowers/plans/2026-09-11-ingestion-restructure.md +++ b/docs/superpowers/plans/2026-09-11-ingestion-restructure.md @@ -495,6 +495,16 @@ Expected: `missing_site=147`, `will write : 147`, `will skip : 0`, then `DRY RU **If `will skip` is not 0, stop and report the skipped rows before going further.** A disagreement between the two sources means the assumption in the spec is wrong for those rows and someone needs to look at them. +> **Know what `will skip : 0` is worth today.** Production has exactly one +> site. Both "independent" resolvers therefore draw from a candidate set +> of size one, and every one of the 54 vehicles points at it — so the two +> sources *cannot* disagree, and agreement is not evidence. The write is +> still correct (with one site there is exactly one possible value, and +> both sources produce it), but the cross-check only acquires real +> discriminating power once the other sites are onboarded. If this task is +> ever re-run against a multi-site database, treat a non-zero skip count as +> a genuine finding rather than noise. + - [ ] **Step 3: Apply** Same command with `--apply` appended. Expected final line: `wrote 147. fuel_logs still missing site: 0`. diff --git a/scripts/migrations/2026-09-11-backfill-fuel-log-sites.py b/scripts/migrations/2026-09-11-backfill-fuel-log-sites.py new file mode 100644 index 0000000..df018eb --- /dev/null +++ b/scripts/migrations/2026-09-11-backfill-fuel-log-sites.py @@ -0,0 +1,243 @@ +#!/usr/bin/env python3 +""" +Backfill fuel_logs.site for rows that have none. + + PB_URL=... PB_ADMIN_EMAIL=... PB_ADMIN_PASSWORD=... \\ + python3 scripts/migrations/2026-09-11-backfill-fuel-log-sites.py [--apply] + +DRY RUN BY DEFAULT. Pass --apply to write. + +As of 2026-09-11 production has 147 such rows, all carrying +project_code='C26/09' and all carrying a vehicle relation. + +WHY THIS RUNS NOW +----------------- +Every read in the permissioned model is filtered by `site IN (user's +allowed sites)`. A fuel log with no site is invisible to everyone. The +only other site attribution these rows carry is the denormalised text +field `project_code`, which Task 18 deletes. After that deletion the +first of the two sources below stops existing and these rows can never +be attributed again. + +THE SAFETY PROPERTY +------------------- +For every row we resolve the site twice, independently: + A. sites where project_code == row.project_code + B. the matched vehicle's own site relation +and we only write when A and B agree. A row where they disagree, or where +either is missing, is reported and skipped - never guessed. + +Idempotent: rows that already carry a site are left alone. +""" +import json +import os +import sys +import urllib.error +import urllib.request + +APPLY = "--apply" in sys.argv + + +def admin_auth(base_url): + body = json.dumps( + { + "identity": os.environ["PB_ADMIN_EMAIL"], + "password": os.environ["PB_ADMIN_PASSWORD"], + } + ).encode() + req = urllib.request.Request( + f"{base_url}/api/collections/_superusers/auth-with-password", + data=body, + headers={"Content-Type": "application/json"}, + method="POST", + ) + with urllib.request.urlopen(req) as resp: + return json.loads(resp.read())["token"] + + +def api(base_url, token, method, path, body=None): + """ + A 404 is only soft for GET - "this collection/record is absent" is a + legitimate answer to a read. On a write it is not: a PATCH against a + since-deleted record returning None would be counted as a successful + write. Mutations raise. + """ + data = json.dumps(body).encode() if body is not None else None + req = urllib.request.Request( + f"{base_url}{path}", + data=data, + method=method, + headers={"Authorization": token, "Content-Type": "application/json"}, + ) + try: + with urllib.request.urlopen(req) as resp: + txt = resp.read().decode() + return json.loads(txt) if txt else None + except urllib.error.HTTPError as e: + if e.code == 404 and method == "GET": + return None + raise RuntimeError( + f"{method} {path}: {e.code} {e.read().decode() if e.fp else ''}" + ) from e + + +def get_all(base_url, token, coll, fields): + out, page = [], 1 + while True: + data = api( + base_url, + token, + "GET", + f"/api/collections/{coll}/records" + f"?perPage=500&page={page}&fields={fields}", + ) + out.extend(data["items"]) + if page >= data["totalPages"]: + return out + page += 1 + + +def rel_id(value): + """ + Normalise a PocketBase relation field to a single id, or "" when it + does not resolve to exactly one. + + Single relations come back as a bare string, multi relations as a + list. Treating a populated list as truthy and then comparing it to a + string would silently make the two sources 'disagree' - or worse, in + a future schema change where both sides are lists, silently agree on + a set. Anything that is not exactly one id is "unresolved", which + routes the row to the skip report rather than to a write. + """ + if isinstance(value, list): + return value[0] if len(value) == 1 else "" + return (value or "").strip() + + +def main(): + base_url = os.environ.get("PB_URL", "http://127.0.0.1:8090").rstrip("/") + token = admin_auth(base_url) + # Say where we are writing. This script mutates production by default + # target, and a run whose output names no host is a run you cannot + # audit afterwards. + print(f"target: {base_url} mode: {'APPLY' if APPLY else 'DRY RUN'}") + + sites = get_all(base_url, token, "sites", "id,project_code") + site_by_code = {} + for s in sites: + code = (s.get("project_code") or "").strip() + if code: + site_by_code.setdefault(code, []).append(s["id"]) + + ambiguous = {c: ids for c, ids in site_by_code.items() if len(ids) > 1} + if ambiguous: + print(f"ABORT: project_code is not unique across sites: {ambiguous}") + return 1 + + vehicles = get_all(base_url, token, "vehicles", "id,site") + vehicle_site = {v["id"]: rel_id(v.get("site")) for v in vehicles} + + logs = get_all(base_url, token, "fuel_logs", "id,site,project_code,vehicle") + todo = [r for r in logs if not rel_id(r.get("site"))] + + print(f"\nsites={len(sites)} vehicles={len(vehicles)}") + print(f"fuel_logs total={len(logs)} missing_site={len(todo)}") + + writes, skips = [], [] + for r in todo: + code = (r.get("project_code") or "").strip() + by_code_all = site_by_code.get(code, []) + by_code = by_code_all[0] if len(by_code_all) == 1 else "" + veh = rel_id(r.get("vehicle")) + by_vehicle = vehicle_site.get(veh, "") + + if by_code and by_vehicle and by_code == by_vehicle: + writes.append((r["id"], by_code)) + continue + + # Name the reason. "Both sources resolved and disagreed" is a very + # different finding from "this row has no project_code at all", and + # a single undifferentiated skip count hides which one happened. + if not code: + reason = "no project_code on row" + elif not by_code: + reason = f"no site has project_code={code!r}" + elif not veh: + reason = "no vehicle on row" + elif veh not in vehicle_site: + reason = f"vehicle {veh} not found" + elif not by_vehicle: + reason = f"vehicle {veh} has no site" + else: + reason = "SOURCES DISAGREE" + skips.append((r["id"], code, by_code or "-", by_vehicle or "-", reason)) + + print(f"\nwill write : {len(writes)}") + print(f"will skip : {len(skips)}") + for s in skips[:20]: + print( + f" SKIP id={s[0]} project_code={s[1]!r} " + f"by_code={s[2]} by_vehicle={s[3]} ({s[4]})" + ) + if len(skips) > 20: + print(f" ... and {len(skips) - 20} more") + + if not APPLY: + if skips: + print( + f"\nNOTE: {len(skips)} row(s) would be skipped. The plan " + f"expects 0. Report these before applying - a disagreement " + f"means the spec's assumption is wrong for those rows." + ) + print("\nDRY RUN. Re-run with --apply to write.") + return 0 + + # Per-record write failures are tracked apart from the data-quality + # skips above. A row we chose not to touch and a row we tried to touch + # and failed on are not the same fact; counting the latter as the + # former makes a half-broken run read as a clean one. + errors = [] + for i, (rec_id, site_id) in enumerate(writes, 1): + try: + api( + base_url, token, "PATCH", + f"/api/collections/fuel_logs/records/{rec_id}", + {"site": site_id}, + ) + except Exception as e: # noqa: BLE001 - report, never swallow + errors.append(f"{rec_id} ({e})") + if i % 25 == 0: + print(f" wrote {i}/{len(writes)}") + + for n in errors[:20]: + print(f" ERROR {n}") + + remaining = [ + r + for r in get_all(base_url, token, "fuel_logs", "id,site") + if not rel_id(r.get("site")) + ] + print( + f"\nwrote {len(writes) - len(errors)}/{len(writes)} " + f"write errors: {len(errors)}" + ) + print( + f"fuel_logs still missing site: {len(remaining)} " + f"(expected {len(skips)} = the rows we deliberately skipped)" + ) + + if errors: + print(f"\nFAILED: {len(errors)} record(s) did not write. Re-run.") + return 1 + if len(remaining) != len(skips): + print("\nFAILED: post-run count does not match the plan. Investigate.") + return 1 + return 0 + + +if __name__ == "__main__": + try: + sys.exit(main()) + except KeyError as e: + print(f"ERROR: missing environment variable {e}", file=sys.stderr) + sys.exit(2)