fix(etl): link namespace-mismatched annexes via a value anchor (#306) - #308
Conversation
…-bg#306) 1,937 EOP annexes (7.2%) don't link to any contract: the annex carries an internal annex-side number (148846, 2886) while the contract carries the buyer's filing number (Д-226, 388-2020), so the exact (unp, contract_number) join drops them out of every annex→contract→company/authority rollup. String normalisation recovers almost none (different namespaces, not dirty strings). Resolve by value: an annex's value_before equals its target contract's signing_value. A resolver in derive-amendments.sql rewrites raw_amendments.contract_number (like the midt-bg#286 УНП bridge) when value_before EXACTLY matches (<0.5 стотинка), currency-guarded, exactly ONE contract on the procedure — measured 99.99% precision on the already-linked corpus (9348/9349). Chain propagation carries one agreed target across annexes sharing the annex-side number, so later steps (whose value_before is the prior cumulative) link too and current_value reflects the last step. Value-ambiguous and no-match annexes are left honestly unlinked. Recovers ~1,379 of 1,937 (71%) at high confidence on the live corpus; downstream joins, promotion, and serving need no change. Slice path (refresh-slice.sql) deferred to a follow-up: its windowed raw_contracts and touched-contracts scoping need a served-table candidate source + touched wiring, so it warrants its own tests — the full path is authoritative and fixes the backlog on the next full rebuild (midt-bg#286 precedent: the slice is best-effort). Tests run the real derive-amendments.sql: single-contract link, multi-lot value disambiguation, chain propagation, ambiguous/no-match left unlinked, currency guard, already-linked untouched, diagnostic counts. @sigma/db 395 green, tsc clean. Plan + real-corpus validation in docs/implementation-plans/306-amendment-contract-namespace-link.md.
2097e57 to
3c6fb34
Compare
Independent validation against the live
|
| Check | PR / plan | Measured on live D1 | ✓ |
|---|---|---|---|
| EOP annexes unlinked (the defect) | 1,937 | 1,937 | ✅ |
| empty cnum / empty unp | 0 / 0 | 0 / 0 | ✅ |
| unp∉tenders / proc-no-contract / none-match | 3 / 15 / 1,919 | 3 / 15 / 1,919 | ✅ |
| Recovery after resolver | 1,563 (80.7%), 374 left | 1,563 / 374 | ✅ |
| Direct unique value-matches | ~1,377 | 1,377 | ✅ |
| Precision (GT blank of 13,804 multi-contract linked) | 9,348 correct, 1 wrong | 9,349 unique → 9,348 / 1 (99.99%) | ✅ |
Proof records verified on real data:
00011-2020-0002(multi-lot):2886chain of 5,value_before56000 → uniquely matches388-2020(not387-2020@64000); last stepvalue_after= 53580.21 →current_value. (Today388-2020sits atannex_count=0— exactly the defect.)00017-2020-0041: annex11725vb 5800 →ОП-3-016/…. ✅00004-2020-0022: annex24035vb 700000 →30. ✅00009-2021-0007: vb 90550 ≠ signing 97350 → correctly left unlinked (honest gap, not guessed). ✅
Shipped test suite amendments-contract-resolve.test.ts: 7/7 green (runs the real SQL). Gate logic (exact cent-match + currency guard + n_match=1 uniqueness + chain propagation with disagreement refusal) is sound and idempotent by construction.
Two boundaries worth a conscious sign-off (both disclosed here, not defects):
- Precision over the ticket's "safe reserve": the issue proposed blanket-linking all 1,012 single-contract procedures by УНП; this PR requires value confirmation even there (e.g.
00009stays unlinked). The right call for a transparency site — a wrong attribution beats an honest gap — but it means the fix is deliberately narrower than the ticket's max-recall option. - Full-rebuild only: the fix lands via
derive-amendments.sql;refresh-slice.sqlis intentionally untouched (candidate-source +refresh_touched_contractswiring would need to change, else go-forward links leave contract totals stale). So the deployed corpus won't reflect the fix until a full rebuild runs, and new go-forward mismatches accumulate until the follow-up slice PR. Consistent with the [Данни]: анексите от OCDS не се свързват с нито един договор (OCID вместо УНП) #286 precedent.
Verdict: does what it claims — 80.7% recovery at 99.99% precision, residual left honestly unlinked. Title's "Fixes part of #306" is accurate. LGTM. 👍
|
Прегледах PR-а на 1. Блокер: резолверът работи СЛЕД prefer-EOP dedup-а и възкресява близнацитеБлокът Построих го и го пуснах през истинския Последиците са две, и втората е спираща:
Тоест PR-ът чупи точно пътя, на който сам разчита. Поправката е разместване, не нова логика: преместих блока 2. Двусмислените анекси не остават несвързаниОписанието и коментарът твърдят, че двусмислените (съвпадащи с 2+ договора) остават несвързани. Разпространението по веригата отменя това: последният Процедура с
Тестът за двусмислие ползва самотен анекс ( 3. Дневните обновявания също трябва да работятОтлагането на slice пътя не е приемливо за нас - искаме поправката да работи и в дневния цикъл, не само при пълно презареждане. Практиката го налага: кронът пуска само slice, а Признавам, че е по-трудно - Какво проверих и е наред
ОбобщениеПървата точка е блокираща и се оправя с разместване. Втората иска или стеснение, или поправено описание. Третата е решение от наша страна: искаме и дневния път. Благодаря за измерването - това, че низовата нормализация е пробвана и отчетена като безполезна (0 / 11 / 19), спести спора дали изобщо трябва стойностна котва. |
nikimilenkov
left a comment
There was a problem hiding this comment.
Обстоен преглед — PR #308 @ 3c6fb34 (поправя част от #306)
Благодаря за този PR — свързването по стойност е правилният ход след като низовата нормализация е измерена като задънена улица, гейтът „точно съвпадение + уникалност + отказ при съмнение" е вярната посока за сайт за прозрачност, а валидацията на @cefothe срещу живата база възпроизвежда всички числа. Прегледът мина по строгия протокол (пет паралелни измерения; всяка находка проследена и възпроизведена през истинските скриптове върху чисто копие на този HEAD), стъпва върху коментарите на @cefothe и @todorkolev без да повтаря находките им — по-долу са независимите потвърждения и новите неща.
Предложение: връщане за промени. Общо на PR-а: блокерът на @todorkolev (потвърден с възпроизвеждане) + 2 нови високи + 7 нови средни + ниски. (Съветодателно — решението е на поддържащите.)
Изпълнени проверки: packages/db 394/395 (единственият неуспех е познатият env артефакт ship-domain.test.ts), packages/ingest 61/61; 7 мутации върху резолвера (2 убити, 4 оцелели, 1 ред-съвместима); четири механични репродукции през реалния derive-amendments.sql (двете находки на @todorkolev + две нови мои); производителност измерена при мащаб 198k/30k и 10×; план-документът прочетен и сверен ред по ред; CI зелен; сканът за сигурност чист (без тайни/URL-и/зависимости; argv-безопасен harness; никакъв нов injection път).
Потвърждения на предишните коментари (не ги повтарям като нови)
- Блокерът на @todorkolev (възкресяване на близнаците) — възпроизведен независимо през реалния скрипт:
annex_count = 2на договор с един анекс. Допълнение към предписанието му: преместването трябва да е над двете #286 диагностики, не само над DELETE-а — иначеocds_annexes_droppedспира да е горна граница на реално изтритите (диагностиката се смята върху старияcontract_number). Проверих и независимостта: резолверът чете самоeop:%редове +raw_contracts, недокоснати нито от моста, нито от DELETE-а — преместването веднага след моста е безопасно, а и седемте нови теста минават с разместения ред (проверено с мутация; нищо обаче не заковава реда в никоя посока — тест за наредбата си струва). Бонус:checkAmendmentTwinsще хване възкресените близнаци като твърд отказ на целия derive — т.е. дефектът се проявява като спиране на pipeline-а, не като тихо двойно броене. - Точка 2 на @todorkolev (двусмислените в верига се закачат) — възпроизведена: C-3 получава
current_value = 600вместо 1100 от анекс, чиято стойност никога не е съвпадала с него. Съгласен съм с предписанието: разпространението да прескача редове сn_match ≥ 2(те носят собствено противоречащо доказателство). - Точка 3 на @todorkolev (slice пътят) — двете страни се оказват верни едновременно, виж НОВА ВИСОКА 1: Worker-ът наистина не изпълнява блока (неговият recall аргумент стои), но CLI slice пътят го изпълнява — с прозоречни кандидати.
- Валидацията на @cefothe — числата се възпроизвеждат; методологическата уговорка към нея е в НОВА ВИСОКА 2.
НОВА ВИСОКА 1 — „Slice path deferred" е фактически невярно: резолверът вече тече по инкременталния CLI път, с кандидати само от прозореца
scripts/import.mjs:301 (runSliceDerive) изпълнява целия derive-amendments.sql — включително резолвера — при всяко --derive=slice, върху преходно staging само с текущия прозорец. Там n_match = 1 означава „уникален в прозореца", не в корпуса: корпусно-двусмислен анекс изглежда уникален в тесен прозорец и се закача, а измерените 99,99% точност са върху пълния корпус и не се пренасят — колкото по-тесен прозорецът, толкова по-висок рискът (обратната посока на обичайното натоварване). Планът твърди обратното на три места (Status; §4; „go-forward анексите остават несвързани до следващия пълен rebuild"). Отделно §4-ият втори аргумент за отлагането е неверен на този HEAD: refresh_touched_contracts вече се попълва и през raw_amendments съединението (refresh-slice.sql:1855-1863), а rollup-ът пали и по EXISTS raw_amendments клона — т.е. „touched wiring" препятствието, както е описано, не съществува. Посока: или блокът да се гейтне за пълен derive (напр. изпълнение само когато staging-ът е пълен), или прозоречната семантика да се приеме изрично, документира и ограничи (напр. кандидати и от сервираната contracts, както планът сам предлага за бъдещия slice resolver).
НОВА ВИСОКА 2 — кандидатите идват от недедупнат кумулативен staging: срив на recall при истински пълен rebuild + грешни връзки през заместени редове
vmatch чете raw_contracts суров, а собственият инвариант на pipeline-а (normalize-raw.sql:1010-1012) казва: дневните EOP емисии са кумулативни — същият договор се повтаря през дните, и дедупликацията до „точно един ред на логически договор" става след derive. Значи COUNT(*) OVER (PARTITION BY amendment_id) брои staging редове, не договори. Възпроизведено през реалния скрипт, в двете посоки:
- Срив на recall (fail-closed): един договор в три дневни bucket-а →
n_match = 3→ отказ. При истински пълен rebuild от суровите емисии почти всеки договор присъства в много дни — правилото отказва масово и обещаните 1 563 връзки не се материализират. - Грешна връзка (fail-open): договор, престейджнат в ден 5 с коригирана стойност 120 000; анексът съвпада само със заместения ред от ден 1 (100 000) →
n_match = 1→ закача се по остаряло число, при чист вид на диагностиката.
Валидацията в описанието не го е видяла по конструкция: изнесеният от живата база корпус е вече дедупнат (по един ред на договор) и е зареден така в staging — т.е. мери резолвера върху вход, който реалният pipeline не му дава. Поправка: дедуп CTE преди съпоставянето (ROW_NUMBER() OVER (PARTITION BY unp, contract_number ORDER BY source DESC, id DESC) = 1 — огледало на правилото „последният кумулативен bucket печели" от normalize-raw), плюс тест с един договор, стейджнат от два eop:contracts:<ден> източника — нито една от сегашните фикстури няма дублиран staging ред.
НОВИ СРЕДНИ
- Разцепена верига: противоречащите direct попадения се прилагат въпреки засеченото несъгласие. Възпроизведено: верига
777с първи анекс → D-1 (вярно) и втори, чиято текуща натрупана стойност случайно съвпада уникално с D-2 → D-2 (грешно).HAVING-ът отказва разпространението, но двете противоречащи си пренаписвания остават — D-2 получава чужд анекс и чужда стойност. Несъгласието в група е доказателство, че съпоставянето по стойност е сбъркало за поне един член — редно е да анулира и direct попаденията на групата, не само разпространението. - NULL-стойностните членове на верига никога не се разпространяват —
unlinkedизискваvalue_before > 0, така че административен/срочен анекс по средата на верига остава на анексния номер. Възпроизведено: верига от 3 →annex_count = 2, а „the whole chain links" в описанието е свръхобещание. Същият клас обяснява и защо сметката на плана не се затваря: 1 379 + 326 изброени остатъка = 1 705 ≠ 1 937 — 232 реда (12% от backlog-а) са точно неназованият класvalue_before IS NULL/≤0. Поправката е евтина: изискването за стойност е нужно на съпоставянето, не на разпространението (групата се дефинира от анексния номер). - Пренаписването влиза в идентичността на реда и това има три остри ръба (
natural_key=am:unp:contract_number:document_number): (а) възпроизведено — резолвиран ред се сблъсква с местен анекс със същияdocument_numberна целевия договор и дедупликацията тихо изтрива истинския анекс от сервираната история (новодошлият печели поORDER BY source DESC, id DESC); (б) по slice пътя престейджнат резолвиран анекс се промотира под стария си ключ → мъртъв дубликат/сирак до следващия пълен rebuild (сервираният DELETE пипа самоocds:%редове); (в) ключовете не са монотонни между ingest цикли — по-късно пристигнал втори договор със същата стойност връща анекса към двусмислие и той сменя обратно ключа си, аannex_countна целта пада. Посока: анексният номер да остане част от идентичността (или изричен integrity assert за колизии на резолвиран срещу местен ключ). - Нулева проследимост на пренаписването. За разлика от #286 (OCID-ът оцелява в
tender_ext_idи вsource), тук анексният номер се унищожава без копие,amendment_contract_resolveсе дропва, а следата е два цели числа в wrangler лог. След пуска никой не може да изброи кои редове са стойностно-свързани — твърдението за 99,99% е непроверимо в продукция, оплакване „този анекс не е наш" е неразследваемо, а integrity gate за брой resolved не може да се напише. Поправка:contract_number_raw+link_methodколони (staging + served, през promote), и броячите вpipeline_statsпо прецедента наcheckStagingReconciliation. - Собственото противоречие на реда се игнорира: анексът носи
contractor_eik, договорите също — възпроизведено закачане на анекс с ЕИК222222222върху договор на111111111(чуждиcompany_totals, чуждаcurrent_value). Едноредов null-толерантен пазач (u.contractor_eik IS NULL OR c.contractor_eik IS NULL OR u.contractor_eik = c.contractor_eik) затваря най-стойностната дупка безплатно — противоречието вече е в самия ред. - Документален клъстер: (а) коментарът в
derive-amendments.sql:128-131описва slice имплементация, която не съществува („runs the same logic but also draws candidates from served contracts") — двойно проверено,refresh-slice.sqlняма нито един resolver ред; (б) прецизността в коментара е „8348/8349", а планът/описанието/валидацията казват 9 348/9 349 — и разликата не е козметична: 8 349 < 9 061 (1%-кандидатите) би обърнало твърдението „exact печели и по recall", т.е. правилната двойка е тази на плана, коментарът носи грешка при препис; (в) планът предписва точно счупения ред („resolver runs after the prefer-EOP dedup" §3) — поправката на реда трябва да пипне и него; (г) §5 препраща къмannex_total_suspect/value_suspect— флагове, които не съществуват на този клон (те са #307, все още в „връщане за промени") — т.е. има незаявена зависимост от реда на сливане: слеят ли се ~23-те новосвързани ≥2× анекса преди #307, влизат в агрегатите нефлагнати. - Тестови пропуски (мутационно доказани): оцеляват мутациите върху отказа при несъгласие (
HAVING), толеранса0.005(разширяване до< 5минава — а върху тази константа стои цялото твърдение за точност),eop:%пазача и> 0филтъра; нито един тест не комбинира резолвера с OCDS редове (точно опасността от блокера), с #286 моста или с promote; няма тест за идемпотентност (декларирана, непазена); диагностиката се идентифицира позиционно („последният 2-колонен ред") и ще се счупи при изискваното разместване.
НОВИ НИСКИ
- Валутният пазач дегенерира при празно-срещу-празно (двете страни стават 'BGN' по подразбиране) — за правило с толкова тесен гейт е по-последователно да изисква изрична валута от двете страни, в духа на „честна празнина пред грешен договор".
amendment_contract_resolveне е в списъка за почистване на преходния staging (refresh.ts:86-100) — прекъснат ход я оставя в D1; водещиятDROP IF EXISTSя самолекува при следващ ход, но чистачът за отказните пътища не я познава.- Двете диагностики не са взаимно допълващи се (втората пропуска
value_before > 0предиката) и нямат заявен праг — всяка друга диагностика във файла казва какво е „здраво". UPDATE-ът на :172 е квадратичен по броя несвързани (SCAN на temp таблицата на ред; измерено 32 ms днес, 2,6 s при 10×) — един редCREATE INDEX ... ON amendment_contract_resolve(amendment_id)между :170 и :172 го маха (измерено 39× при 10×). Застраховка, не днешен проблем.- Предсъществуващо сляпо петно за протокола: анекс, чийто анексен номер случайно съвпада с реален (но грешен) договорен номер, никога не влиза в
unlinkedи не участва в проверката за несъгласие на групата си — не е внесено от този PR, но резолверът не може да го спаси.
Извън обхвата, но си заслужава отделен issue
Измерено покрай прегледа: предсъществуващият rollup на :190-233 е 99,5% от времето на целия скрипт (59,3 s от 61 s при мащаб на корпуса — двете SET подзаявки правят пълен скан на 30k-редовия dedup за всеки от 198k договора). Пренаписване с материализирана индексирана temp таблица дава 59,3 s → 0,28 s при нула разлики по (annex_count, current_value) на всичките 198 123 договора. Отделен PR, но е там, където реално живее времето на batch-а.
Какво беше проверено (чеклист)
- Скан за сигурност чист; нулев нов примитив за автор на анекс (той и днес контролира номера директно) — новата повърхност е само косвено насочване през чужди редове, покрито в СРЕДНА 5 / ВИСОКА 2
- Пакетите изпълнени тук (394/395 + 61/61); 7 мутации; 4 механични репродукции през реалния скрипт
- Блокерът и точка 2 на @todorkolev възпроизведени; преместването проверено като безопасно и тест-съвместимо; уточнена правилната цел на преместването (над диагностиките)
- Числата на плана сверени (вкл. незатварящата се сметка: 232 неназовани реда); 8348-срещу-9348 адюдицирано в полза на плана
- REAL прецизност опровергана като риск (ULP при 1,5e8 е ~150 000× под толеранса; един и същ parser от двете страни); валутният idiom сверен с конвенцията на repo-то; индексното покритие потвърдено (планове през EXPLAIN, линейни; CTE-рескан капанът не хапе)
- Производителност: блокът е 70 ms в 60-секунден скрипт; един латентен квадратичен UPDATE с едноредова поправка
Здравна оценка: 5.5 / 11
Коректност (пълен път) ◐ (ВИСОКА 2, СРЕДНИ 1-3) · Коректност (slice) ✗ (блокерът + ВИСОКА 1) · Сигурност ✓ · Производителност ✓ · Тестове ◐ (истински SQL, но половината пазачи неохранявани — СРЕДНА 7) · Документация/план ✗ (СРЕДНА 6) · Идемпотентност ◐ (в един ход да, между ingest цикли не — СРЕДНА 3в) · Наблюдаемост ✗ (СРЕДНА 4) · Обхват ✓ · Съвместимост с конвенциите ✓ (scratch-таблицата, диагностиките и месторазположението следват прецедента) · Честност на валидацията ◐ (числата реални, но входът на валидацията не е входът на pipeline-а — ВИСОКА 2).
Преди сливане
- Блокерът на @todorkolev — преместване над #286 диагностиките + тест за наредбата + поправка на §3 в плана.
- ВИСОКА 2 — дедуп на кандидатите (rn=1 по прецедента на normalize-raw) + тест с дублиран staging ред + ревалидация на recall/precision върху суров кумулативен staging.
- ВИСОКА 1 — решение за CLI slice пътя: гейт или изрична, документирана и ограничена прозоречна семантика; плюс поправка на плана (Status/§4, вкл. неверния touched-wiring аргумент).
- Точка 2 на @todorkolev + СРЕДНИ 1-2 — трите заедно оформят едно правило: група с каквото и да е вътрешно противоречие (двусмислен член / разцепени direct попадения) се отказва цялата, а разпространението не изисква стойност.
- СРЕДНИ 3-5 — идентичност/проследимост/ЕИК пазач; 4 и 5 са евтини и режат клас бъдещи спорове.
- Ниските и тестовите допълвания — по преценка; редът-с-индекса (НИСКА 4) е безплатен.
Идеята е точната и измерването зад нея е сериозно — точно затова си струва свързването по стойност да стъпи на дедупнати кандидати, да остави следа след себе си и да важи само там, където гаранциите ѝ важат. С блокера преместен, кандидатите дедупнати и провенансът записан това ще е достойният втори етаж върху #286. Благодаря — и на @cefothe за възпроизводимата валидация, и на @todorkolev за двата остри улова.
Move the annex→contract value resolver out of derive-amendments.sql into its own script that runs BEFORE the prefer-EOP dedup, full-derive path only. Running after the dedup resurrected OCDS twins (annex_count=2 on a one-annex contract) and tripped the midt-bg#303 twin gate (todorkolev #1); the slice path's windowed raw_contracts can't answer "unique on the procedure" so it is gated off (nikimilenkov HIGH 1). - dedup cumulative raw_contracts candidates before matching, else a contract in N daily buckets reads as n_match=N (nikimilenkov HIGH 2) - one group rule: refuse a (unp, annex-number) group on any internal contradiction (ambiguous member or disagreeing anchors), voiding even its direct hits; value-less members inherit the agreed target (todorkolev #2, nikimilenkov MEDIUM 1 & 2) - null-tolerant contractor-EIK guard; explicit currency both sides (MEDIUM 5, LOW 1); index the resolve scratch table (LOW 4) - provenance: contract_number_raw + link_method through staging, served amendments (migration 0006), and promote; the raw number also keys the amendment natural_key so a resolved row never collides with a native annex sharing document_number on the target (MEDIUM 3, MEDIUM 4) - sweep the resolve scratch table in transient cleanup (LOW 2) - fix comments and plan §3/§4/§5 (8348→9348, order, slice gating, the midt-bg#307-flag merge-order dependency) (MEDIUM 6) Tests run the real resolver+derive composition: twin-ordering, group contradiction, cumulative-dup, value-less inherit, EIK, exact-cent tolerance, gate, idempotency, natural-key collision, provenance.
Review addressed — pushed
|
Live-corpus validation showed the whole-group-refusal rule (nikimilenkov MEDIUM 1) is net-negative: the sole anchor-disagreement group in the entire corpus is a legitimate lot-base annex number spanning two lots (00026-2020-0027 → …-Л01 @ 22569.98, …-Л03 @ 28557.50), each annex exactly-uniquely matching its own lot. Refusing it dropped 2 confirmed-correct links and prevented zero wrong ones. Revised rule: a member's own unique exact match always applies; disagreement withholds only propagation to no-own-match members, never the direct hits; ambiguous members still never link (todorkolev #2). Recovery back to 1,563 (1,377 direct + 186 propagated), precision 9,348/9,349 unchanged.
Live-corpus validation (
|
| Check | plan | live sigma-dev |
✓ |
|---|---|---|---|
| EOP annexes unlinked | 1,937 | 1,937 | ✅ |
| Recovery (1,377 direct + 186 propagated) | ~1,563 | 1,563 | ✅ |
| Precision (GT blank, exact+currency) | 9,348/9,349 | 9,348 / 9,349 (99.99%) | ✅ |
Two things the data corrected:
1. @nikimilenkov MEDIUM 1 (whole-group refusal) was net-negative — revised in e4ad2b4. There is exactly one anchor-disagreement group in the entire corpus, and it's correct:
unp 00026-2020-0027, annex '20РП-У50А015' (a lot-BASE):
value_before 22569.98 → 20РП-У50А015-Л01 (signing 22569.98) ✓
value_before 28557.50 → 20РП-У50А015-Л03 (signing 28557.50) ✓
The annex number is a lot-base shared across two lots; each annex exactly-uniquely matches its own lot. Refusing the group dropped these 2 confirmed-correct links and prevented zero wrong ones (ambiguous_would_inherit_old = 0). So I kept your insight where it pays — a direct exact-cent + unique-on-procedure hit is trustworthy on its own (the 99.99%) — and applied refusal only to propagation: disagreement now withholds inheritance to no-own-match members, but never voids the direct hits. Ambiguous members (n_match ≥ 2) still never link (todorkolev #2). Recovery returns to 1,563; a new test pins both the lot-base link and the withheld-propagation case.
2. The value_before IS NULL/≤0 class is empty — all 1,937 unlinked annexes carry a value, so the 1,379+326≠1,937 gap is tier-estimate rounding, not a value-less class. The value-less-propagation code stays (defensive, correct) but recovers 0 rows today.
Still owed: the served corpus is deduped one-row-per-contract, so this run does not exercise the cumulative-staging dedup (HIGH 2) — that needs a local full backfill on raw cumulative staging. Doing that next.
@sigma/db 408 green, @sigma/ingest 61 green, tsc + prettier clean.
|
Прегледах наново на
Тестовете минават ( Остават две неща. 1. Дневните обновявания - това е изискване от наша странаКоментарът в Искаме поправката да работи и в дневния цикъл. Причината не е предпочитание: кронът пуска само slice, а Възражението „прозоречният 2. Сблъсък на номера на миграции с #307И двата отворени PR-а добавят миграция 0006:
Който влезе втори, ще носи дублиран номер. Понеже сервираните бази се пълнят и извън ledger-а, това е точно мястото, където подредбата тихо се разпада. Който и от двата да мърджнем пръв, вторият трябва да се преномерира преди мърдж - за #308 това значи ОбобщениеМеханиката е наред и доказана. Остава дневният път, който е нашето изискване, и преномерирането. Благодаря за бързия и точен завой по двете находки - изнасянето на резолвера в самостоятелен скрипт е по-добро решение от това, което предложих. |
midt-bg#306) The value-anchor resolver only ran on the full derive, so the cron — which runs only refresh-slice.sql — left new namespace-mismatched EOP annexes unlinked between full rebuilds. Add a corpus-safe resolver inside refresh-slice.sql: candidate contracts come from the served `contracts` corpus UNIONed with the window's raw_contracts, so "unique on the procedure" is asked corpus-wide, not just within the window. It runs before the prefer-EOP dedup (same ordering the full path uses before derive-amendments), carries contract_number_raw + link_method provenance through the served amendments promotion, and keys the amendment natural_key on the raw annex number so slice and full keys agree. Resolved prior-window targets land in refresh_touched_contracts via the existing amendment touch join. Renumber 0006_amendment_provenance.sql -> 0008 to avoid the migration-number collision with midt-bg#307 (0006/0007), assuming midt-bg#307 merges first. Addresses PR midt-bg#308 review (todorkolev): daily-path requirement + migration renumber.
|
@todorkolev Благодаря за прегледа. Адресирах и двете оставащи неща в 1. Дневният път — резолверът вече тече в slice цикълаИзнесох стойностния резолвер в самата
Проверка срещу реалния
2. Номер на миграциятаПреномерирах Тестове: 61 db + 20 etl зелени, вкл. нов Странична бележка: |
…+ test fixes Completes commit 26f9db1, which (due to a partial `git add`) shipped only the migration rename and the new test — not the resolver itself. This adds the rest: - refresh-slice.sql: a corpus-safe value-anchor resolver in the setup batch, before the prefer-EOP dedup. Candidates come from the served `contracts` corpus UNIONed with the window's raw_contracts, so "unique on the procedure" is asked corpus-wide, not just within the window. Carries contract_number_raw + link_method provenance through the served amendments promotion and keys the amendment natural_key on the raw annex number so slice and full keys agree. Resolved prior-window targets land in refresh_touched_contracts via the existing amendment touch join. - import.mjs / resolve-amendment-contracts.sql: comments corrected (the slice path is no longer gated off; the standalone script is the full-derive form). - docs plan §4/§5: slice path marked implemented; migration renumber noted. - Tests: point the 4 migration references at 0008 (the rename); apply the 0008 provenance migration in contractor-identity / etl-entity-canonicalization / value-flag-stotinki (they run refresh-slice.sql, which now writes the provenance columns); whole-array asserts in the slice test for noUncheckedIndexedAccess. Addresses PR midt-bg#308 review (todorkolev): daily-path requirement + migration renumber.
|
Прегледах наново на Проверих slice резолвера, не го приемам по описание: подредбата е вярна (тече след моста и преди prefer-EOP триенето, тъй че близнаците пак си остават грижа само на дедупа), кандидатите наистина идват от сервираната Остават един блокер и едно разминаване. Блокер: миграция
|
…/full on zero-value contracts Two follow-ups from PR midt-bg#308 review (todorkolev): - Blocker: refresh-slice.sql writes contract_number_raw/link_method into served `amendments`, but nothing added those columns to the deployed DB, so the first cron after release would crash. Add an idempotent deploy.yml step (probe pragma_table_info, ALTER only the missing columns, malformed response fatal) before the Worker deploys — mirroring the 0002/midt-bg#307 pattern. The base schema is created out-of-band, so `d1 migrations apply` can't be used. - Discrepancy: the slice resolver asked "is there a matchable candidate" (which requires signing_value > 0), so an annex pointing BY NUMBER to a zero-value contract was treated as namespace-mismatched and could be value-linked to a neighbour — diverging from the full path. Ask a value-agnostic all_contract_numbers CTE instead, so a by-number match always links by number. Regression test added. - Nit: candidate dedup now orders by source-day then id (source DESC, id DESC), matching normalize-raw and the comment. Addresses PR midt-bg#308 review (todorkolev): deploy blocker + zero-value discrepancy + ordering nit.
|
@todorkolev Благодаря за прегледа ред по ред. И трите неща са адресирани в Блокер: колоните вече стигат до базатаДобавих идемпотентна стъпка в Потвърдено на живия — точно както каза: колоните ги няма, значи първият крон след деплой на този клон би паднал без стъпката. С нея деплоят ги добавя преди Worker-ът да тръгне. (Ако #307 влезе пръв, най-чисто е двете колони да се сложат в неговата стъпка вместо да остане тази — отбелязано в кода и в plan §4.) Разминаване: анекс върху нулев договорПрава си — Възпроизведох обхвата на живия корпус: — съвпада с твоите 201/25/0: не хапе в производството днес, но разминаването между двата пътя е затворено. Добавих регресионен тест (Д-1 при 0 и Д-2 при 5000; анекс с номер Д-1 и ДребноДедупът на кандидатите вече подрежда Тестове: |
|
Прегледах на Блокерът е затворен - проверено, не прието по описаниеСтъпката е по образеца на #307: сондира двете колони поотделно, Пуснах истинския сценарий: база от Разминаването с нулевите стойности е затворено
Тоест обединението със сервираната Оправили сте и дребното с деня на източника -
Остава само редът на мърдж. |
nikimilenkov
left a comment
There was a problem hiding this comment.
Прегледах петте нови commit-а до 09e40fe със същата дисциплина: повторих цялата си батерия от репродукции през новите скриптове в реалния им ред (resolve-amendment-contracts.sql → derive-amendments.sql → promote-amendments.sql), плюс пакетите. Всичко от прегледа ми е адресирано; вдигам „връщане за промени" — от моя страна това е одобрение (съветодателно; бележката за реда на сливане с #307 остава на дневен ред, виж края).
Повторени репродукции — всички вече дават верния резултат
| Сценарий (моят оригинален вход) | Преди | На 09e40fe |
|---|---|---|
| ВИСОКА 2а — договор в 3 кумулативни bucket-а | отказ (n_match = 3) |
свързан (дедупът брои договори) |
| ВИСОКА 2б — анексът съвпада само със заместен staging ред | грешна връзка по остаряла стойност | честно несвързан (печели последният bucket) |
| СРЕДНА 2 — административен анекс по средата на верига | annex_count = 2 от 3 |
3 от 3 (безстойностните членове наследяват) |
СРЕДНА 3а — колизия по natural_key изтриваше истински анекс |
само натрапникът оцелява | двата реда се сервират (am:…:7070:N1 и am:…:Д-9:N1 — ключът пази суровия номер) |
СРЕДНА 5 — анекс с чужд contractor_eik |
закачаше се за чужд договор | отказан (link_method NULL) |
| Блокерът на @todorkolev — възкресяване на близнак | annex_count = 2 |
1 (резолверът тече преди дедупа — и над диагностиките) |
| Точка 2 на @todorkolev — двусмислен във верига | закачен, current_value 600 |
несвързан, current_value 1100 |
Плюс: packages/db 410/410 (без познатия env артефакт), дървото чисто, 9348/9349 поправено в новия скрипт, тестове за идемпотентност и за OCDS-комбинацията са налице, gate тестът доказва, че derive сам по себе си не резолвира.
Двете ревизии на мои находки — приемам ги, с данните е трудно да се спори
- СРЕДНА 1 (отказ на цялата група при несъгласие): доказателството от живата база е убедително — единствената несъгласна група в целия корпус е lot-base случай, в който двете direct попадения са верни, т.е. пълният отказ би махнал 2 верни връзки срещу 0 предотвратени грешни. Ревизията в
e4ad2b4(отказ само на разпространението; direct попаденията остават;n_match ≥ 2никога не се свързва) взима моя механизъм там, където плаща, и го оставя там, където вреди. Остатъчният ми синтетичен сценарий (натрупана стойност, съвпадаща случайно с чужд договор в несъгласна група) остава теоретично възможен, но измерено празен днес — честен компромис, записан с тест. - СРЕДНА 2 (незатварящата се сметка): класът
value_before IS NULL/≤0се оказва празен на живия корпус — разликата е закръгляне на оценките по tier-ове, не скрит клас. Кодът за безстойностното наследяване все пак остава (и моята репродукция потвърждава, че работи) — защитно и правилно.
Отделно признание
Двата нови улова на @todorkolev върху свежия код — колоните, които не стигат до сервираната база (същият клас като при #307), и разминаването при нулеви договори — бяха точно каквото прегледът-след-поправките трябва да хваща; проверих деплой стъпката и all_contract_numbers в диff-а и потвърждавам механичните му проверки. Изнасянето на резолвера в самостоятелен скрипт + гейт тестът е по-стабилно решение и от двете ни първоначални предложения.
Оставащо (координация, не дефект)
Редът на сливане с #307: 0008 е верният номер, ако #307 влезе пръв, а двете деплой стъпки ще се допрат в deploy.yml (очакван, дребен конфликт). Ако #308 изпревари — преномериране обратно и разширяване на неговата стъпка, както вече е отбелязано в кода и плана.
Образцов цикъл: три прегледа, пет commit-а, всяка поправка с тест, а двете спорни точки решени с измерване вместо с мнение. Благодаря на всички!
Разрешени конфликти след midt-bg#307 (двойно броене в анексите) и midt-bg#310. deploy.yml — двете колони на midt-bg#306 се сливат в стъпката на midt-bg#307, вместо да се държи втора стъпка. Точно каквото искаше бележката в самия midt-bg#308: „If midt-bg#307 merges first, fold these two columns into its provenance step instead of keeping this one." Една сонда, един механизъм; втора ръчно написана стъпка е още един шанс за грешката, която midt-bg#310 трябваше да оправи. Заявката вече пита за пет колони, а ensure_column се вика пет пъти. Проверено срещу живата база: {value_restated: 1, value_treatment: 1, value_suspect: 1, contract_number_raw: 0, link_method: 0} — трите на midt-bg#307 ги има, двете на midt-bg#306 ще се добавят. promote-amendments.sql и refresh-slice.sql — списъкът с колони в INSERT-а събира и двете страни: contract_number_raw/link_method от midt-bg#306 и value_restated/value_treatment/value_suspect от midt-bg#305. Редът отговаря на SELECT-а, който git вече беше слял правилно. Тестове — всяко от двете подавания добавяше своята миграция към веригата, с която строи схемата. Сега всеки файл, който сервира amendments, прилага и 0006/0007, и 0008; иначе скриптовете падат на липсваща колона от другата страна. Това важи в двете посоки: файловете на midt-bg#305 получиха 0008, а тези на midt-bg#306 получиха 0006/0007. Пълният суит е зелен: db 438, web 482, ingest 84, etl 20, shared 45, config 10. Typecheck и prettier чисти.
|
Слях
тоест трите на #307 ги има, а твоите две ще се добавят на следващия деплой. Пуснах и сондата върху точно този отговор: 0/0/0 за наличните, 1/1 за липсващите - никога 2. (Контекст: първоначалната стъпка на #307 имаше разминаване в псевдонимите и не можеше да мине изобщо; оправено в #310.)
Тестове: всяко от двете подавания добавяше само своята миграция към веригата, с която строи схемата, тъй че след сливането всеки от двата набора падаше на липсваща колона от другия. Сега всеки файл, който сервира Пълният суит е зелен: Мърджвам. |
todorkolev
left a comment
There was a problem hiding this comment.
Одобрявам на c25a8c2d. Двете находки от последния кръг са затворени и проверени срещу истинските скриптове: деплой стъпката вече добавя колоните (пуснах я върху база без тях - refresh-slice минава), а разминаването с нулевите стойности е закрито и в по-трудния случай, където целевият договор е само в сервирания корпус.
…ара за 0010 Преномериране: midt-bg#307 взе 0006/0007, midt-bg#308 взе 0008, тъй че тези две се местят на 0009 и 0010. Обновени са всички препратки - тестове, deploy.yml, related-persons-data.yml, scripts/cacbg/load.mjs, docs/deploy.md. Коментарът над прилагането на 0010 в deploy.yml беше останал от предишния замисъл и твърдеше три неверни неща: че 0003 обявявала CHECK-овете (тя е върната непокътната и не обявява нищо), че се слагало control_hash NOT NULL (остава NULL-ируема нарочно - регистърът я пропуска за част от декларациите, а индексът по естествен ключ ги сгъва с COALESCE) и че миграцията е „rebuild-based" (вече е тригерна; заглавието ѝ обяснява защо пресъздаването е опасно - оголва външните ключове и обира доказателствените печати). Последното беше най-неприятно: обещаваше на следващия четец точно операцията, която самата миграция отхвърля, а „0003 declares them now" щеше да го прати обратно да добавя CHECK-ове в приложена миграция. docs/deploy.md вече изброява и стъпките, които сондират таблицата и добавят само липсващите колони, за да не изглежда, че --file е единственият механизъм. db 424 зелени, typecheck и prettier чисти, scripts/tr 134/134.
Преномерирането смени пътищата, но остави имената migration6Path/migration7Path, което е подвеждащо и се сблъсква с едноименните променливи, които midt-bg#307 и midt-bg#308 въведоха за 0006/0007/0008.
Разрешени конфликти след midt-bg#307, midt-bg#310 и midt-bg#308. Всички са от един и същ вид: и двете страни добавяха своя миграция към веригата, с която тестът строи схемата, и понякога под едно и също име на променлива. Сега всеки тест, който сервира amendments или пуска refresh-slice/ normalize-raw, прилага цялата верига - 0006/0007 (стойност на анекса), 0008 (провенанс) и 0009 (доказателствен печат). Иначе всеки набор пада на липсваща таблица или колона от другия. Пълният суит е зелен: db 474, web 493, ingest 84, etl 20, shared 45, config 10. Typecheck и prettier чисти.
Fixes part of #306.
Problem
1,937 EOP annexes (7.2%) don't link to any contract. #286 fixed the OCDS/procedure axis; this is the contract-number axis: the annex carries an internal annex-side number (
148846,2886) while the contract carries the buyer's filing number (Д-226,388-2020). The exact(unp, contract_number)join fails, so the annex drops out of every annex→contract→company/authority rollup. String normalisation recovers almost none of them (measured:TRIM+UPPER0, digits-only 11, substring 19) — they are genuinely unrelated identifiers.Fix
Link by value: an annex's
value_beforeis the contract's value at amendment time, so it equals the target contract'ssigning_value. A resolver inscripts/derive-amendments.sqlrewritesraw_amendments.contract_numberin place (exactly like the #286 УНП bridge) whenvalue_before:A chain of annexes shares one annex-side number but only the first carries
value_before = signing_value(later steps carry the running cumulative), so once any sibling resolves, that target is propagated across the(unp, annex-number)group (refused if members disagree) — so the whole chain links andcurrent_valuereflects the last step. Value-ambiguous (matches 2+) and no-match (target not yet ingested — the #249 class) annexes are left unlinked: an honest gap beats a wrong contract. Downstream (rollup,promote-amendments.sql, serving join) needs no change; the block is idempotent.Validated on the real corpus
Exported the live corpus (26,770 EOP amendments, 198,123 contracts), loaded it into ETL staging, and ran the actual
derive-amendments.sql:annex_countstable00011multi-lot chain → all 52886annexes to388-2020(cur = last step 53580.21);00017→ОП-3-016/…;00004→30The single wrong link (0.011%) is the irreducible value-collision: an annex whose value had drifted to exactly a sibling contract's signing value — which is why the gate is exact-match + unique.
Scope (deliberate)
00009-2021-0007(value_before90550 ≠ signing 97350) is left unlinked rather than guessed. A max-recall variant (link single-contract procedures regardless of value, minus known multi-lot) is possible but unverifiable — deferred.refresh-slice.sql's windowedraw_contractsand touched-contracts scoping need a served-table candidate source + touched wiring, so it warrants its own PR. The full rebuild is authoritative and fixes the backlog (per the [Данни]: анексите от OCDS не се свързват с нито един договор (OCID вместо УНП) #286 "slice is best-effort" precedent).Tests
packages/db/src/amendments-contract-resolve.test.tsruns the realderive-amendments.sql: single-contract link, multi-lot value disambiguation, chain propagation +current_value, ambiguous/no-match left unlinked, currency guard, already-linked untouched, diagnostic counts.@sigma/db395 green,tsc+ prettier clean. Plan + real-corpus validation indocs/implementation-plans/306-amendment-contract-namespace-link.md.