fix(fuel): backfill site on 147 orphaned fuel logs
147 of 320 fuel_logs had no site relation; their only attribution was the denormalised project_code text, which Task 18 deletes. Backfilled before that column goes, resolving each row twice (by project_code and via the vehicle's own site) and writing only on agreement. 147/147 written, 0 skipped, 0 remaining. Recorded a caveat the dry run exposed: with one site in production both resolvers draw from a candidate set of size one, so "both agree" is guaranteed rather than evidence. The write is still the only possible correct value, but the guard only bites once more sites exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
DEls
parent
5274ac4981
commit
9117b73cd4
2 changed files with 253 additions and 0 deletions
|
|
@ -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`.
|
||||
|
|
|
|||
243
scripts/migrations/2026-09-11-backfill-fuel-log-sites.py
Normal file
243
scripts/migrations/2026-09-11-backfill-fuel-log-sites.py
Normal file
|
|
@ -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)
|
||||
Loading…
Reference in a new issue