fix(scripts): backfill legacy NULL amount_eur rows from the cron bug window (#158 follow-up) - #264
Draft
lyubomir-bozhinov wants to merge 10 commits into
Draft
fix(scripts): backfill legacy NULL amount_eur rows from the cron bug window (#158 follow-up)#264lyubomir-bozhinov wants to merge 10 commits into
lyubomir-bozhinov wants to merge 10 commits into
Conversation
…-bg#158 follow-up) Ground truth = the real refresh-slice derive WITH rates; damaged twin = the same corpus derived WITHOUT rates (the bug-window state: NULL amount_eur AND mis-classified value_flag - a 300x-estimate contract falls through to 'ok' because eff_eur was NULL). The backfill must bring every EUR column, flag, and rollup table to byte-equality with ground truth. Fails on this commit: scripts/backfill-fx.mjs does not exist yet.
The cron bug window (midt-bg#158) left served foreign-currency contracts with amount_eur = NULL and misresolved fx-dependent value_flag branches. backfill-fx.mjs repairs them in place: loads missing fx_rates through the shared FX module (host-pinned, fail-closed), recomputes fx_rate, flags, amount_eur and the value_suspect estimate repair with the exact derive semantics, then refreshes only the touched rollups by reusing refresh-slice.sql's own batch groups. Report mode (default, exit 1 on damage) vs --apply; idempotent. runSliceDerive in import.mjs now runs the same assertFxPopulated gate as the full derive, with the failure message pointing at the backfill. Equivalence suite proves byte-identical output to a ground-truth derive across contracts, flags and every rollup table; mutation-tested. ADR-0008 records the targeted-repair-vs-re-derive decision and residuals.
arg() only parses --work-db=<path>; the bare space form silently became the literal DB path 'true'. Fail loudly instead and document the = form. Found by live CLI validation.
Same .d.mts pattern as integrity-checks: the plain-ESM script gets a declaration so the TS equivalence suite typechecks under strict + noUncheckedIndexedAccess.
Adversarial review HIGH: the repair committed in one exec and the rollup groups in up to seven more — a death in between left contracts repaired but rollups stale, and no re-run could recapture the rows (the touched set keys off the already-cleared damage predicate), silently corrupting public totals. Repair + rollups now go out as a single exec, and the leftover refresh_touched_* tables (cleanup drops them last) are treated as the durable resume signal: detected up front — before the early return and before any fetch — the rollup batches re-run over them. Report mode flags the interrupted state and exits 1. Both resume paths are equivalence-tested and mutation-proven.
…e rollups Adversarial review round 2, both HIGHs fixed: value_flag also depends on the TENDER estimate's currency — a BGN/EUR (or priced foreign) contract whose estimate was in an uncovered foreign currency mis-resolved its value_suspect/review branches with amount_eur perfectly populated, invisible to the amount_eur damage predicate. The backfill now recomputes the fx-dependent flag branches for every such candidate (one shared SQL expression for detect + repair), using the derive's own current-based effective value instead of amount * fx_rate (they diverge on annex_suspect rows). Report mode surfaces candidates it cannot verify offline. runSliceDerive's FX gate moved to the same slot as the full derive's: after the derive groups, BEFORE the rollup groups — a gate after the rollups fired loudly but left the corrupted totals already written. The split shares REFRESH_SLICE_ROLLUP_GROUPS with the backfill (canonical home next to the parser, with a contiguous-tail drift test). Also: the work-db exec path wraps each script in BEGIN/COMMIT (-bail + rollback on error), and a resumed run re-applies the row repair too, so a partial failure inside the repair converges on retry. Corpus grows three adversarial rows (flag-only BGN review/suspect, ballooned-annex suspect); equivalence + mutation coverage extended.
wrangler is a devDependency — bare node invocations have no node_modules/.bin on PATH, so the local/remote D1 paths died with ENOENT. Resolve the workspace binary first, fall back to a global install. Found by live-validating the wrangler runner against a local D1.
The FX cron ADR was renumbered 0007 -> 0029 during upstream review (midt-bg#263); its backfill follow-up takes the next number, 0030, and its cross-references to the FX ADR follow.
…nnex, cohort-stats) Rebasing onto current main surfaced three ways the derive moved on since the backfill was written, each caught by the (strengthened) equivalence suite: - Damage predicate keyed on the ROOT cause, fx_rate IS NULL, not the amount_eur IS NULL symptom. A foreign contract whose headline value is a EUR-denominated annex (current_value_currency='EUR', midt-bg#245) has amount_eur populated at par yet NULL fx_rate/signing_value_eur — the old predicate missed it. Proven: without the fx_rate predicate the euro-annex row diverges from a clean derive. - EUR columns recomputed through the derive's own per-value currency ladder (toEur): amount at trusted_currency, current_value at current_value_currency, signing at the contract currency — not a blanket amount*fx_rate, which double-converted the euro-annex current_value (45000 EUR -> 90000). Proven: breaking the EUR-par branch reddens equivalence. - cohort-stats (cpv_division_stats) added to REFRESH_SLICE_ROLLUP_GROUPS: a full-rebuild percentile rollup over amount_eur that must sit after the FX gate and be refreshed by the backfill. Drift-alarm test + equivalence now cover it. Test fixture applies migration 0002 (current_value_currency) and adds the euro-annex adversarial row. ADR-0030 documents the fx_rate predicate and the per-value conversion.
lyubomir-bozhinov
force-pushed
the
fix/158-fx-backfill
branch
from
August 3, 2026 13:29
834bee3 to
a376c83
Compare
The derive's value_low "< 1000 EUR signed AND < 5% of the estimate" branch reads the contract's own estimated_value / procurement_currency from raw staging, which the served contracts table does not carry. A repaired tiny foreign row that should be value_low stays 'ok' — neutral for SUM rollups (value_low sums like ok) but NOT for cpv_division_stats, which filters value_flag='ok': such a row pollutes its CPV percentiles. Cannot be recomputed in place (the raw estimate is not served), so surface it instead of hiding it: - add summary.valueLowUnverifiable — an upper-bound count of repaired 'ok' foreign rows with a tiny (< 1000 EUR) signing value whose value_low classification is undecidable offline; printed by the CLI --apply summary - correct the ADR-0030 residual claim (was falsely "label-only, sums not affected" universally) and the repairRowsSql comment - pin the boundary with a test: the backfill prices amount_eur correctly, leaves the flag 'ok', counts it, and the cpv_division_stats divergence from a full re-derive is asserted so any future change is deliberate Full CLI re-derive remains the path to absolute percentile correctness.
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.
Follow-up на #158 (cron поправката #263 вече е в
main): договорите, derive-нати през прозореца на бъга, седят в served D1 сamount_eur = NULL(извън всички суми) и — по-коварно — с грешно решени fx-зависимиvalue_flagклонове.Какво добавя
scripts/backfill-fx.mjs— report (по подразбиране, exit 1 при щета) /--apply; цели катоload-fx.mjs(--work-db=/ локална /--remoteD1). Тегли липсващите ЕЦБ курсове през споделенияfx.ts(host-pinned, fail-closed) и опреснява само засегнатите rollups през собствените батчове наrefresh-slice.sql— нула дублиран rollup SQL.refresh_touched_*и следващият ги засича и лекува.import.mjs:runSliceDeriveминаваassertFxPopulatedна същото място като full derive-а — след derive групите, преди rollup групите (сплит по споделенияREFRESH_SLICE_ROLLUP_GROUPS, с drift тест).Предикат по коренната причина. Щетата се засича по
fx_rate IS NULLза чужд договор — коренът, а не симптомътamount_eur IS NULL. Това лови и euro-annex класа (#245): чужд договор с EUR-номиниран анекс има коректноamount_eurпо номинал, но NULLfx_rate/signing_value_eur. EUR колоните се преизчисляват през стълбицата на derive-а —current_valueпоcurrent_value_currency(миграция 0002), не наивноamount × fx_rate(което удвояваше 45000 EUR → 90000).Проверка
cpv_division_stats); adversarial редове: euro-annex, BGN/USD-оценка review/suspect, издут анекс.