fix(etl): хващай стотинки грешката и по прогнозата на позицията (#247) - #304
Conversation
Лентата от #298 мери спрямо процедурната прогноза, тъй че вижда грешката само когато процедурата е с една позиция. При процедура с няколко позиции изтърваният десетичен знак пак стои на точно 100x прогнозата НА ПОЗИЦИЯТА, а съотношението към целия процедурен таван пада там, където го отвежда делът на позицията. Докладваният 00621-2020-0008 стои на 89,8x и се сервира като 14 212 416 EUR вместо около 142 000. Само смяна на знаменателя обаче е опасна и docs/etl.md го казва отдавна: при рамковите и unit-price процедури прогнозата на реда е ЕДИНИЧНА цена, а цял call-off законно я надхвърля многократно. Затова рамото е съюз - 95-105x собствената прогноза И поне 10x процедурната, прагът, който вече значи "неправдоподобно голямо за тази процедура". Call-off по единична цена стои на около 1x процедурната и се пощадява. Премерено през прогнозите на позициите: 7 договора, 40,3 млн. EUR показвани падат на 1,75 млн. Всичките са прегледани поединично и са безспорни - учебници за 2 938 EUR прогноза, показани като 293 778. Докладваният договор няма ред в lots, тъй че не влиза в тази бройка, но аритметиката го потвърждава: 14 212 416 / 100 = 277 970,6 лв, точно прогнозата от доклада. Тестът кара истинските скриптове по двата derive пътя и покрива и трите неща: хваща докладваната форма, НЕ пипа call-off при 100x единичната цена, и заковава пода на съюза - точно 10x минава, 8,9x не.
Test coverage
✅ No workspace dropped below its baseline (tolerance 0.5pp). 📈 Coverage rose by more than 1pp — run |
cefothe
left a comment
There was a problem hiding this comment.
Review — стотинки грешката по прогнозата на позицията (#247)
Прегледах на HEAD: security scan + логика на SQL по 5-те флагови места + строгост на теста, с проверка срещу самия branch. CI изцяло зелено. Одобрявам — няма блокер по коректността.
Проверих и ОТХВЪРЛИХ два фалшиви сигнала
- „pipeline_stats копието не е обновено" — невярно. Блокът за брой кандидати (
normalize-raw.sql:1247) носи и новото рамо (:1267), иown_est_eur(:1384);:1394потвърждава, че огледваINSERT INTO contracts. Реконсилиацията е в синхрон. - „parity break от
classifier_estimated_valuefallback-а" — надценено. На петото място проц-лентата95–105×procEst(refresh-slice.sql:1818) стои плътно до own-рамото и засенчва fallback случая, тъй че ред без собствена прогноза се флагва еднакво по INSERT и по UPDATE пътя. Няма видима разлика в флага или в поправената стойност.
Реални бележки (нито една блокираща)
| # | Ниво | Бележка |
|---|---|---|
| 1 | MEDIUM (тест) | expect(row?.value_flag).not.toBe('value_suspect') минава вакуумно, ако сийднатият ред не се вмъкне (row е undefined). Никъде няма expect(row).toBeDefined(), а UNP-UNDER10 няма и amount_eur подсигуровка. Добави проверка за съществуване. |
| 2 | MEDIUM (docs) | docs/etl.md:256 още описва value_suspect само като eff > 200·procEst; нито това own-рамо, нито #298 лентата са в спецификацията, а SQL-ът казва „keep in sync with etl.md". Наследен drift, който този PR разширява. |
| 3 | LOW (покритие) | Всички сийдове са точно на 100× собствената. Непокрити: подовете ≥1000, ръбовете 95×/105×, и случаят с NULL собствена прогноза по UPDATE пътя (единственото място, където пътищата се разминават). |
| 4 | LOW (SQL) | На петото място валутната верига на own_est_eur ползва само cb.currency срещу procurement_currency→currency→BGN в 4-те INSERT места. Латентен full-vs-slice ръб за чуждовалутни редове; безобиден за BGN/EUR. |
| 5 | NIT | Петото място извежда own_est_eur от classifier_estimated_value (пада към процедурната прогноза) вместо да дава NULL като другите 4 — безобидно, но заслужава ред коментар. |
Приета уговорка (в описанието): поправката котвира към procEst, не към позиционната прогноза, тъй че флагнат ред на многопозиционна процедура може да се поправи към леко завишен amount_eur. Разумно отложено.
Потвърдено добро: без нов value_flag enum/миграция (преизползва value_suspect); 5-те копия огледват съществуващия proc_est_eur шаблон (не нов дълг); тестът кара истинските скриптове по двата derive пътя; обхватът е атомарен; без TODO/мъртъв код.
Преди merge бих помолил само за евтиното подсилване на теста (#1: expect(row).toBeDefined() + amount_eur асерция на UNP-UNDER10) и по желание ред в docs/etl.md за стотинки рамото (#2). Останалото е незадължителна политура.
APPROVE.
Бележка 1 (тест минаваше вакуумно): всяко търсене вече твърди, че редът СЪЩЕСТВУВА, преди да твърди нещо за него - иначе `row?.value_flag` върху undefined кара `.not.toBe(...)` да мине без нищо да е проверено. Плюс асерция за `amount_eur` на пощадения ред: важното при него е, че си запазва парите. Бележка 2 (документацията): docs/etl.md описваше `value_suspect` само като `eff > 200 * procEst`. Сега изброява и трите повода, включително стотинки лентата в двете ѝ форми - спрямо процедурната прогноза (#298) и спрямо прогнозата по позиция (#247) - и обяснява защо второто рамо иска и `eff >= 10 * procEst`. Добавен е и `annex_total_suspect` от #305, който липсваше в подредения списък, а редът в него има значение. Бележка 3 (покритие): нови случаи за двата ръба на лентата (95x и 105x влизат, 94x и 106x не), за двата пода от 1000 EUR поотделно, и за ред без собствена прогноза. Всеки от тях убива мутация: 95→90, 105→110, махане на който и да е от двата пода вали по два теста. Бележки 4 и 5 (реконсилиационният пас): собствената прогноза там се вадеше от `classifier_estimated_value` (който пада към процедурната) и се превръщаше по валутата на ДОГОВОРА. Сега е като на четирите INSERT места - собствената прогноза на реда, NULL когато няма такава, превърната по СВОЯТА си валута (`procurement_currency → currency → BGN`) с датирано fx търсене. Валутната част има видима цена: прогноза в евро при договор в лева се делеше на 1,95583 и падаше извън лентата, тъй че същият договор получаваше различен флаг според това кой път го е пипнал последен - и се показваше с 51 129 EUR вместо с 500 000. Има тест, който го заковава. Липсата на fallback-а е изчистване на смисъла, не промяна в поведението, и това е проверено: мутация, която го връща, не вали нито един тест. Причината е, че при fallback собственото рамо пали точно там, където процедурната лента и без това пали. Записвам го както е, а не както би било по-ласкателно. Премерено наново върху корпуса след #307: същите 7 договора, 40,3 млн. EUR показвани, 1,75 млн. след поправката. Осмият, който лентата хваща, вече е `value_suspect` по правилото за 200x - тоест рамото се съгласява с предшественика си там, където се застъпват. Договори, палещи едновременно това рамо и новия `annex_total_suspect`: нула - но и целият корпус още няма нито един `annex_total_suspect`, защото флагът се пълни при стейджване на анекса, а кронът пуска само прозореца. Тоест въпросът за предимството остава непремерен до пълен derive; по конструкция `value_suspect` печели, защото е първи в CASE-а.
|
@cefothe благодаря - и петте бележки са затворени, плюс сливане с 1 (тест минаваше вакуумно)Всяко търсене вече твърди, че редът съществува, преди да твърди нещо за него. Добавена е и асерция за 2 (документацията)
3 (покритие)Нови случаи за двата ръба (95x и 105x влизат, 94x и 106x не), за двата пода от 1000 EUR поотделно, и за ред без собствена прогноза. Всеки убива мутация: 95→90, 105→110, махането на който и да е от двата пода вали по два теста. Проверих и че петте копия са покрити - махането на рамото само от 4 и 5 (реконсилиационният пас)Права беше и за двете. Сега е като на четирите INSERT места: собствената прогноза на реда, Валутната част се оказа с видима цена, като се стигне до нея: Има тест, който го заковава - и той струваше най-много работа, защото реконсилиацията запазва флага от INSERT пътя, освен ако договорът пали и анексно условие. Тоест собственото рамо там решава нещо само в този ъгъл; първите ми два опита за тест минаваха и с мутацията, докато не сложих анекс, който наистина кара паса да преизчисли. За бележка 5 казвам го както е: липсата на fallback-а е изчистване на смисъла, не промяна в поведението. Мутация, която го връща, не вали нито един тест - точно по причината, която описа: при fallback собственото рамо пали там, където процедурната лента и без това пали. Премерено наново след #307Същите 7 договора, 40,3 млн. EUR показвани, 1,75 млн. след поправката. Осмият, който лентата хваща, вече е Договори, палещи едновременно това рамо и новия Зелено: |
Единственият конфликт е познатият: новият тест на midt-bg#304 строи схемата без 0009, а refresh-slice.sql чете interest_link_evidence. Добавена е миграцията в неговата верига. db 487, web 493, ingest 84, etl 20, shared 45, config 10. Typecheck и prettier чисти.
Довършва #247. Първата половина беше #298; това е втората.
Проблемът
Лентата от #298 мери спрямо процедурната прогноза, тъй че вижда изтървания десетичен знак само когато процедурата е с една позиция. При процедура с няколко позиции грешката пак стои на точно 100x прогнозата на позицията, а съотношението към целия процедурен таван пада там, където го отвежда делът на позицията.
Вторият договор от доклада,
00621-2020-0008, е точно такъв: стои на 89,8x процедурната прогноза, минава под лентата и се сервира като 14 212 416 € при реални около 142 000 €.Защо не просто смяна на знаменателя
Защото
docs/etl.mdотдавна записва обратното, и с основание: при рамковите и unit-price процедури (гориво, храни, лекарства) прогнозата на реда е единична цена, а цял call-off законно я надхвърля десетки пъти. Флаговете „твърде високо" ползват процедурната прогноза именно за да не изядат такъв договор.Затова новото рамо е съюз:
Десетократното не е ново число - това е съществуващият праг за
review, който вече значи „неправдоподобно голямо за тази процедура". Call-off по единична цена стои на около 1x процедурната и се пощадява.Ефект
Премерено през прогнозите на позициите: 7 договора, 40,3 млн. € показвани падат на 1,75 млн. Прегледани са поединично и всичките са безспорни:
Учебници с прогноза 2 938 € показани като 293 778 € - сигнатурата е недвусмислена.
Две уговорки, за да е честно измерването:
00621-2020-0008не е в таблицата, защото няма ред вlotsи заместителят, с който мерих, не го вижда. Аритметиката обаче го потвърждава: 14 212 416 / 100 = 277 970,6 лв, което е точно прогнозата, цитирана в доклада. Тоест реалният брой засегнати е над 7 и ще се уточни при зареждане.Реализация и тестове
Ново
own_est_eurв петте флагови пътя (2 вnormalize-raw.sql, 3 вrefresh-slice.sql), плюс рамото в самите CASE-ове. В reconciliation паса се ползва вече наличнатаclassifier_estimated_value- собствената прогноза на реда с процедурната като резервен вариант.Тестът кара истинските скриптове по двата derive пътя срещу истинска sqlite и покрива и трите неща:
packages/dbминава изцяло (единственото падане в средата е предсъществуващият артефакт с глобалния wrangler вship-domain.test.ts, файл, който не е пипан).Какво остава извън обхвата
Поправката връща стойността към процедурната прогноза, установената котва за
value_suspect. При тези договори тя е малко над позиционната - за докладвания случай 158 347 € вместо 142 124 €. По-точно би било да се връща към позиционната, когато е пламнало позиционното рамо, но това е втора котва на пет места и заслужава отделно решение.