fix(etl): correct/flag amendment value double-counts (#305) - #307
Conversation
…ct (midt-bg#305) Tier 1 of midt-bg#305: an annex that announces a new TOTAL contract value has that total put in ЦАИС ЕОП's change field, so currentContractValue = before + newTotal — the value doubles. A single amendment cannot legally >2x a contract (ЗОП чл.116 caps at +50%), so a driving annex with value_after >= 2*value_before (and value_before ~ signing_value, same currency, <10x, tied to current_value) is a data defect. New value_flag 'annex_total_suspect' threads through the value fallback exactly like annex_suspect: these contracts fall back to signing_value and drop out of every EUR aggregate (amount_eur, current_value_eur), in both normalize-raw.sql and refresh-slice.sql; details.ts renders the suspect note. Diagnostic count added. Scope: high-precision same-currency first-annex doublings — ~310 contracts on the live corpus, catches the issue's 145652 and 189325. Deliberately NOT covered (deferred to Tier 2, which needs the основание free text): currency-change doublings like 84818 (exactly 2x EUR annex on a BGN contract, data-indistinguishable from a legitimate cross-currency case) and multi-annex-chain doublings where value_before != signing. 393/393 db tests green, no regressions.
midt-bg#305 Tier 2) Tier 2 restates the double-counted value from the основание free text instead of only flagging it. packages/ingest/src/amendment-total.ts classifies each EOP annex by the preposition in front of the figure — 'на <N>'/'обща стойност … <N>' = a new TOTAL (double-count → corrected value_after = N), 'с <N>' = a genuine increment (value is correct → do NOT flag), currency re-denomination ('лева в евро' + exact 2x) → corrected value_after = value_before. Validated against real основание text (145652, 189325, 79382, 113291, and the false-positive controls 108677, 103903, 84818). base.ts sets raw_amendments.value_treatment / value_after_restated at ingest; derive/normalize/ promote/refresh-slice use COALESCE(value_after_restated, value_after) for current_value and served amendments, and exclude value_treatment-set rows from the Tier-1 annex_total_suspect flag (fixes the 108677-class false positive). Migration 0006 adds value_restated/value_treatment to served amendments; details.ts exposes a 'restated' marker. Untreated doubles (no text signal, e.g. 84818/103903) still fall to the Tier-1 flag. @sigma/db 400 + @sigma/ingest 71 green.
…ETL runs under plain Node (midt-bg#305) load-eop.mjs/import.mjs import @sigma/ingest source directly under plain Node, which requires explicit .ts extensions on imports within that graph (base.ts → './amendment-total.ts'). The extensionless import passed vitest (esbuild) but broke the real ETL with ERR_MODULE_NOT_FOUND. Enable allowImportingTsExtensions in the ingest tsconfig so tsc (Bundler resolution) accepts the same specifier the runtime needs.
…value (midt-bg#305 Tier 2b) When value_after ≈ 2×value_before, the source's 'difference' field merely echoed the OLD value, so the value is unchanged and was doubled onto itself (currency re-denomination or a non-value administrative annex). ЗОП чл.116 caps a single amendment at +50%, so an exact +100% is a definitional defect, not a real increase — restate value_after to value_before. Text-free and currency-agnostic; the genuine- increment check ("с <delta>") runs first, so a legitimately-announced increase is never mis-restated. Generalises the former currency-only rule (renamed treatment 'unchanged_restated'). Real-corpus coverage of the 686 doubled annexes rises 25%→54% (362 corrected + 10 exonerated); all three named records now resolve, incl. 84818 (76.77M EUR annex on a BGN contract). @sigma/ingest 72 + @sigma/db 400 green.
…idt-bg#305 precision) The bare-'на' branch of the total detector matched Bulgarian 'в размер на <N>' ("in the amount of N"), 'ресурс … в размер на N', and 'допълнителни … на обща стойност N' — which name the CHANGE/added-work amount, not the new contract total. Restating value_after := N there understated genuine >100% increases and rewrote wrong numbers (verified on the real corpus: ~65 of 159 total_restated were such false positives, incl. unit-price rows). Add a wider TOTAL_VETO (в размер / ресурс / допълнителн / увеличени / намалени) over a ~55-char window. Real feed: total_restated 159→94, 'в размер на' contamination 69→4; all legitimate 'от X на Y' / 'възлиза на Y' totals retained (145652, 79382, 113291). Demoted rows fall to the Tier-1 arithmetic flag (they are ≥2×), so aggregate coverage holds while correction precision jumps.
…s the doubled value in the timeline (midt-bg#305) annex_total_suspect is contract-level, so a flagged contract's served amendment row still showed the doubled value_after in the timeline. Add a per-amendment value_suspect column (migration 0007), set at promote time (promote-amendments.sql + refresh-slice.sql) for an untreated annex whose value_before ≈ signing_value and value_after in [2x,10x) with matching currency — the same arithmetic gate, at row grain. details.ts blanks valueAfterEur/deltaEur for a suspect row; contract.tsx renders '— непотвърден тотал'. Text-corrected (value_restated=1) and genuine-increment rows are never suspect. @sigma/db 404 + ingest 73 green, tsc clean.
…hor (midt-bg#305) The annex_total_suspect gate only fired when a doubled annex's value_before matched the contract's signing_value, so it caught single-annex doubles but missed a later-in-chain annex whose value_before is the prior CUMULATIVE total (a preceding annex's value_after). Relax the anchor across all gate sites to value_before ≈ signing OR ≈ any recorded prior annex value_after. The ≥2× single-step gate (ЗОП чл.116 caps one amendment at +50%) remains the defect signal, so slow legitimate multi-annex climbs — whose steps never reach 2× — stay untouched. Applied consistently to the contract-level flag (normalize-raw, refresh-slice, and the has_double CTE) and the per-row value_suspect marker (promote-amendments and refresh-slice). Adds positive/negative regression fixtures that fail against the pre-change SQL.
…he CI gates (midt-bg#305) The prior-annex anchor from 6709c57 over-fired on compounding chains where every annex doubles (fixture UNP-SLOW8: signing→2x→4x→8x): a later step could anchor its value_before to a preceding annex value_after that was ITSELF a double. Require the anchoring prior total to have been reached legitimately (prev.value_after < 2x prev.value_before), so only a single >=2x step sitting on a genuinely-grown base is flagged. Applied across all gate sites. Also green the remaining CI gates that were red on this branch: - typecheck: hoist allowImportingTsExtensions into tsconfig.base.json so every consumer of the @sigma/ingest source graph (apps/etl, packages/db) accepts base.ts's './amendment-total.ts' import; drop the now-redundant local copy. - lint: prettier-format the new test fixtures. - docs integrity: index the midt-bg#305 implementation plan in docs/README.md.
cefothe
left a comment
cefothe
left a comment
There was a problem hiding this comment.
Review — parallel specialized agents + manual verification
Phase 0 security scan: CLEAN. No secrets, no new dependencies, only external URL is this repo's own issue tracker. execFileSync('sqlite3', [...]) is test-harness code using array args (no shell injection). All 5 CI checks green.
Verdict: no blocking issues. This is a careful, conservative, genuinely well-tested change — the DB tests run the real SQL scripts against a real SQLite through the actual migration chain and pipeline order, and assert full-rebuild vs. slice-refresh parity. The design fails safe: when uncertain it flags-and-excludes or restates to a lower bound, so residual error trends toward under-correction, not corruption.
Two initial agent findings that did NOT survive verification
- "CRITICAL: absolute tolerance
ABS(am.value_after - c.current_value) < 0.01" — false positive.current_valueis a direct copy ofCOALESCE(value_after_restated, value_after)from the driving amendment (derive-amendments.sql:149-155), no arithmetic, so the tie is exact. Absolute0.01is actually more correct here. - "HIGH:
prevself-join can matchamitself" — false positive. Self-match needsprev.value_after < 2×prev.value_before(normalize-raw.sql:980) while the outer gate needsam.value_after >= 2×am.value_before(line 983) — mutually exclusive.
Actionable findings
1. MEDIUM — Rule 3 (exact-2× restatement) rewrites value with no textual evidence and no ЗОП-exemption check. amendment-total.ts:124-126 fires whenever value_after ≈ 2×value_before, justified by the чл.116 +50% cap — which doesn't bind outside_zop contracts or multi-lot aggregates. A genuinely-doubled legal contract whose основание lacks "с <delta>" would be silently restated. The outside_zop field already exists on the row (base.ts:335). → fixing: thread outsideZop into the classifier and skip Rule 3 when set.
2. MEDIUM — the restated marker is plumbed through the API but never rendered. details.ts:697 sets it and api-contract/src/index.ts:285 declares it, but contract.tsx never references a.restated. A silently text-corrected value gives the user no indication SIGMA rewrote it — a transparency gap. → fixing: render a marker on the corrected row.
3. LOW/MEDIUM — full-vs-slice "latest amendment" tiebreak diverges. derive-amendments.sql:155 orders published_at DESC, natural_key DESC; the refresh-slice.sql rollup orders published_at DESC, id DESC. On annexes sharing published_at the two paths can pick different driving amendments. → fixing: align on id DESC.
Minor (LOW)
- Tautological parity assertions (
amendments-total-restated.test.ts:297-298,amendments-total-suspect.test.ts:300-301):expect(full?.X).toBe(slice?.X)after both asserted against a concrete value — passes trivially on dual-undefined. - No DB test for a ≥2× legitimate annex arriving with
value_treatment=NULL(heuristic miss) — it gets flagged (safe direction), but worth pinning. normalizeBgNumbercan't parse dot-thousands1.234.567,89— silent miss (falls to arithmetic flag), acceptable under the conservative design.- Migrations 0006/0007 additive & nullable/defaulted — safe for D1.
Findings #1–#3 and the tautological-assertion nit are being fixed in follow-up commits on this branch.
…acts (midt-bg#305) The rule-3 fallback restates any value_after ≈ 2×value_before to value_before with no textual evidence, justified by ЗОП чл.116's +50% single-amendment cap. That cap does not bind contracts procured outside ЗОП (isExceptionContract), where a genuine +100% is legal — so a text-free restatement there could erase a real doubling. Thread outsideZop through classifyAmendmentValue and gate rule 3 on it; the text-confirmed rules (total / genuine-increment) are unaffected since the double-count is a feed defect independent of ЗОП scope. Untreated exception doubles fall to the arithmetic flag (exclude, don't rewrite).
…#305) The incremental rollup picked the driving amendment with ORDER BY published_at DESC, id DESC while the full-rebuild path (derive-amendments.sql) uses natural_key DESC. On two annexes sharing published_at the two paths could select different rows and diverge on current_value / value_flag. Match the canonical natural_key tie-break in both the current_value and current_value_currency subqueries.
…g#305) The 'restated' flag was plumbed through details.ts and the API contract but never rendered, so a value СИГМА silently corrected from a doubled total showed no indicator. Render a 'коригиран тотал' chip beside the corrected value, parallel to the existing 'непотвърден тотал' suspect marker.
…idt-bg#305) expect(full?.X).toBe(slice?.X) after both were pinned to a concrete literal adds no coverage and would also pass on dual-undefined. Keep the per-path literal assertions.
Ревю — коригиране/маркиране на двойно броене на стойността при анекси (#305)Заключение: искам промени (request changes). PR-ът поправя реален дефект (емисията слага новата обща стойност в полето за промяна, така че Блокиращи (2 HIGH — и двете корумпират публикувана стойност)HIGH-1 — голото „на" пренаписва случайно съвпаднала непарична стойност (
HIGH-2 — безтекстовото правило за точно 2× разполовява легитимни удвоявания в обхвата на ЗОП (
Средни (потвърдени)
Изчистено (проверено)Няма SQL инжекция (пренаписаните стойности са валидирани числа; стейджнатият текст е escape-нат по единични кавички). Tier 2 се смята в ingest ( ОбобщениеИскам промени по двата HIGH — реални ръбове, които корумпират стойност в самата коригираща евристика (свръх-пренаписване на непарично „на"-число; разполовяване на легитимно 2×). Останалото е стабилно: флаг-и-изключи слоят, ingest-базираното (без дрейф) пренаписване и SQL fallback-ът са коректни. Стесняване на двете правила (паричен котвен маркер за „на"; текстов гейт за 2×) плюс тестове точно за тези входове биха го изчистили. |
|
Прегледах PR-а на 1. Новите колони няма как да стигнат до сервираната база (ново)
Мърдж към Иска се едно от двете: стъпка в Свързано с това е и бележката за идемпотентността: 2. Голото „на" пренаписва стойността с непарично числоПотвърждавам HIGH-1 на @DiyanaDimitrova - възпроизведено срещу Договор със стойност 300 става 200, защото 200 е брой дни. Предложението в прегледа е правилното: искайте стойностен маркер около „на" ( 3. Правилото за точно 2× пренаписва без никакво потвърждениеПотвърждавам HIGH-2, и то е по-широко, отколкото звучи. Не става дума само за законно удвояване с текст - правилото е по подразбиране за всеки ред с точно 2×: Празен текст и текст без никаква връзка със стойността дават един и същ резултат: разполовяване. Това е съзнателно и дори закотвено с тестове ( Не се наемам да отсъждам правото. Структурната бележка обаче стои и е от най-неприятния сорт: пренаписваме публикувана стойност без нито едно потвърждение, а грешката в тази посока подценява реален договор и е невидима за читателя. Или текстов гейт, или по-тясна ЗОП предпоставка. Какво проверих и е наред
ОбобщениеСкелетът е добър и си струва да влезе. Трите неща преди това: стъпка за миграциите, паричен котвен маркер за „на", и потвърждение за 2× правилото. Първото е блокер за деплоя, другите две са блокери за данните. Благодаря за работата - и за плана в |
nikimilenkov
left a comment
nikimilenkov
left a comment
There was a problem hiding this comment.
Обстоен преглед — PR #307 @ 8652d69 (поправя #305)
Благодаря за този PR — проблемът е диагностициран прецизно, консервативната посока („при съмнение — флаг, не пренаписване") е правилната, тестовете изпълняват истинските скриптове, а покритието е измерено върху реалния масив от данни. Прегледът мина по строгия протокол (пет паралелни измерения; всяка находка проследена, възпроизведена или мутирана върху чисто копие на този HEAD), стъпва върху прегледите на @DiyanaDimitrova и @todorkolev, без да повтаря находките им — тук са само независимите потвърждения с добавени доказателства и новите неща.
Предложение: връщане за промени — присъединявам се към исканията на двамата. Общо на PR-а: 1 критична (точка 1 на @todorkolev, потвърдена тук с допълнителни последици), 2-те високи на @DiyanaDimitrova (потвърдени), 2 нови високи, плюс нови средни/ниски по-долу. (Съветодателно — решението е на поддържащите.)
Изпълнени проверки: packages/ingest 75/75, packages/db 407/408 (единственият неуспех е познат env артефакт — ship-domain.test.ts извиква pnpm без пълен път); 6 мутации върху класификатора (4 убити, 2 оцелели) + 2 SQL мутации по двата пътя (двете хванати от parity тестовете); две механични репродукции през реалните SQL скриптове; производителност, измерена при мащаба на реалните данни; план-документът прочетен и сверен ред по ред с кода; CI зелен.
Потвърждения на предишните прегледи (независимо, с изпълнение — не ги повтарям като нови)
- HIGH-1 на @DiyanaDimitrova (голото „на") — потвърдена чрез изпълнение срещу реалния класификатор (същия вход изпълни и @todorkolev, същият резултат):
total_restated: 200, т.е. сервирана стойност, презаписана с брой дни. Съгласен съм с предписанието (паричен котвен маркер). - HIGH-2 на @DiyanaDimitrova (безтекстовото точно-2×) — потвърдена чрез изпълнение (опционна клауза →
unchanged_restated; @todorkolev показа и празно-текстовия вариант по подразбиране), и добавям две нови доказателства в подкрепа: (а) собственият тест на PR-а показва правилото да измисля число, което никой не е казвал — за „увеличава се от 15 120 лв. … до 18 900 лв." истината е 18 900, източникът казва 30 240, кодът записва 15 120 и го рендира с чип „коригиран тотал"; (б) правилото противоречи на три собствени документа: заглавния договор на модула („NEVER rewrites … cannot corroborate from text"), коментара в0007(„never invent a number") и решения §3.3 от план-документа, който за точно-2× изисква валутния текстов маркер (евро|валута|EUR) — планът на автора вече аргументира точно нейната поправка. - Трите ѝ средни — потвърдени (таймлайн рендерингът на флагнатите редове; recall загубата на точковия формат — виж НОВА СРЕДНА 2 за втора, по-остра посока на същия парсер; неидемпотентните миграции — виж КРИТИЧНАТА, която е ескалация на същата тема).
КРИТИЧНА — точка 1 на @todorkolev, потвърдена независимо, с две допълнителни последици
Стигнах до същата находка успоредно (възпроизведено тук: no such column: am.value_restated срещу база по 0000_init; deploy.yml няма стъпка за 0006/0007) — той я публикува пръв и анализът му, включително ledger състоянието на живата база, е точен. Добавям две последици, които разширяват blast radius-а отвъд страниците на договори:
- дневният Worker също пада:
refresh-slice.sql:1687-1763INSERT-ва новите колони в сервиранатаamendments, така че освен четенето (details.ts) се чупи и самият дневен refresh; ship-domain.mjsстрои INSERT списъка си от PRAGMA на работната база → корабирането също прекъсва срещу немигрирана цел.
Съгласен съм с предписанието му (сонда по образеца на 0002, преди Worker деплоя — не migrations apply); допълвам само: 0006 носи два неидемпотентни ALTER-а в един файл, така че или разделяне, или сондата да отказва изрично при частично състояние (1–2 от 3 колони).
НОВА ВИСОКА 1 — мултианексното замърсяване: рестейтнат първи анекс + нормален втори = сервирана стойност, която остава грешна, без нито един gate да я хване
Възпроизведено механично през реалния derive-amendments.sql: подписана 1 000 000; анекс1 удвоен (2 400 000, рестейтнат на 1 400 000); анекс2 идва от емисията върху замърсената база (2 400 000 → 2 760 000, реални +15%). Резултат: current_value = 2 760 000 при истинска стойност 1 760 000 — постоянно надценяване. Никой gate не помага: собственото отношение на анекс2 е 1,15× < 2×, а anchor проверката пада по конструкция (prev.value_after < 2×prev.value_before е невярно точно когато prev е удвоеният). Рестейтването е per-ред и не се пренася по веригата — а бройката „362 коригирани" смята такива договори за оправени, докато сервираната им стойност не е (метриката брои редове, не договори). Тестовете покриват само обратния ред (легитимен анекс1 → удвояващ анекс2), никога този.
Посока: пренасяне на корекцията по веригата (value_before на анекс N от COALESCE(value_after_restated, value_after) на N−1) или anchor разширение („prev беше коригиран, а am.value_before още е суровата му стойност" → флаг), плюс огледален тест и преизмерване на „коригирани" на ниво договор.
НОВА ВИСОКА 2 — reconciliation копието на slice пътя смята гейта върху различни стойности от пълния път
Копията в normalize-raw.sql:970-980 и refresh-slice.sql:1209/:1542 четат FROM raw_amendments prev (сурови стойности); reconciliation блокът refresh-slice.sql:1869-1894 чете FROM amendments prev (сервирани, т.е. рестейтнати) — проверено на този HEAD. Конкретика: анекс1 рестейтнат 2,4M→1,4M; анекс2 1,4M→2,8M. Пълният път дава ok (котвата пада срещу суровите), slice пътят дава annex_total_suspect (котвата минава срещу рестейтнатите) — флагът се обръща между дневния refresh и следващия пълен rebuild. В продукция Worker-ът никога не пуска derive-amendments.sql, така че решаващото копие е точно разминаващото се — а тестовете пускат slice винаги след derive, т.е. продукционната топология не се упражнява от никой тест. Седемте копия на предиката са и без lockstep маркери, въпреки че планът изрично поръча разширяване на #286 образеца. (Уточнение към чистия списък на @todorkolev: „Tier 2 се смята в ingest → няма дрейф" е вярно за Tier 2 — разминаването тук е в Tier-1 reconciliation копието, което е SQL и не се наследява от ingest.)
НОВИ СРЕДНИ
- Числовият парсер, втора посока (допълва нейната M-2): фрагментирането на точково групирани числа е и false-positive повърхност, не само recall загуба — „234.567.890,12" се разпада на „234.56"…, а дати („15.03.2026") дават малки числа точно след „на"/„с"; фрагмент, който случайно ≈ малка
value_delta(прозорец [100, 999.99]), класифицира грешно. Точков токенизатор + фикстури с дати/точкови числа до тригер-думите. - SQL копията нямат проверката за самосъгласуваност (
value_after ≈ value_before + value_delta), която планът §4 изисква за флага — има я само TS класификаторът; ред, за който моделът доказуемо не важи, пак се флагва и изпада от агрегатите. prevне е ограничен темпорално и в promote не е дедупнат — по-късен анекс или недедупнат staging ред може да „закотви" по-ранен; посоката е безопасна (свръх-флагване), но коментарът „a preceding annex's value_after" е неверен както е написан, в седемте копия.- Ред-срещу-договор противоречие: при изпреварен дубъл (фикстурата 100→200, после 200→130) договорът правилно е
okи сервира 130, но първият ред носиvalue_suspect = 1и се рендира „— непотвърден тотал" — две правила, един екран, противоположни присъди. value_lowсе унищожава по slice пътя приhas_double— preserve клонът се прескача, а re-derive веригата нямаvalue_lowрамо; пълният път го пази → флагът се различава между rebuild и refresh. (Дупката предхожда PR-а заhas_step10; тук се разширява към нов предикат.)- Без backfill за историческите сервирани редове — маркерите пристигат само чрез пълен rebuild + ship; никъде не е казано, а repo-то има точния прецедент (
backfill-current-value-currency.sql+ deploy стъпката му). annex_total_suspectне е регистриран в нито едно канонично изброяване наvalue_flag:0000_init.sql, ADR-0003 (дефиниращият документ),core-scope.md,etl.mdи — най-осезаемо —describe-schema.ts: асистентът е grounding-нат с изчерпателен списък и генериранитеIN-списъци ще изпускат тези договори от бройки, не само от суми.- План-документът се разминава с кода по „решени" точки (освен §3.3 по-горе): „Fix location (decided): raw/derive, не ingest… не мутирай raw_amendments" срещу имплементация в ingest с две нови staging колони (изборът е защитим — суровите стойности остават непокътнати, текстов парс в SQL е нереалистичен — но обръща „решено" без запис защо); никъде не е документирано, че
value_treatmentе замразен при ingest (промяна на евристиката иска ре-ингест, не re-derive); статус „Draft", стари line референции, несъгласувани бройки (686 в плана срещу „~183" в 0007). Планът да се пренапише или маркира като изместен. - Тестови пропуски (мутационно доказани): редът правило-1-преди-правило-2 е незакован (размяната минава 75/75, а коментарът твърди гаранция); границата на 2× бандата е незакована (разширяване до 1,5× минава зелено — а би позволило рестейтване под
value_before); db тестовете сейдватvalue_treatmentна ръка — реалният ingest никога не тече през SQL тестовете,load-eop.mjsе с нулево покритие,'unchanged_restated'никога не минава през SQL;outsideZopmapper веригата, multi-text join и NBSP числата — без фикстури.
НОВИ НИСКИ
- Параграф „модел на заплахата" в заглавието на
amendment-total.ts: текстово-управляемият opt-out от флага е data-quality ремонт за небрежен източник, не адверсариален контрол (проверено: не дава нов примитив — пренаписаната стойност е закована в 0,5% отvalue_deltaи е самоdилиb). value_deltaв сервираната таблица тихо сменя семантиката си (изчислен, вече не суров; NULL при NULLvalue_before) — единственият консуматор си преизчислява, но промяната е недокументирана.- Асиметрията на
genuine_increment: един false positive на голото „с" е по-лош от липса на евристика (връща удвоена стойност вamount_eur, която Tier 1 щеше да изключи) — върви заедно с поправката на нейната HIGH-1. - Рестейтнат ред без FX курс губи чипа „коригиран тотал" — визуално неразличим от празен ред.
outside_zop: NULL се свива доfalse(прилага безтекстовото правило); тристепенна семантика + броячrestated_unknown_zop. Отворен въпрос: популира ли сеisExceptionContractизобщо на ниво анекс ред в реалната емисия? Ако не — guard-ът е инертен на този път.- Бройките в описанието са остарели (75 ingest / 408 db срещу 72/400);
currencyе мъртво входно поле на класификатора (планът §3.3 всъщност го искаше за валутния маркер).
Проверено и здраво (за баланс)
Никаква инжекция и никакъв нов примитив за контролиращ емисията; EOP пътят типизира value_after_restated коректно като REAL (през baseColumnKind, не SQL_REAL_COLS — проверено); производителността е чиста за сливане (класификаторът строго линеен ~25 ns/знак до 800 KB; всички нови предикати индексирани; +15 ms на дневния batch; един 30-секунден pre-merge check: MAX(COUNT(*)) анекси на договор < ~300 затваря O(k²) наблюдението); natural_key стабилен (hash върху суровите стойности — без #286 риск); флаговете дизюнктни с обща fallback пътека; валутната конверсия взима валутата от печелившия ред (84818 е коректен); follow-up-ите от самопрегледа са реални (tiebreak изравнен, tautology поправена — проверено с мутация, restated чипът рендиран, outsideZop прокаран); двете опровержения от самопрегледа са верни (проверени независимо, с уговорките в НОВА СРЕДНА 3).
Здравна оценка: 6 / 11
Коректност (пълен път) ◐ · Коректност (slice) ✗ · Деплой ✗ · Сигурност ✓ · Производителност ✓ · Тестове ◐ · Документация/план ✗ · Идемпотентност/ре-дерив ◐ · Наблюдаемост ◐ · Обхват ✓ · Честност на валидацията ◐ (метриката брои редове, не договори).
Преди сливане
- КРИТИЧНАТА — деплой стъпка за 0006/0007 по образеца на 0002 + разделяне на 0006; без нея всичко останало е мут.
- Двете ѝ ВИСОКИ — вече имат съгласувана посока и от нейния преглед, и от собствения план §3.3.
- НОВА ВИСОКА 1 — верижно пренасяне или anchor разширение + огледален тест; „коригирани" да се преизмери на ниво договор.
- НОВА ВИСОКА 2 — reconciliation копието да се изравни (или разминаването да се документира и тества); slice-без-derive тестово рамо; lockstep маркери за седемте копия.
- Средните по преценка (2, 5, 7 и 8 са евтини и режат по клас бъдещи дефекти); ниските — по желание.
Задачата е трудна по същество — свободен текст върху финансови числа — и подходът е правилният: консервативен, слоест, измерен. Критичната е малка като поправка, двете „стари" високи имат готова посока, двете нови са локализирани. С тях това ще е образцовото решение на наистина неприятен дефект. Благодаря — и на @DiyanaDimitrova и @todorkolev за прецизните прегледи преди този.
…or (midt-bg#305) Addresses the two data-corruption findings on PR midt-bg#307: - HIGH-1: the total-restatement rule accepted a bare "на <N>", so a non-monetary number (days, article nos.) that coincidentally matched value_delta could overwrite the published value ("удължава на 200 дни" → value 200). Require a monetary anchor bracketing the figure: a value keyword before it or a currency unit within ~60 chars after. - HIGH-2: the exact-2× "unchanged" rule rewrote value_before with no text confirmation, silently halving a legitimate ЗОП чл.116 ал.1 т.1 in-scope +100% (pre-announced option clause) that outsideZop cannot model. Require a positive signal (currency re-denomination / "unchanged / non-material" phrasing, or the before-value announced as the total); otherwise return none and let the arithmetic annex_total_suspect flag exclude the row — an honest gap beats a silent corruption. Also parse dot-thousands + comma-decimal amounts ("1.234,56" / "1,234.56") so mixed-separator totals are read correctly.
…idt-bg#305) The contract-page query reads amendments.value_restated/value_suspect and promote/refresh-slice INSERT value_treatment, but migrations 0006/0007 never reach the served DB — deploy.yml probes only 0002/0003 and no workflow runs `d1 migrations apply`. Merging would deploy the Worker against a schema missing those columns and every contract page (and the ETL write) would fail. Add a column-probe step mirroring the 0002 pattern that ALTERs only the missing columns (all three: value_restated, value_treatment, value_suspect), before the Worker deploys. Additive columns with safe defaults, so no backfill or completion marker is needed. Document the out-of-ledger application in the migration headers so a ledger replay does not hit a duplicate-column error.
…idt-bg#305) For annex_total_suspect (a KNOWN exact 2× double-count) current_value_eur is NULL, so details.ts fell back to the doubled native current_value and the page rendered it under an "unverified" label. Show — instead: a known-wrong number is worse than an honest gap. Adds a currentValueDoubled flag to the value timeline; the trustworthy signing value is still shown.
|
Благодаря за двата задълбочени прегледа. Адресирах и трите блокиращи находки; ето какво се промени (нови комити HIGH-1 — голото „на" пренаписва непарично число (@DiyanaDimitrova, @todorkolev)
Една бележка за котвата: чистото искане на HIGH-2 — правилото за точно 2× пренаписва без потвърждение (@DiyanaDimitrova, @todorkolev)
Не тръгнах по стесняване само на ЗОП-предпоставката — Блокер за деплоя — новите колони не стигат до сервираната база (@todorkolev, точка 1)
Средни
Тестове: |
…ion (midt-bg#305) Two correctness findings from review, both on the annex_total_suspect gate: - NEW HIGH 1 (multi-annex chain contamination): the double-count correction is per-row and does not propagate down a chain. A restated prior annex (doubled → corrected down) leaves a later annex still computed by the feed on the raw, doubled base; its own step ratio is legitimate (<2×) so the arithmetic gate missed it and the prior is text-treated (excluded), yet current_value inherited the doubled total. Add a branch that flags the driving annex whose value_before ties to a prior annex's raw value_after where that prior was restated lower → signing fallback (honest exclusion beats a served overstatement). Full-corpus per-chain value_before propagation (the exact correction) is deferred to a follow-up. - NEW HIGH 2 (slice reconciliation parity): the reconciliation reads the cumulative served `amendments` (whose value_after is restated) while the full path anchors on raw values, so a restated prior annex flipped the "prev not itself a double" test and the flag flipped between the daily slice and the next full rebuild — and production only runs the slice copy. Restrict the anchor to non-restated prevs (prev.value_restated = 0), whose served value_after equals raw, reproducing the full-path decision without losing cross-window history. The slice cannot see a restated prev's raw value_after, so the contamination detection there is a documented conservative approximation; exactness returns on the next full rebuild. Also add the value_after ≈ value_before + value_delta self-consistency guard (NULL-tolerant) to the gate copies, matching the TS classifier's precondition. Tests exercise both fixes across the full and slice paths (real scripts).
…midt-bg#305) The flag was missing from the assistant grounding (describe-schema.ts) and the 0000_init schema comments, so generated IN-lists could drop those contracts from counts (not just sums). List it alongside the other value_flag verdicts.
|
Благодаря, @nikimilenkov — прегледът по строгия протокол хвана две реални неща, които предишните два не бяха повдигнали. Адресирах и двете нови ВИСОКИ + двете евтини средни (M2, M7); новите комити са НОВА ВИСОКА 1 — верижно замърсяванеПотвърдих механично: НОВА ВИСОКА 2 — паритет на reconciliation копието — с една корекция към предписаниетоПотвърдих флипа. Но при имплементацията излезе, че предложеният one-token fix ( Истинската ос на разминаването е Средните
Отбелязани за follow-up (по преценка на поддържащите)Per-chain Тестове: |
Валидация на NEW HIGH 1 срещу живия корпус (
|
| метрика | стойност |
|---|---|
| Замърсени договори (старият гейт пропуска, новият хваща) | 37 |
…с current_value ≥ 2× signing (явно надценени) |
33 |
Сумарна сервирана current_value |
~210 млн. лв. |
| След fallback към signing | ~80 млн. лв. |
| Надценяване, което поправката маха | ~130 млн. лв. |
Конкретен пример 00752-2021-0007: анекс1 утроява 13.58М→40.73М (дубъл, който #305 рестейтва), анекс2 добавя легитимни +16% върху замърсената база 40.73М → сервирана current_value = 47.06М. Утрояващият анекс не задвижва current_value (40.73М ≠ 47.06М), затова днес е ok; новият клон го флагва → 47.06М пада към signing 13.58М.
Честни уговорки:
- 37 е горна граница по формата. Новият клон гърми само когато предходният дубъл реално е рестейтнат (
value_after_restated IS NOT NULL) — което зависи от текста на основанието, а сервираната база не носи текстова класификация, тъй че точното подмножество не се разделя оттук. Flag-only предходници (нерестейтнати) са свързан остатък — покрит от отложения follow-up за per-chain пренасяне наvalue_before. - NEW HIGH 2 (slice-vs-full паритет) не се мери от сервирания изход — то е за това коя стойност чете reconciliation-ът по време на ETL. Валидира се от новия parity тест по двата пътя, не от заявка към D1.
- Не мога да пусна самата поправена флаг-логика срещу D1 (D1 е изходът на ETL, не стейджингът). Категоричното доказателство е сюитата от 413 теста върху реалните SQL скриптове; тази D1 проверка показва, че поправката цели реална, немалка популация (~130 млн. лв. надценяване в 37 договора).
(Методология: заявките са срещу D1 sigma-dev, акаунт b2abee…353d.)
D1 validation against the live corpus surfaced 46 contracts (~96M EUR) whose exact double-count sits on an "orphan" value_before — one tying neither the signing value nor any prior annex (e.g. ticket example 84818: an annex reports 76.77M → 153.54M on a base unrelated to the contract). The arithmetic gate's anchor (signing OR a legit prior total) can't reach them, and midt-bg#307's HIGH-2 gating correctly declines to text-freely halve them — so they were served at the doubled value, neither flagged nor corrected. Relax the gate's anchor with a third arm: an EXACT single-step 2× (a legal +100% in one amendment is impossible under ЗОП чл.116) flags → signing fallback even on an orphan base. It EXCLUDES only — it never rewrites the value (that stays text-gated per HIGH-2). An orphan guard (NOT EXISTS a prior annex tying value_before) leaves a legitimate compounding-doubling chain untouched, exactly as the legit-prior arm does. Applied to all gate copies (full, slice, per-row). On the live corpus this lifts contract-level detection to ~360 (337 from `ok`), 84818 among them. Test: orphan exact-2× flagged; non-exact orphan jump stays ok.
End-to-end валидация на #305 + PR срещу живия
|
| Тикет (rebuild 12.08) | Живо sigma-dev |
|
|---|---|---|
| анекси с ръст: 7 335 | 7 306 | ✓ |
| ≥100% заподозрени: 686 | 687 | ✓ |
| точно +100% дубли: 199 | 281 (по-нов, по-голям снапшот: 31 597 анекса) | ✓ |
2. Покритие на поправката — скок
Фиксираният аритметичен гейт флагва 360 договора, 337 от които са ok днес → детекцията на ниво договор скача от ~20 → ~360, изваждайки удвоени стойности към signing fallback. NEW HIGH 1 (верижно замърсяване) добавя 37, които старият гейт пропуска (~130 млн. лв.).
3. Трите примера от тикета
| Запис | Резултат |
|---|---|
| 189325 (77М→154М, „от лева в евро") | ✅ хванат — валутна деноминация → unchanged_restated/гейт → 77М |
| 145652 (442К→981К дубъл) | ✅ хванат от гейта → signing fallback 442К |
| 84818 (76.77М→153.54М точен дубъл, „курсове…преструктурират") | ✅ сега хванат — виж §4 |
4. Находка от D1 → нова поправка (03b8384)
84818 разкри дупка: точният му 2× стои на сирашка база (76.77М не се връзва нито към signing 113.4М, нито към предходен анекс). Старият анкер не го стига, а #307 HIGH-2 гейтингът (правилно) отказва да го разполови без текст → сервираше се удвоен. Мащаб: 46 договора, ~96 млн. EUR.
Добавих трети арм към анкера: точен единичен 2× (легален +100% в един анекс е невъзможен по чл.116 ЗОП) флагва → signing fallback дори на сирашка база. Само изключва — никога не пренаписва (текст-фрий рестейтът остава гейтнат, HIGH-2). Orphan guard (NOT EXISTS предходен анекс, връзващ value_before) оставя легитимните компаундиращи вериги непокътнати. На живия корпус това вдига детекцията до 360 (337 от ok), вкл. 84818.
5. Статус на всяка находка (валидирано на D1 където е измеримо)
| Находка | Митигирано | Доказателство |
|---|---|---|
| Blocker A (колоните не стигат до сервираната база) | ✅ | D1: колоните липсват + 0 annex_total_suspect — точната повреда; probe я решава |
| HIGH-1 (голото „на") | ✅ (тест) | ingest текст логика — не се вижда от изхода; 17 unit теста |
| HIGH-2 (текст-гейт 2×) | ✅ (с компромис) | §4 — държи се както поискахте; извади 46-те сирашки дубъла, вече покрити |
| NEW HIGH 1 (верижно замърсяване) | ✅ | 37 реални договора, ~130 млн. лв. (измерено) |
| NEW HIGH 2 (reconciliation паритет) | ✅ (тест) | ETL-вътрешен флип — не се вижда от изхода; parity тест |
| 84818-class (сирашки точен дубъл) | ✅ (нова поправка) | 46 договора / ~96 млн. EUR; 84818 сега хванат |
Display — / числов формат / M7 / M2 |
✅ (код/тест) | презентация & ingest — не се виждат от сервираните редове |
Тестове: @sigma/db 415, @sigma/ingest 80, tsc/prettier чисти. Всичко от прегледите е адресирано; D1 проверката показа реален остатък (84818-class), който вече е поправен.
(Заявки срещу D1 sigma-dev, акаунт b2abee…353d.)
Пълен тест на #305 + PR срещу живия
|
| Претенция (rebuild 12.08) | Живо sigma-dev |
|---|---|
| общо анекси: 26 921 | 31 597 (по-нов снапшот) |
| с ръст: 7 335 | 7 306 |
| ≥100% заподозрени: 686 | 687 |
| точно +100% дубли: 199 | 274 |
| текущи флагове хващат ~20 | 11 value-флагнати (5 annex_suspect + 5 review + 1 value_suspect) |
| незасечени | 479 заподозрени договора са ok днес |
Договори, чиято current_value е реално надута от дубъл: 397 (ok), сервиращи ~506 млн. EUR удвоена стойност.
B. Покритие на поправката
- Фиксираният аритметичен гейт флагва 360 договора, 337 от
ok→ детекция на ниво договор ~20 → ~360. - Тези 337 свалят ~506 млн. → ~272 млн. EUR (signing fallback) — маха ~234 млн. EUR фиктивна стойност.
- Трите примера: 189325 ✅ (валута→restate 77М), 145652 ✅ (гейт→signing 442К), 84818 ✅ (сега хванат).
C. Всяка находка, тествана на D1
| Находка | Митиг. | Доказателство от D1 |
|---|---|---|
| Blocker A | ✅ | колоните липсват + 0 annex_total_suspect — точната повреда |
| HIGH-1 (голото „на") | ✅ | 12 800 анекса с непарично „на N дни/месец" + 694 с „чл. N на ЗОП" — реалната рискова повърхност, която money-anchor-ът пази; фиксът гърми само на 9 599-те парични |
| HIGH-2 (текст-гейт 2×) | ✅ | от 274 точни дубъла само 12 носят restate-сигнал (1 валута + 11 „несъществен") → пренаписват се; другите ~262 падат към флаг — точно консервативността, която поискахте |
| NEW HIGH 1 (верижно замърсяване) | ✅ | 37 договора, ~130 млн. лв. |
| NEW HIGH 2 (reconciliation паритет) | ✅ (тест) | ETL-вътрешен флип — не се вижда от изхода; parity тест |
| 84818-class (сирашки точен дубъл) | ✅ | 46 договора / ~96 млн. EUR; 84818 сега хванат |
Число 1.234,56 |
✅ | 7 точни + 120 multi-dot суми в описанията — реален (малък) recall |
| M2 (само-съгласуваност) | ✅ | 0 заподозрени с несъгласувана делта днес — чисто защитен guard, не мени нищо сега (честно) |
Display — / M7 (изброявания) |
✅ (код) | презентация/asистент grounding — не се вижда от сервираните редове |
D. Честни остатъци
- ~60 договора (397 надути − 337 хванати) са non-exact сирашки дубли (напр. 2.5× на несвързана база), които консервативният фикс нарочно оставя — може да са легитимни +100% или искат текст. Кандидати за верижното пренасяне (follow-up).
- Сляпо петно (тикетът го отбелязва): дубли под 100% (нова обща стойност < старата) дават ръст <100% и не се засичат по подписа — не се мери от суровите данни.
Заключение
Проблемът от #305 е реален и се възпроизвежда (687 заподозрени, ~506 млн. EUR надути в 397 договора). PR-ът вдига детекцията от ~20 на ~360 договора, а D1 проверката потвърди всяка находка и извади един реален остатък (84818-class, ~96 млн. EUR), който вече е поправен. Тестове: @sigma/db 415, @sigma/ingest 80, tsc/prettier чисти.
(Заявки срещу D1 sigma-dev, акаунт b2abee…353d.)
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.
|
Прегледах наново на
Тестовете минават: Остават две тесни дупки в самите нови пазачи. И двете са от същия клас - пренаписват публикувана стойност - и за двете предложението е проверено, че не вали нито един от вашите 80 теста. 1. Паричната котва се задоволява с валута другаде в изречението
А това е много честа формулировка в основанията - срок и стойност в едно изречение. Проверено предложение: вето, когато веднага след числото стои непарична мерна единица. Добавих един ред преди проверката за котва: if (/^\s*(?:дни|дн\.|месец\p{L}*|години|год\.|броя|бр\.|%|процент\p{L}*)/iu.test(after)) continue;И двата случая горе стават 2. Голото „в евро" пуска клауза за валута на плащане
Голата алтернатива е и излишна: вашата собствена фикстура за 189325 е „…се променя от лева в евро", която първата алтернатива вече хваща. Махнах голото
Тоест това е една махната алтернатива без загуба на покритие. ОстаналоДвете дупки са тесни, но са в кода, чиято единствена работа е да пази от точно този клас грешка, и цената при грешка е несиметрична - подценен реален договор, невидим за читателя. С тези две стеснения от моя страна PR-ът е готов. |
… payment-currency clauses (midt-bg#305) - MONEY_AFTER anchored a total on any currency token in the 60-char window, so '...удължава на 200 дни, стойността остава 100 лв.' restated value_after to a day count. Veto the figure when a non-monetary unit (days/months/years/count/%) immediately follows it, before the currency anchor is consulted. - RESTATE_UNCHANGED_CTX matched a bare 'в евро', so a payment-in-euro clause ('Плащанията...се извършват в евро...') halved a real +100%. Drop the bare alternative; the 'X в евро' re-denomination form (189325) already covers the fixture.
|
И двете тесни дупки са затворени в 1. Паричната котва + непарична мерна единица. 2. Голото „в евро". Махнато от Добавени са и два теста точно за тези случаи. Тестове: Проверка срещу реалния корпус (D1
Двете стеснения са строго subtractive (вето + махната алтернатива): могат само да превърнат пренаписване в С това PR-ът е готов от моя страна. |
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.
|
Прегледах на Затворено: голото „в евро"Проверих го, не го приемам по описание: Точно това искахме: махната алтернатива без загуба на покритие. Отворено: ветото хваща само низа, който съобщих, не класа
Пуснах ги през истинския
Тоест договор за 300 лв. се публикува като договор за 200 лв., защото 200 е брой работни дни. „Работни дни" и „календарни дни" са по-честите формулировки от голото „дни" - ветото минава покрай тях. Същото важи и за новия тест: той заковава дословно изречението, което дадох, а не класа грешка. Затова и не хваща варианта с една дума повече. Проверено предложение - пуска една незадължителна дума (и незадължителна скоба за изписаното с думи число) преди единицата, и разширява списъка: const NON_MONEY_UNIT_AFTER =
/^\s*(?:\([^)]*\)\s*)?(?:\p{L}+\s+)?(?:дни|дн\.|к\.\s?д\.|р\.\s?д\.|месец\p{L}*|години|год\.|броя|бр\.|кв\.?\s?м|куб\.?\s?м|тона|литра|%|процент\p{L}*)/iu;С него всичките седем реда горе стават Проверих и обратната посока - че по-широкото вето не изяжда истински парични изрази: Дори да сгреши, посоката е безопасната: ветото води до ОбобщениеЕдно нещо остава, и е на един ред разстояние. С това вето PR-ът е готов от моя страна. |
…/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.
…idt-bg#305) NON_MONEY_UNIT_AFTER was anchored on the slice immediately after the figure, so any word between the number and its unit ("200 работни дни", "200 календарни дни") slipped past the veto and let a downstream currency token restate the contract value to a day count. Allow one optional adjective word (and an optional spelled-out number in brackets) before the unit and extend the unit list (к.д., р.д., кв.м, куб.м, тона, литра). Tests now assert the error class (qualified units, brackets, area/volume) plus the reverse direction — the wider veto must not swallow a genuine monetary total. @sigma/ingest 84/84, tsc + prettier clean.
|
Готово на
const NON_MONEY_UNIT_AFTER =
/^\s*(?:\([^)]*\)\s*)?(?:\p{L}+\s+)?(?:дни|дн\.|к\.\s?д\.|р\.\s?д\.|месец\p{L}*|години|год\.|броя|бр\.|кв\.?\s?м|куб\.?\s?м|тона|литра|%|процент\p{L}*)/iu;Всичките седем реда от прегледа стават Валидация срещу реалния корпус (
|
|
Прегледах на Ветото е разширено и, което е по-важно, тестът вече заковава класа, а не изречението, което дадох - осемте варианта с работни/календарни дни, к.д., р.д., изписаното с думи число и мерките за площ и обем. Плюс обратната посока с петте истински парични израза. Точно това липсваше предния път. Проверих го не само на измислени изречения, а на реалния корпус. Изтеглих от stage всички 564 реда, които имат формата на двойното броене (
Нула разлики. Тоест разширеното вето не изяжда нищо в днешния корпус - това беше рискът, който исках да изключа, преди да го предложа за мърдж. Обратната страна на същото число, честно казано: класът с брой дни не се среща в днешния корпус. Пазачът е превантивен - пази бъдещи редове и класа като цяло, не поправя жив дефект. Държа да го кажа, за да не изглежда находката ми по-голяма, отколкото е. Проверих и едно семейство от пренаписаните редове, за да не приема механиката по описание - 17 реда с
Обявената нова стойност наистина е 1,52, а 2,90 = 1,38 + 1,52 е точно двойното броене. Пренаписването е вярно.
Остава само номерът на миграцията - #309 също носи |
todorkolev
left a comment
todorkolev
left a comment
There was a problem hiding this comment.
Одобрявам на 36dd21e. И двете дупки от предния кръг са затворени, а тестът вече заковава класа, а не изречението, което дадох.
Проверено на реалния корпус: всичките 564 реда с формата на двойното броене класифицират еднакво преди и след разширеното вето - нула съжаления. Спот-проверих и едно семейство пренаписани редове срещу текста на анекса; пренаписването е вярно.
ingest 84/84.
Стъпката „Apply amendment restated/suspect columns" от #307 падна на първото си истинско пускане и нямаше как да мине - нито на тази база, нито на никоя. Заявката именуваше колоните has_restated / has_treatment / has_suspect, а read_flag търси реда по низа, с който е извикана ensure_column, тоест value_restated / value_treatment / value_suspect. Object.hasOwn връща false за всяка от трите, сондата чете това като нечетим отговор и излиза с 2, което case-ът третира като фатално. Понеже стъпката стои преди деплоя на Worker-а, деплоят до stage спираше на нея. Сайтът не беше засегнат - нищо не е било разгърнато (стъпки 12-14 прескочени), stage връщаше 200 с изданието отпреди #307. Псевдонимите стават самите имена на колоните, плюс коментар защо не могат да бъдат описателни. Проверено срещу истинската база: заявката връща {value_restated: 0, value_treatment: 0, value_suspect: 0}, а сондата дава статус 1 (добави) при липсващи и 0 (има ги) при налични - никога 2. Пропускът е показателен: deploy.yml не се изпълнява преди мърдж (пуска се само при push към main), нито един тест не го чете и в CI няма линтер за workflow файлове, тъй че първото изпълнение на всяка промяна в деплой стъпка е винаги СЛЕД мърджа. Изнасянето на сондата в проверим скрипт е отделна следваща стъпка.
Разрешени конфликти след 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 чисти.
…ара за 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 чисти.
Бележка 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-а.
4 participants
Fixes #305.
Problem
When an EOP annex announces a new total contract value, ЦАИС ЕОП puts that total in the change field (
contractValueDifference→value_delta), so the feed'scurrentContractValue(→value_after) arrives aslastContractValue + newTotal— the value is doubled.base.tsstores it verbatim (no arithmetic), and a ~2× inflation sits below every existing flag (annex_suspect/#299 need ≥5×), so it flows intoamount_eurand inflates every rollup, CSV, and the contract page. Verified on the real corpus: 686 annexes at ≥100% growth; proof record 145652 shows 981 240 instead of 539 240.Fix (three tiers, all in the ETL — no schema break beyond a nullable served column)
value_flag = 'annex_total_suspect'for a single annex withvalue_after ≥ 2×value_before(ЗОП чл.116 caps a single amendment at +50%, so ≥2× is a defect). Threaded through the value fallback exactly likeannex_suspect→ these contracts fall back tosigning_valueand drop out of every EUR aggregate, on both derive paths (normalize-raw.sql+refresh-slice.sql).packages/ingest/src/amendment-total.ts) reads the основание text and classifies each annex by the preposition before the figure: "…на<N>" / "обща стойност …<N>" = a new total →value_afterrestated toN; "…с<N>" = a genuine increment → the value is correct and is not flagged; the corrected value drivescurrent_valueand the served amendment row (with avalue_restatedmarker so the timeline shows the right number).value_after ≈ 2×value_before, the change field just echoed the OLD value (currency re-denomination or a non-value administrative annex); restate tovalue_before. Text-free, currency-agnostic; the genuine-increment check runs first so a real increase is never mis-restated.Coverage (measured over the full 2020→2026 annex corpus)
Of the 686 doubled annexes: 362 corrected (53%), 183 flagged & excluded (27%), 10 exonerated as genuine, 25 uncaught (multi-annex chains). Isolating the double-count band #299 can't reach (580): 545/580 = 94% handled. All three named proof records resolve — including 84818 (EUR annex on a BGN contract). Full breakdown and comparison to the ticket: #305 (comment)
Honest residuals (documented in
docs/implementation-plans/305-amendment-value-double-count.md)Closing these needs the v2 direct-total parse and cross-referencing the ЦАИС ЕОП contract feed — deliberately deferred rather than over-fitting a fragile regex on financial figures.
Tests
New:
amendment-total.test.ts(heuristic vs real основание text incl. false-positive controls),amendments-total-restated.test.ts+amendments-total-suspect.test.ts(real derive→normalize/refresh-slice→promote→precompute, full-vs-slice parity).@sigma/ingest72 +@sigma/db400 green; tsc + prettier clean. No CI/CD or infrastructure changes.