Skip to content

fix(etl): never pair a full derive with a partial catch-up window - #270

Merged
todorkolev merged 5 commits into
mainfrom
fix/catchup-derive-mode
Aug 8, 2026
Merged

fix(etl): never pair a full derive with a partial catch-up window#270
todorkolev merged 5 commits into
mainfrom
fix/catchup-derive-mode

Conversation

@todorkolev

@todorkolev todorkolev commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Проблемът

pnpm run import --catchup избираше derive=full, щом празнината надхвърли LARGE_GAP_DAYS (14 дни). Прозорецът на догонването обаче е gap-aware - той покрива само опашката на емисията (maxLoadedDate минус 3 дни lookback).

Пълният derive презижда доменните таблици от staging: scripts/normalize-raw.sql започва с изчистване, а transient staging таблиците се пресъздават празни на всяко пускане и се пълнят само с прозореца. Тоест корпусът се възстановяваше от тези няколко седмици, а всичко по-старо отпадаше и не се връщаше.

Двете половини никога не е трябвало да вървят заедно, точно както е описано в docs/etl.md: пълен derive върви с изричен прозорец през цялата емисия, gap-aware прозорец върви със slice derive. Разминаваше се само клонът „съществуващ корпус + голяма празнина" - при липсващ корпус кодът вече прави правилното (from = DEFAULT_FROM + пълен derive).

Открито наживо: локално догонване с 49-дневна празнина избра derive=full върху корпус от 193 023 договора.

Промяната

  • догонването вече винаги прави slice derive, колкото и голяма да е празнината; изричното --derive=full продължава да важи
  • assertDeriveWindowSafe отказва пълен derive, чийто прозорец не стига до началото на емисията, докато има зареден корпус. Проверката е преди зареждането, не след триенето
  • инвариантът живее в @sigma/ingest като fullDeriveIsSafe, до computeCatchupWindow, с тестове
  • docs/etl.md описва вече действителното поведение
  • LARGE_GAP_DAYS отпада - вече няма кой да го чете

Поправка след ревюто: пазачът допускаше точно загубата, срещу която стои

Първата редакция питаше SELECT COUNT(*) FROM contracts. Изчистването в normalize-raw.sql изпразва четиринайсет таблици.

Значи корпус без договори, но с попълнени tenders, bidders или authorities - точно каквото оставя половинчат пробег - минаваше през пазача, а презиждането ги отнасяше без откъде да се върнат. Пазачът срещу загуба на данни допускаше загуба на данни.

Три неща в поправката:

  • списъкът идва от самия SQL през нов знак @full-clear, вместо да е преписан в JS. Преписът на един рушащ списък е бъг със закъснител - ето го доказателството. Обхватът спира преди data_freshness и pipeline_stats: те се пишат наново всеки пробег и ако се броят за корпус, пазачът отказва всичко, включително първоначалното зареждане, а пазач, който винаги отказва, го изтриват;
  • проверката вали и когато не може да отговори. Липсваща таблица караше safeD1 да върне празно, което се четеше като „няма корпус" - една липсваща таблица заслепяваше пазача за другите тринайсет и пускаше рушащия път;
  • отказът разчиства преходния staging след себе си. По-рано излизаше с изход 1, оставяйки схемата, създадена за пробег, който така и не тръгна. По-рано от това не може да се провери - планът за догонване чете raw_contracts.

Как е проверено този път

Проверката само на предиката е точно каквото пропусна този бъг: fullDeriveIsSafe беше зелена, докато мястото, което я вика, задаваше грешния въпрос.

Затова scripts/import-guard.test.mjs кара истинския import.mjs като отделен процес, с подставени wrangler и node най-отпред в PATH, и твърди за това какво скриптът е направил: отказал ли е, кои таблици е назовал, изчистил ли е след себе си, и тръгнало ли е зареждането. Отделно refresh-full-clear.test.ts държи разбора вързан за истинския SQL файл.

Мутации:

мутация резултат
пазачът пита пак само за contracts 3 падат
знакът @full-clear е махнат от SQL-а 2 падат
разборът връща само contracts 3 падат
разборът признава само голи имена 1 пада
изчистването е пренаписано с кавички 0 падат, и таблицата остава в проверката

Втори кръг ревю: тестът споделяше слепотата на кода

Ревюто счупи пазача с промяна, която в преглед изглежда като форматиране:

-DELETE FROM search_index;
+DELETE FROM "search_index";

Валиден SQLite. Разборът признаваше само голи имена, тъй че таблицата изпадаше от списъка и дупката се отваряше наново - корпус, в който само тя е попълнена, минаваше през пазача и зареждането тръгваше.

По-лошото беше в теста. Той извеждаше очаквания списък от същия SQL файл със свое копие на същия израз, тъй че наследяваше същата слепота: и двата набора оставаха зелени, докато пазачът пропускаше рушащия път. Оракул, който споделя допусканията на кода, не проверява нищо - същата грешка по форма като първата, само едно ниво по-нагоре.

Поправката е в двете посоки: разборът приема и трите форми кавички на SQLite, а списъкът в теста вече е изписан на ръка, с отделна проверка, която държи него и SQL-а един срещу друг. Разминаване вали, вместо да изчезне.

Ревюто потвърди и че шестте искани мутации по производствения код падат, и че самата сглобка с подставените двоични файлове не може да мине празна (пет начина да я счупя дават от 1 до 7 провала).

Второто му намиране: описанието твърдеше, че --catchup винаги прави slice. При първо пускане, когато няма заредени дни, той взима прозорец от началото на емисията и пълен derive. Поправено в описанието.

Обхват

Деплойнатият cron не е засегнат: пуска се на 6 часа, тъй че никога не натрупва такава празнина, а RefreshWorkflow изобщо не вика normalize-raw. Ударът е по локалните и възстановителните пускания, тоест точно когато някой възстановява корпус след срив.

Описанието в docs/etl.md твърдеше, че отказът важи само при изрично --derive=full. Всъщност важи и за подразбиращия се - без --catchup подразбиращият се derive е full, тъй че и import.mjs --from=2026-06-01 получава същия отказ. Поправено в описанието, не в кода: отказът там е правилното поведение.

Проверка

  • scripts/: 86 теста; @sigma/ingest: 59; tsc --noEmit чист
  • предпазителят е пуснат срещу истинската локална база и отказа коректно, преди каквото и да е зареждане
  • след поправката --catchup мина 49-дневен прозорец със slice derive и reconciliation gate-ът мина (5 проверки, 1 пропусната, 1 предупреждение), тъй че slice derive държи и на дълъг прозорец

`--catchup` escalated to `derive=full` whenever the gap exceeded
LARGE_GAP_DAYS (14), but the catch-up window is gap-aware and only covers
the tail of the feed. A full derive rebuilds the domain from staging
(normalize-raw.sql opens with `DELETE FROM contracts`), so the corpus was
rebuilt from those few weeks and every older contract was dropped and never
reloaded.

The two halves were never meant to go together: a full derive belongs with
an explicit, feed-wide window, a gap-aware window with a slice derive. Only
the "existing corpus + large gap" branch mismatched them.

- catch-up now always derives a slice, however wide the gap; an explicit
  `--derive=full` still overrides
- `assertDeriveWindowSafe` refuses a full derive whose window does not reach
  the start of the feed while a corpus exists, before the load rather than
  after the delete
- the invariant lives in @sigma/ingest as `fullDeriveIsSafe`, unit-tested
  alongside computeCatchupWindow

The deployed cron is unaffected: it refreshes every 6 hours so it never
accumulates the gap, and RefreshWorkflow does not run normalize-raw. This
bites local and recovery runs, exactly when a corpus is being restored.
@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Прегледах срещу дифа на връх e3dec09 — бъгът е реален, поправката коректна. Ключовите твърдения ги потвърдих независимо.

Корпусът наистина се трие. normalize-raw.sql:47 е DELETE FROM contracts; (:48 DELETE FROM lots;), после INSERT-ва наново само от staging, а staging се пълни само с прозореца. Стар --catchup при gap > 14 дни → derive=full → целият корпус преди maxLoadedDate − lookback изчезва и не се връща. (Тялото сочи „ред 41" — на текущия връх е 47, изместено от евро-анекс промените на #261; същината е точна.)

Няма #158 регресия — проверих специално. runSliceDerive (import.mjs:268) пуска load-fx.mjs --apply точно както runFullDerive (:255). Тоест catch-up-винаги-slice НЕ връща NULL amount_eur за чуждовалутни договори — FX минава и по двата derive пътя. Важно, понеже #263 затвори точно този теч.

Guard-ът е fail-closed преди зареждане. assertDeriveWindowSafe се вика след wrangler d1 migrations apply (:392) и преди load-eop, тъй че contracts вече съществува → safeD1 COUNT-ът чете 0 за пресен корпус (не удря error пътя) → hasCorpus=false → initial backfill минава чисто. fullDeriveIsSafe е чист и тестван (вкл. malformed→throw); лексикографското сравнение на ISO дати е коректно; отказът е exit 1 преди load, без частично състояние.

Единствената деструктивна комбинация (--derive=full + частичен прозорец + непразен корпус) вече се блокира, преди да изтрие нещо. Одобрявам по същество (Triage — не формален approve).

t added 3 commits August 7, 2026 23:25
Пазачът срещу пълен derive с частичен прозорец броеше редовете в `contracts`,
а изчистването в normalize-raw.sql изпразва четиринайсет таблици. Корпус без
договори, но с попълнени `tenders`, `bidders` или `authorities` - точно каквото
оставя половинчат пробег - минаваше през пазача и презиждането ги отнасяше,
без да има откъде да се върнат.

Списъкът вече идва от самия SQL през нов знак `@full-clear`, вместо да е
преписан в JS: преписът на един рушащ списък е бъг със закъснител. Обхватът
спира преди `data_freshness` и `pipeline_stats` - те се пишат наново всеки
пробег и ако се броят за корпус, пазачът отказва всичко, включително
първоначалното зареждане.

Проверката вали и когато не може да отговори: липсваща таблица правеше
`safeD1` да върне празно, което се четеше като „няма корпус" и пускаше
рушащия път. Отказът вече и разчиства преходния staging след себе си.

Тестовете карат истинския `import.mjs` като отделен процес с подставени
`wrangler` и `node`, защото проверката само на предиката е точно каквото
пропусна този бъг. Мутацията „питай пак само за договорите" вали три теста.

Описанието в docs/etl.md твърдеше, че отказът важи само при изрично
`--derive=full`; всъщност важи и за подразбиращия се - поправено.
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Test coverage

Workspace Lines Δ Branches Δ Functions Statements
apps/etl 75.43% +1.43pp 63.52% +5.32pp 70.00% 74.11%
apps/web 91.02% +1.32pp 82.50% +0.70pp 91.19% 89.73%
packages/config 92.85% +0.05pp 72.22% +0.02pp 92.85% 89.18%
packages/db 94.54% +0.34pp 79.38% +0.38pp 87.19% 91.55%
packages/ingest 88.17% +2.37pp 82.30% +2.30pp 80.17% 86.36%
packages/shared 95.50% +0.10pp 80.83% +0.83pp 92.30% 89.56%
Total (informational) 91.23% 80.72% 86.95% 89.08%

✅ No workspace dropped below its baseline (tolerance 0.5pp).

📈 Coverage rose by more than 1pp — run node scripts/check-coverage.mjs --update locally and commit coverage-baseline.json to ratchet the threshold up.

Ревюто счупи пазача с промяна, която в преглед изглежда като форматиране:
`DELETE FROM "search_index";` е валиден SQLite, но разборът признаваше само
голи имена, тъй че таблицата изпадаше от списъка и дупката се отваряше наново
- корпус, в който само тя е попълнена, минаваше през пазача.

По-лошото беше в теста. Той извеждаше очаквания списък от същия файл със
свое копие на същия израз, тъй че наследяваше същата слепота: и двата набора
оставаха зелени, докато пазачът пропускаше рушащия път. Оракул, който споделя
допусканията на кода, не проверява нищо.

Затова списъкът в теста вече е изписан на ръка, а отделна проверка държи него
и SQL-а един срещу друг - всяко разминаване вали, вместо да изчезне. Разборът
приема и трите форми кавички на SQLite.

Поправено и твърдението в docs/etl.md, че `--catchup` **винаги** прави slice:
при първо пускане, когато няма заредени дни, той взима прозорец от началото на
емисията и пълен derive.
@todorkolev
todorkolev merged commit 8139adb into main Aug 8, 2026
5 checks passed
todorkolev added a commit that referenced this pull request Aug 9, 2026
Поправката от #277 нямаше нищо, което да я пази: `import.mjs` изпълнява код
при внасяне, тъй че `safeD1` не се тества модулно. Сглобката от #270 обаче
кара истинския скрипт като отделен процес с подставен `wrangler`, и това е
достатъчно.

Подставката е сверена срещу истинския двоичен файл, а не измислена - при
`d1 execute --json` върху липсваща таблица грешката от SQLite излиза на
stdout, известията на stderr, а съобщението на изключението е само
„Command failed". Точно това разминаване изпускаше старата проверка. Наблюдаваният
изход е записан в теста.

Мутациите: връщането на проверката само върху `err.message` вали два теста,
а превръщането на safeD1 в общ капан вали третия.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants