test(db): golden regression dataset for traceable totals - #220
Conversation
|
Проверих отново (
Should-fix (не блокер): инвариантът Nit: Одобрявам; should-fix-ът е малък и си струва преди merge. |
lyubomir-bozhinov
left a comment
lyubomir-bozhinov
left a comment
There was a problem hiding this comment.
Издържано — точно това, което #99 иска. Датасетът е реален: ръчно изведени очаквани стойности, прекарани през истинския production SQL pipeline, всяка value_flag пътека ударена по веднъж. Кръстосах homeTotals.value_eur = 241100 (150100 + 91000, hand-derived) — бие. assertIntegrity върви срещу същата база → golden + reconciliation мрежите са комплементарни. Липсата на --update скрипт е правилният избор — авто-регенерация би благословила точно бъга, който датасетът лови.
Дребно: assertIntegrity се внася по суров relative path извън пакета (../../../scripts/integrity-checks.mjs) — при разместване на scripts/ тестът ще пада с module-not-found вместо assertion; workspace alias/entry point би ги развързал.
c98e4ef to
c137871
Compare
|
Аритметиката на golden set-а е коректна (проверих c1/c6/c8, rollup-ите и NULL c9 срещу пега). Пропуск: euro-annex double-conversion капанът (#245 — 2026 EUR анекс върху BGN договор, Добави 10-ти договор: BGN-подписан с EUR анекс, и assert-ни |
nedda76
left a comment
nedda76
left a comment
There was a problem hiding this comment.
Прегледах fixture-а, golden числата и асъртите. Чист, чисто адитивен PR (нов тест + docs, нула продукционен код) с висока стойност като регресионна мрежа — прекарва синтетичния корпус през реалния derive ред и сравнява с ръчно сметнати абсолютни числа, тъй че мисатрибуция, която пази грандтотала, тук пада (точно каквото integrity gate-ът структурно не хваща).
Проверих на ръка (съвпадат)
- Per-contract: c1 195583÷1.95583=100000, c3 50000×0.9=45000, c4 300×оценка→
value_suspect→repair до 1000, c5 15×→review, c6 100×→annex_suspect→връщане към 100, c8 анекс 1.5×→30000. Всичко излиза. - c9 (граничният случай) е най-хубавото зърно: USD, подписан на 06-12, единствен курс на 06-01;
date(06-12,'-10d')=06-02 > 06-01→ извън 10-дневния carry-forward →amount_eurNULL. И проверява точно правилното следствие: c9 влиза в корпуснитеCOUNT(*)(home/facet/search/freshness), но пада от всеки value SUM и сдвоената му бройка;as_of=06-12. Заковаваamount_eur IS NOT NULLпредиката на шестте rollup-а. - Агрегатите се затварят:
company_totals(B1 160000/4/2 авт.),authority_totals(A1 150100, A2 91000, avg-ите),flow_pairs(A2→B1 60000/2 без c9),home_totals241100,facet_countseu '0' 8/191100. Всичко се сумира.
Бележка (дребно)
- Описанието на PR-а казва „3 изпълнители × 8 договора", а fixture-ът има 9 (деветият е именно c9 — граничният fx случай). Струва си да се оправи текстът, за да не подведе ревюъра, че липсва ред.
Отличен тест. Само преглед — самият мърдж не е мой.
Synthetic 8-contract corpus (all 5 value_flag outcomes, BGN/EUR/USD paths, amendments, state-owned EIK) run through the real production derive order (derive-amendments -> normalize-raw -> promote-amendments -> precompute) against sqlite3, asserting hand-computed absolute numbers at every grain: per-contract amount_eur/value_flag/EUR timeline, company_totals, authority_totals, sector_totals, facet_counts, flow_pairs, home_totals, value_flag distribution, search surface, freshness boundary, plus the reconciliation gate. Sensitivity proven by throwaway mutations: changing the BGN peg fails 5/9 tests; loosening the annex_suspect threshold fails 6/9.
…bg#99) review-testing.md gains the golden-dataset description and the manual update procedure (recompute by hand from the fixture, never copy pipeline output; deliberately no --update script). integrity-gate.md's two blind-spot notes now point at the implemented test.
The file was committed with data rows exceeding printWidth 100; CI gates on `prettier --check .` (midt-bg#88), so format it before the substantive change lands. Pure reformatting, no semantic change.
Add c9 — a USD contract signed one day past the 10-day fx carry-forward window (only USD rate is dated 06-01, signing 06-12), so it derives with amount_eur NULL. It probes two paths the prior fixture left un-exercised: - the 10-day carry-forward at its rejection boundary (widen the window and the row would convert), and - the amount_eur IS NOT NULL predicate on all six value rollups: c9 counts in the corpus COUNT(*) tallies (home/facet/search/freshness) but is excluded from every value SUM and its paired count, and is 'ok' not value_suspect so the suspect KPI is untouched. Dropping the predicate now leaks a NULL into a sum instead of being a no-op. Also pass -bail to readScript so a SQL error inside a .read derive step fails beforeAll instead of exiting 0 (matches the sibling db tests). GOLDEN constants hand-derived and reconciled against the real pipeline (clean_total 241100, 9 contracts, integrity gate green).
…the fixture current The rebase onto main left the dataset unrunnable, and three things had to move before a new case could be added at all: - the fixture built its schema from 0000_init.sql alone, but normalize-raw.sql now writes current_value_currency (migration 0002) — apply every migration in the directory, sorted, the way precompute-cohort.test.ts does, so a pinned subset cannot drift out of the served schema again; - B1's ЕИК 111111111 fails the ЕИК/Булстат control digit that normalize-raw.sql now enforces (midt-bg#195), so the bidder silently fell back to the name-keyed rung and every id, key and rollup shifted. 111111113 carries the correct control digit and restores the "valid ЕИК, private" role the fixture documents; - assertIntegrity is async now; the gate assertion ran against a pending Promise and its queries landed after afterAll had removed the temp DB. c10 is the case itself: a BGN-signed contract with a 2026 EUR amendment. Its current_value is already euros, so amount_eur and current_value_eur must take it as-is — the derive keys the conversion off current_value_currency rather than contracts.currency. Regressing either site yields 150000 ÷ 1.95583 = 76693.78, exactly the halving midt-bg#245 reported in production, and the 1.5× step is legitimate so the row stays 'ok' and IS summed: the wrong number would be published, not suppressed. Verified by mutation — dropping the current_value_currency term from precompute.sql fails 2 of 9, from normalize-raw.sql's trusted_currency 6 of 9. Every dependent constant recomputed by hand: won_eur, spent_eur, sector, facet, flow, home totals, the FTS row count and the freshness tally. docs/review-testing.md said 8 contracts where the fixture had 9; it now says 10 and names both boundary cases.
c137871 to
dee1013
Compare
|
Пребазирано върху Ремонт след пребазирането (тестът падаше в
|
| Регресия | Резултат |
|---|---|
precompute.sql: current_value_eur по валутата на договора |
2/9 червени — current_value_eur 150000 → 76693.78 |
normalize-raw.sql: trusted_currency по валутата на договора |
6/9 червени — халвирането се разлива през amount_eur, won_eur 310000→236693.78, spent_eur 300100→226793.78, секторните и home тоталите |
Ключово за дискриминиращата сила: 1.5× стъпката е легитимна, тъй че редът остава 'ok' и влиза в сумите — сгрешеното число би било публикувано, не потиснато. Правилата за annex_total_suspect пък изискват валутата на анекса да съвпада с тази на договора, тъй че кросвалутният случай не се хваща отникъде другаде.
Всички зависими константи преизчислени на ръка (won/spent/sector/facet/flow/home, FTS брой, freshness).
2) „8 договора" в описанието — @nedda76, поправено
Описанието на PR-а и docs/review-testing.md казваха 8; вече казват 10 и именуват двата гранични случая.
3) -bail в readScript — вече е налице
Добавен беше в c98e4ef заедно с обяснителния коментар.
4) assertIntegrity по суров relative path — оставям го, ето защо
../../../scripts/*.mjs е конвенцията в цялото репо, не изключение на този тест: refresh-slice.test.ts, integrity-checks.test.ts, anomaly-report.test.ts и продукционният apps/etl/src/integrity.ts внасят по същия начин. Въвеждането на workspace alias само тук би било непоследователно и извън обхвата на test-only PR — ако alias-ът си струва, редно е да е отделна промяна върху всичките пет места. Кажи, ако предпочиташ обратното.
packages/db/src/golden-dataset.test.ts: 9/9 зелени. check:docs зелен, prettier чист.
|
@ydimitrof, PR-ът е готов по същество, остана само да го изравниш с Проверих съдържанието срещу текущия Единствената спънка е, че клонът е BEHIND, а branch protection иска главата да е up-to-date преди мърдж. От наша страна не можем да обновим fork-клона (нямаме право върху главата на fork-а), затова молбата е към теб:
Благодаря за търпението! |
4 participants
Какво
Closes #99.
Синтетичен golden корпус (2 възложители × 3 изпълнители × 10 договора), прекаран end-to-end през реалния production derive ред (
derive-amendments.sql→load-nuts.sql→seed-state-owned.sql→normalize-raw.sql→promote-amendments.sql→precompute.sql, огледално наimport.mjsrunFullDerive) срещу sqlite3, с абсолютни, ръчно сметнати очаквани стойности на всяко зърно.Покритие на fixture-а
value_flagизхода:ok,value_suspect(300×估 → repair към估имацията),value_low(signing 0),annex_suspect(анекс 100× signing),review(15×).fx_rates, 10-дневния carry-forward прозорец).derive-amendments→promote-amendments(един suspicious 100×, един легитимен 1.5×, чийтоcurrent_valueвлиза вamount_eur).amount_eurNULL, брои се в корпуснитеCOUNT(*), но пада от всяка value SUM — заковаваamount_eur IS NOT NULLпредиката на шестте rollup-а; (2) евро анекс върху лев договор ([Бъг]: Анекси в евро от 2026 на 1/2 от стойността #245) —current_valueвече е в евро, тъй че не се дели повторно на 1.95583; деривацията тръгва отcurrent_value_currency, не отcontracts.currency.seed-state-owned.sql; name-keyed изпълнител без валиден ЕИК (eik_valid=0); NUTS/settlement обогатяване през OCDS parties + реалнияload-nuts.sql.Асърти (9 теста)
Per-contract recount (id/
amount_eur/value_flag/EUR timeline/fx одит) — зърното, което reconciliation gate-ът (#97) структурно не може да провери;company_totals,authority_totals,sector_totals,facet_counts,flow_pairs,home_totals, разпределение наvalue_flag, search surface,data_freshness, плюс самия integrity gate (двете мрежи са допълващи се).Всяко golden число носи деривацията си в коментар (напр.
195583 ÷ 1.95583 = 100000). Умишлено няма--updateскрипт — регенериране от pipeline-а би благословило точно бъга, който тестът пази. Процедурата за умишлена промяна е документирана вdocs/review-testing.md(„Golden dataset");docs/integrity-gate.mdбележките за blind spot-а вече сочат към теста.Доказана чувствителност (mutation checks, dev-time, reverted)
normalize-raw.sql: 5/9 теста червени.annex_suspectпрага ≥100× → ≥1000×: 6/9 червени.precompute.sql(current_value_eurпо валутата на договора вместо поcurrent_value_currency): 2/9 червени —current_value_eur150000 → 76693.78.trusted_currencyнаnormalize-raw.sql: 6/9 червени — халвирането се разлива презamount_eur,won_eur,spent_eur, секторните и home тоталите.Проверки
packages/db: 192 теста / 28 файла зелени (вкл. новите 9);pnpm testзелен и в 6-те workspace-а.pnpm typecheckзелен;node scripts/check-docs.mjsзелен.Свързано
#97 (gate, чиито граници този тест допълва), #98 (при промяна на стойностната база golden числата се преизчисляват ръчно по документираната процедура), #217 (derive слоят се покрива поведенчески — този PR е този крак).