fix(etl): authority/executor identity — canonical name, EIK checksum, multi-ЕИК guard (#194 #195 #196)

[StanislavBG]

Fixes #194. Also addresses #195 and #196.

Problem

Authorities are deduped on ЕИК (correct — one legal entity), but the displayed name was chosen with MIN(authority_name). MIN() returns the alphabetically-earliest string, so where several labels share one ЕИК, a rare variant wins over the dominant one.

Concrete case (shared ЕИК 000695114): „БСУ Д-р Петър Берон" sorts before „МИНИСТЕРСТВО НА ОБРАЗОВАНИЕТО И НАУКАТА", so the ministry (620 rows, dominant) lost its page title to the school. Contracts were already linked to the single entity — only the label was wrong.

What changed

Canonical authority name (#194) — MIN(authority_name) replaced with a mode (most-frequent label) per ЕИК, with a deterministic tiebreak:

1. COUNT(*) DESC — the most-common label wins.
2. Prefer mixed-case over ALL-CAPS. SQLite's UPPER()/LOWER() are ASCII-only and don't fold Cyrillic, so mixed-case is detected as "contains a lowercase letter" via a GLOB range *[a-zа-я]* (Latin + Cyrillic).
3. Longer label, then lexically-first — for full determinism.

The ЕИК key (id = 'auth:'||ЕИК) is untouched, so slugs/URLs — which key on ЕИК in packages/db/src/queries/identity.ts — don't change. Only the label changes.

ЕИК checksum (#195) — eik_valid now enforces the Bulstat checksum. The ~30-line inline logic (repeated 3+ times) was extracted into a shared scripts/lib/eik-valid.fragment.sql, expanded via a new @include mechanism in scripts/generate-sql.mjs.

Multi-ЕИК guard (#196) — list-valued authority_eik rows are excluded from the canonical-name step, superseding #205. The exclusion is deliberate and tracked in #196; scripts/anomaly-report.mjs reports the count so it isn't silent.

Other fixes landed in review — NULL authority-name fallback; an EOP-wins NULL-collapse bug (bare equality, so two different contracts with a NULL contract_number no longer merge as duplicates); idx_raw_cnum; an O(groups×rows) canonical-name subquery; ownership_kind UPDATE scoped to the refresh window; path-traversal hardening on @include fragment names.

How it was tested

Local staging (raw_contracts/raw_tenders) had been cleared after the last ETL, so the SQL was validated against representative fixture rows in an in-memory SQLite (node:sqlite, same engine):

• MIN() picked the school/all-caps variant (reproducing the bug); the mode picks МИНИСТЕРСТВО НА ОБРАЗОВАНИЕТО И НАУКАТА. PASS
• Equal-frequency tie ОБЩИНА ТЕСТ vs Община Тест → mixed-case wins. PASS
• Blank/whitespace names ignored. PASS
• GLOB lowercase-detection over a Cyrillic/Latin matrix (all-caps vs mixed, both scripts). PASS

Verification pass 1 — pnpm --filter db test -- refresh-slice: 194/194 pass, including the bidder_id scalar-subquery + ownership_kind scoping regression test and the template/checksum-parity sync test (GENERATED_TARGETS, diffing regenerated output against the checked-in .sql).

Verification pass 2 (independent) — full suite re-run against a clean worktree at the PR head with a locally-fetched sqlite3 binary: same 194/194, confirming the fix doesn't depend on stale local state.

Path-traversal guard confirmed manually: expandTemplate rejects any @include fragment name containing /, \, or .. before resolve()/readFileSync.

Quality checks

• CI: green (check job) on head 3629ae9.
• ETL two-path parity: every identity/dedup/keying change is applied to both normalize-raw.sql (full rebuild) and refresh-slice.sql (incremental), enforced by packages/db/src/refresh-slice.test.ts. The one intentional divergence: normalize-raw.sql has no ownership_kind WHERE scoping — correct, since it processes the entire corpus each run, whereas refresh-slice.sql scopes to refresh_batch_bidders (derived once, shared by the bidders INSERT and the UPDATE subquery).
• Known follow-up (not in scope): winner/bidder selection uses the same MIN(contractor_name) pattern. Name-keyed bidders have identical names by construction (the key is the normalized name); only ЕИК-keyed bidders with multiple raw name variants share the latent issue. Noted as a separate follow-up rather than widening this PR.

Review

All review threads from both rounds are addressed and resolved. The two most recent (2026-07-13) were confirmations answered with evidence, requiring no code change:

• Multi-ЕИК reconciliation counter — already implemented in 935d59f: FETCH_MULTI_EIK_EXCLUDED (scripts/anomaly-report.mjs:176) counts distinct excluded lists + raw rows across raw_contracts/raw_tenders, surfaced at anomaly-report.mjs:251. It reads raw_* post-ETL, so it covers both ETL paths.
• EOP always carries contract_number — enforced at ingest, not in SQL: packages/ingest/src/base.ts:156 drops null rows before staging (30/156 of a real day's EOP contracts are dropped there). The nullable contract_number at ocds.ts:320 is the ocds: path only, whose dedup branch deliberately keeps NULL rows. No composite fallback key needed.

[StanislavBG]

Folded in the #196 multi-ЕИК guard (commit a1ff9e2) + the raw-staging authority_eik indexes (d60b11f). This PR is now the single step-1 change: canonical-name mode (#194) + multi-ЕИК exclude (#196), perf-safe. #205 closed as superseded.

[StanislavBG]

Consolidated the identity fixes into this one PR and mirrored them into both ETL paths so the parity test passes:

• fix(etl): каноничното име на възложителя (MIN на името) подвежда при споделен ЕИК — поръчки на МОН излизат под училище #194 canonical name by mode (not MIN)
• fix(etl): eik_valid е само синтактична проверка без контролна сума — служебни/сгрешени ЕИК обединяват несвързани изпълнители #195 Bulstat checksum in eik_valid
• fix(etl): многоЕИК-ови стойности в authority_eik се ключат като един несъществуващ възложител (~406 реда) #196 multi-ЕИК joint-procurement guard
• perf: authority_eik indexes on raw staging

Applied to normalize-raw.sql (full rebuild) and refresh-slice.sql (incremental) — the latter is what caught the earlier gap via refresh-slice.test.ts parity. Verified statements execute correctly on fixtures via node:sqlite (mode→ministry, multi-ЕИК excluded, fake/typo EIKs → name-key). #205 and #207 folded in and closed.

[StanislavBG]

@todorkolev готов за ревю 🙏 — CI зелен, mergeable. Този PR консолидира трите data-quality поправки по гражданския сигнал (#194 каноничното име, #195 Булстат контролна сума, #196 многоЕИК гард); #205 и #207 са слети тук и затворени. Поправено в двата ETL пътя (normalize-raw + refresh-slice), parity минава: refresh-slice 12/12, целият @sigma/db 183/183.

[ydimitrof]

Ревю на PR #203 — fix(etl): authority/executor identity — canonical name, EIK checksum, multi-ЕИК guard (#194 #195 #196)

Благодаря за консолидирания и много добре документиран PR. Прегледах го стриктно, с фокус върху сигурност, целостност на данните и съответствие с OWASP, и валидирах критичните части локално в node:sqlite (същият енджин като ETL-а), а не само по описанието.

Какво проверих и потвърдих

Контролна сума на ЕИК/Булстат (#195) — извлякох точния SQL израз от normalize-raw.sql (стъпка 4) и го изпълних срещу известни стойности:

• Реални ЕИК минават: 131106522 (Просвета), 000695114 (МОН), 2016195800141, 204799888. ✅
• Фиктивни/сгрешени падат до eik_valid = 0: служебният 000000001, близнакът с грешна цифра 131106552, както и 111111111 / 222222222. ✅
• Новите тестови фикстури 111111113 / 222222226 са истински валидни по контролна сума — смяната в refresh-slice.test.ts е коректна и запазва намерението „два различни субекта". ✅
• Алгоритъмът (тегла 1..8 → fallback 3..10; за 13-цифрен: водещите 9 валидни, после 2,7,3,5 → fallback 4,9,5,7; втора 10 → 0) съвпада с референтния Булстат алгоритъм.

Каноничното име по mode (#194) — репликирах казуса с ЕИК 000695114: mode избира „МИНИСТЕРСТВО НА ОБРАЗОВАНИЕТО И НАУКАТА" вместо училищния вариант; при равна честота печели смесеният регистър („Община Тест"); празните/whitespace имена се игнорират. Потвърдих и че GLOB '*[a-zа-я]*' наистина хваща малка кирилична буква (тъй като UPPER()/LOWER() не сгъват кирилица) — коментарът е точен. ✅

МногоЕИК гард (#196) — NOT LIKE '%;%' изключва списъчните authority_eik. Проследих транзитивния ефект: обявленията изискват EXISTS възложител, договорите изискват EXISTS обявление, затова съвместните поръчки отпадат от всички downstream rollups, а суровият staging остава непокътнат (проследимо). Това е точно консервативният подход по предложение 2 в #196.

Паритет — блокът за контролна сума е байт-идентичен и на 7-те места (3 в normalize-raw + 4 в refresh-slice), пазен от parity теста. Съответствието „by construction" се държи.

Сигурност / OWASP

Изцяло статичен ETL SQL — няма динамичен/конкатениран от вход SQL, няма стойности, интерполирани в заявките. SQL injection е неприложим (A03), няма секрети, промени по URL, нови зависимости, обфускация или задни вратички. Промяната всъщност засилва валидацията на входа. Чисто.

Незадължителни бележки (не блокират merge)

1. Дублиране на кода — ~40-редовият CASE за контролната сума е вписан 7 пъти. CLAUDE.md формално забранява дублиране; тук екстракцията е ограничена (няма UDF в този pipeline), а parity тестът смекчава риска — но всяка бъдеща корекция трябва да се приложи и на 7-те места, иначе паритетът тихо се чупи. Струва си да се обмисли generated column / еднократно предварително изчислен eik_valid като follow-up.
2. Видимост на спада в покритие — изключването на ~405 многоЕИК реда премахва тези съвместни поръчки от всички тотали/rollups. Поведението е по спецификация, но добре е anomaly-report или docs да отбележат спада, за да не бъде разчетен като загуба на данни.

Забележка по верификацията

Зависимостите не са инсталирани в review worktree-а, затова цялостният пакетен suite (183/183) и CI приемам по докладваното; критичната логика (контролна сума, mode, изключване, паритет) я потвърдих независимо локално. Моля, преди merge се уверете, че CI е зелен на последния commit.

Verdict: Approve на същество — блокери няма; двете бележки са незадължителни follow-up.

[ydimitrof]

Ревю на PR #203 — fix(etl): authority/executor identity — canonical name, EIK checksum, multi-ЕИК guard (#194 #195 #196)

Благодаря за консолидирания и изчерпателно документиран PR. Прегледах го отново стриктно на последния commit (72c5886), с фокус върху сигурност, целостност на данните и OWASP, и валидирах критичните части независимо и локално в node:sqlite (същия енджин като ETL-а), а не по описанието.

Какво проверих и потвърдих локално

Контролна сума на ЕИК/Булстат (#195) — извлякох точния CASE израз от дифа и го изпълних срещу матрица от стойности:

• Реални ЕИК минават (eik_valid = 1): 131106522 (Просвета), 000695114 (МОН), 2016195800141, 204799888, 000590540, 831391875. ✅
• Фиктивни/сгрешени падат до 0: служебният 000000001, близнакът с грешна цифра 131106552, 111111111, 222222222, както и къси/нецифрови входове. ✅
• Новите фикстури 111111113 / 222222226 в refresh-slice.test.ts са истински валидни по контролна сума — смяната запазва намерението „два различни субекта". ✅
• Алгоритъмът (тегла 1..8 → fallback 3..10; за 13-цифрен: водещите 9 валидни, после 2,7,3,5 → fallback 4,9,5,7; втора 10 → 0) съвпада с референтния Булстат.

Каноничното име по mode (#194) — потвърдих, че GLOB '*[a-zа-я]*' наистина различава смесен от ALL-CAPS регистър при кирилица (тъй като UPPER()/LOWER() не сгъват кирилица): „Община Тест" → предпочетено, „ОБЩИНА ТЕСТ"/„МОН" → all-caps. Tiebreak веригата (COUNT DESC → mixed-case → по-дълго → лексикографски) е детерминистична; празни/whitespace имена се игнорират. ЕИК ключът е непокътнат, значи слъгове/URL (packages/db/src/queries/identity.ts) не се променят. ✅

МногоЕИК гард (#196) — NOT LIKE '%;%' присъства и в двата пътя и изключва списъчните authority_eik. Транзитивно обявления/договори изискват EXISTS възложител, затова съвместните поръчки отпадат от всички downstream rollups, а суровият staging остава проследим. Консервативният подход по предложение 2. ✅

Паритет — блокът за контролна сума е налице на 7-те места (3 в normalize-raw + 4 в refresh-slice, вкл. touched-bidder клона, който вече ключи на eik_valid), а многоЕИК гардът — в двата. Съответствието „by construction" се държи и е покрито от parity теста.

Сигурност / OWASP

Изцяло статичен ETL SQL — няма динамичен/конкатениран от вход SQL, няма интерполирани стойности, няма exec/eval/child_process. SQL injection е неприложим (A03); няма секрети, промени по URL, нови зависимости, обфускация или задни вратички. Промяната засилва валидацията на входа. Чисто.

Незадължителни бележки (не блокират merge)

1. Дублиране — ~40-редовият checksum CASE е вписан 7 пъти. CLAUDE.md формално забранява дублиране; parity тестът смекчава риска, но всяка бъдеща корекция трябва да пипне 7-те места или паритетът тихо се чупи. Струва си follow-up (generated column / еднократно предизчислен eik_valid).
2. Видимост на спада в покритие — изключването на ~405 многоЕИК реда маха тези поръчки от тоталите. По спецификация е, но добре е anomaly-report/docs да го отбележат, за да не се чете като загуба на данни.

Забележка по верификацията

Зависимостите не са инсталирани в review worktree-а, затова пакетния suite (183/183) и CI приемам по докладваното; критичната логика (контролна сума, mode, изключване, паритет) я потвърдих независимо локално. Моля, преди merge се уверете, че CI е зелен на 72c5886.

Verdict: Approve на същество — блокери няма; двете бележки са незадължителен follow-up.

[ydimitrof]

Ревю на PR #203 — fix(etl): authority/executor identity — canonical name, EIK checksum, multi-ЕИК guard (#194 #195 #196)

Благодаря за консолидираната и много прегледно документирана работа. Прегледах диффа ред по ред на глава 72c5886, съпоставих го с описанията на трите тикета и изпълних SQL логиката локално в node:sqlite, за да проверя поведението, а не само коментарите.

Съответствие с тикетите

• fix(etl): каноничното име на възложителя (MIN на името) подвежда при споделен ЕИК — поръчки на МОН излизат под училище #194 (канонично име): MIN(authority_name) е заменен с истинска мода (най-често срещан етикет) на ЕИК, с детерминиран tiebreak: COUNT(*) DESC → предпочитане на смесен регистър пред ВЕРСАЛ → по-дълъг → лексикографски. Възпроизведох казуса с ЕИК 000695114: старият MIN() избираше училището, новата мода избира „МИНИСТЕРСТВО НА ОБРАЗОВАНИЕТО И НАУКАТА" (доминантния етикет). Ключът auth:'||ЕИК е непроменен, така че slug-ове/URL-и не мърдат — само етикетът. Точно както е описано в issue-то.
• fix(etl): eik_valid е само синтактична проверка без контролна сума — служебни/сгрешени ЕИК обединяват несвързани изпълнители #195 (Булстат контролна сума): Алгоритъмът за 9- и 13-цифрен ЕИК е реализиран коректно (тежести 1..8, fallback 3..10, второ 10 → 0; за 13 цифри — валиден водещ 9-цифрен ЕИК, после 2,7,3,5 / fallback 4,9,5,7). Изпълних точния SQL израз срещу матрица: реалните 000695114, 131106522, 2016195800141, 204799888 минават; фалшивите/typo 000000001, 131106552, 111111111, 222222222 и NULL/къси/буквени падат. Невалидните коректно се връщат към name-ключа, което разделя погрешно слетите доставчици (казусът Elsevier).
• fix(etl): многоЕИК-ови стойности в authority_eik се ключат като един несъществуващ възложител (~406 реда) #196 (многоЕИК гард): NOT LIKE '%;%' изключва списъчните ЕИК от съвместни поръчки. Тъй като tenders/contracts се закачат само за authority, който EXISTS тук, изключването е транзитивно — фантомният запис не влиза в нито един rollup, а суровият staging остава недокоснат и проследим. Съответства на консервативния exclude (proposal 2) от issue-то; пълното разделяне е коректно отложено.

Сигурност / OWASP

• Целият диф е статичен ETL SQL. Няма динамично конкатениран или интерполиран SQL, няма потребителски вход в заявките — SQL injection е неприложим (OWASP A03). Тестовите стойности минават през параметризиран ? binding.
• Няма секрети, нови URL-и, нови зависимости, нито обфускиран/бекдор код (A05/A08 чисти). Индексите idx_raw_*_authority_eik са IF NOT EXISTS и безвредни.
• Промените в refresh-slice.test.ts са само пренасочване на фикстурите към контролно-валидни ЕИК (111111113/222222226), запазвайки намерението „два различни субекта".

Паритет: Checksum CASE блокът е байт-идентичен на всичките 7 места (3 в normalize-raw + 4 в refresh-slice), а модата и гардът са огледални в двата пътя → паритетът е по конструкция и е защитен от refresh-slice.test.ts. Авторът докладва зелено CI (refresh-slice 12/12, @sigma/db 183/183).

Незадължителни бележки (не блокират):

1. ~40-редовият checksum израз е инлайнван 7 пъти — това е дублиране, което CLAUDE.md по принцип не обича. Разбираемо е (SQLite тук няма UDF), а parity тестът го покрива, но си струва да се отбележи като tech-debt, ако някога стане достъпен generate/UDF слой.
2. Изключването на ~405 многоЕИК реда премахва тези договори от всички обобщения/суми. Моля, уверете се, че anomaly-report или docs го отбелязват изрично, за да не бъде разчетено като загуба на данни.

Verdict: Approve на същество — одобрявам по същество; двете бележки са незадължителни follow-up, не блокери.

[ydimitrof]

Ревю на PR #203 — fix(etl): authority/executor identity — canonical name, EIK checksum, multi-ЕИК guard (#194 #195 #196)

Благодаря за консолидирания и изчерпателно документиран PR. Прегледах го повторно и строго — с акцент върху сигурност, целостност на данните и OWASP — и валидирах критичната логика независимо, локално в node:sqlite (същият енджин като ETL-а), а не само по описанието. Кодът е непроменен спрямо предишния ми преглед на същия commit (72c5886); заключенията се потвърждават, плюс една допълнителна дребна бележка по-долу.

Съответствие с issue-тата

• fix(etl): каноничното име на възложителя (MIN на името) подвежда при споделен ЕИК — поръчки на МОН излизат под училище #194 (каноничното име) — описанието на проблема (споделен ЕИК 000695114, където училищният етикет побеждава министерството през MIN()) е адресирано точно: MIN() е заменено с mode (най-честия етикет) с детерминиран tiebreak. ✅
• fix(etl): eik_valid е само синтактична проверка без контролна сума — служебни/сгрешени ЕИК обединяват несвързани изпълнители #195 (Булстат контролна сума) — синтактичната проверка е заменена с реалния контролен алгоритъм. ✅
• fix(etl): многоЕИК-ови стойности в authority_eik се ключат като един несъществуващ възложител (~406 реда) #196 (многоЕИК гард) — списъчните authority_eik се изключват (консервативният вариант, предложение 2). ✅

Какво проверих и потвърдих независимо

Контролна сума (#195) — извлякох точния SQL израз и го изпълних срещу известни стойности:

• Реални ЕИК минават: 131106522, 000695114, 204799888, 2016195800141. ✅
• Фиктивни/сгрешени падат до eik_valid = 0: служебният 000000001, близнакът с грешна цифра 131106552, 111111111, 222222222. ✅
• Новите фикстури 111111113 / 222222226 са истински валидни по контролна сума — смяната в refresh-slice.test.ts е коректна и запазва намерението „два различни субекта". ✅
• 9-цифрен (тегла 1..8 → fallback 3..10; втора 10 → 0) и 13-цифрен (водещите 9 валидни, после 2,7,3,5 → fallback 4,9,5,7) съвпадат с референтния Булстат алгоритъм.

Каноничното име по mode (#194) — mode избира „МИНИСТЕРСТВО НА ОБРАЗОВАНИЕТО И НАУКАТА"; при равна честота печели смесеният регистър; празните/whitespace имена се игнорират. GLOB '*[a-zа-я]*' наистина хваща малка кирилична буква (тъй като UPPER()/LOWER() не сгъват кирилица) — коментарът е точен. ЕИК ключът е непроменен, така че slug-ове/URL остават стабилни. ✅

МногоЕИК гард (#196) — NOT LIKE '%;%' изключва списъчните стойности; проследих транзитивния ефект (обявления → EXISTS възложител, договори → EXISTS обявление), затова съвместните поръчки отпадат от всички downstream rollups, а суровият staging остава непокътнат и проследим. ✅

Паритет — блокът за контролна сума присъства байт-идентично на 7 места (3 в normalize-raw + 4 в refresh-slice); наборът от аритметични тегла е идентичен между двата скрипта, пазен от parity теста. Съответствието „by construction" се държи. ✅

Сигурност / OWASP

Изцяло статичен ETL SQL — няма динамично конкатениран от вход SQL, няма интерполирани стойности в заявките. A03 (Injection) е неприложим. Сканирах за опасни конструкции (ATTACH, load_extension, PRAGMA, writefile/readfile и т.н.) — няма (единственото попадение на „attach" е дума в прозаичен коментар). Няма секрети, промени по URL, нови зависимости, обфускация или задни вратички. Промяната засилва валидацията на входа. Чисто.

Незадължителни бележки (не блокират merge)

1. Гранична стойност 000000000 — при независимия тест открих, че код от само нули минава контролната сума (сумата е 0, контролната цифра е 0 → самосъгласувано; това е математическо свойство на стандартния Булстат алгоритъм, а не грешка в имплементацията). Служебният 000000001 коректно пада, но чист placeholder от нули би бил приет като валиден ЕИК и би могъл да слее субекти. Ако такава стойност се среща в корпуса, струва си явен guard (напр. изключване на 000000000) като follow-up.
2. Дублиране на кода — ~40-редовият CASE за контролната сума е вписан 7 пъти. CLAUDE.md формално забранява дублиране; тук екстракцията е ограничена (няма UDF в този pipeline) и parity тестът смекчава риска, но всяка бъдеща корекция трябва да се приложи на всичките 7 места, иначе паритетът тихо се чупи. Струва си да се обмисли generated column / еднократно предварително изчислен eik_valid.
3. Видимост на спада в покритие — изключването на многоЕИК редовете премахва тези съвместни поръчки от всички тотали/rollups. Поведението е по спецификация, но добре е anomaly-report или docs да отбележат спада, за да не бъде разчетен като загуба на данни.

Забележка по верификацията

Зависимостите не са инсталирани в review worktree-а, затова цялостният пакетен suite и CI приемам по докладваното; критичната логика (контролна сума, mode, изключване, паритет) я потвърдих независимо локално. Моля, преди merge се уверете, че CI е зелен на последния commit.

Verdict: Approve на същество — блокери няма; трите бележки са незадължителни follow-up.

[ydimitrof]

Всичко проверено локално. Извеждам финалния коментар.

Ревю на PR #203 — fix(etl): authority/executor identity — canonical name, EIK checksum, multi-ЕИК guard (#194 #195 #196)

Благодаря за прегледно документирания и консолидиран PR. Прегледах го стриктно с фокус върху сигурност, целостност на данните и OWASP съответствие, и валидирах критичните части независимо и локално в node:sqlite (същия енджин като ETL-а), а не само по описанието. Съпоставих и с текстовете на трите issue-та.

Съответствие с issue-тата

• fix(etl): каноничното име на възложителя (MIN на името) подвежда при споделен ЕИК — поръчки на МОН излизат под училище #194 (каноничното име) — искането е mode вместо MIN(); реализирано точно, с детерминиран tiebreak. ✅
• fix(etl): eik_valid е само синтактична проверка без контролна сума — служебни/сгрешени ЕИК обединяват несвързани изпълнители #195 (контролна сума) — искането е замяна на чисто синтактичната проверка с истинска Булстат контролна сума; реализирано. ✅
• fix(etl): многоЕИК-ови стойности в authority_eik се ключат като един несъществуващ възложител (~406 реда) #196 (многоЕИК списъци) — избрано е предложение 2 (консервативно изключване), както изрично е допуснато в issue-то; пълното разделяне (предложение 1) коректно е отложено с бележка. ✅

Какво потвърдих локално

Контролна сума на ЕИК/Булстат (#195) — извлякох точния CASE израз от normalize-raw.sql и го изпълних срещу известни стойности:

• Реални минават: 131106522 (Просвета), 000695114 (МОН), 2016195800141, 204799888, 831641791. ✅
• Фиктивни/сгрешени падат до eik_valid = 0: служебният 000000001, близнакът с грешна цифра 131106552, 111111111/222222222. ✅
• Новите фикстури 111111113 / 222222226 са истински валидни по контролна сума — смяната в refresh-slice.test.ts е коректна и запазва намерението „два различни субекта". ✅
• 9-цифрен (тегла 1..8 → fallback 3..10; втора 10 → 0) и 13-цифрен (водещите 9 валидни, после 2,7,3,5 → fallback 4,9,5,7) съвпадат с референтния алгоритъм.

Каноничното име по mode (#194) — репликирах ЕИК 000695114: mode избира „МИНИСТЕРСТВО НА ОБРАЗОВАНИЕТО И НАУКАТА" вместо училищния вариант; при равна честота печели смесеният регистър („Община Тест"); празните/whitespace имена се игнорират. Потвърдих и че GLOB '*[a-zа-я]*' действително хваща малка кирилична буква (тъй като UPPER()/LOWER() не сгъват кирилица) — коментарът е точен. ✅

МногоЕИК гард (#196) — NOT LIKE '%;%' изключва списъчните стойности; фантомният „000590540; 000582020" отпада. Проследих транзитивния ефект: обявления изискват EXISTS възложител, договорите — EXISTS обявление, затова съвместните поръчки отпадат от всички downstream rollups, а суровият staging остава непокътнат (проследимо). ✅

Вложената корелирана подзаявка за bidder_key — потвърдих локално, че SQLite резолвва вътрешния FROM (SELECT … c.contractor_eik …), който корелира към външния c през два нива на подзаявки; ключът се изчислява коректно (валиден ЕИК → eik:…, невалиден → name:…). ✅

Паритет — контролната сума е байт-идентична на всичките 7 места (3 в normalize-raw + 4 в refresh-slice); NOT LIKE '%;%' гардът е и в двата пътя; parity тестът пази съответствието „by construction". ✅

Сигурност / OWASP

Изцяло статичен ETL SQL — няма динамично конкатениран от вход SQL, няма интерполирани стойности (SQL injection неприложим, A03), няма секрети, промени по URL, нови зависимости, обфускация или задни вратички. Сканирах diff-а за опасни конструкции (DROP/ATTACH/load_extension/креденшъли) — чисто. Промяната всъщност засилва валидацията на входа. CI е зелен на последния commit (72c5886).

Незадължителни бележки (не блокират merge)

1. Дублиране на кода — ~40-редовият CASE за контролната сума е вписан 7 пъти. CLAUDE.md формално забранява дублиране; тук екстракцията е ограничена (няма UDF в pipeline-а) и parity тестът смекчава риска, но всяка бъдеща корекция трябва да се приложи на 7-те места, иначе паритетът тихо се чупи. Струва си като follow-up да се обмисли generated column / еднократно предизчислен eik_valid.
2. Видимост на спада в покритие — изключването на ~405 многоЕИК реда премахва тези съвместни поръчки от всички тотали/rollups. По спецификация е, но е добре anomaly-report или docs да отбележат спада, за да не се разчете като загуба на данни.

Verdict: Approve на същество — блокери няма; двете бележки са незадължителен follow-up.

[ydimitrof]

Преглед на PR: fix(etl): authority/executor identity (#194 #195 #196)

Резюме на промяната. PR-ът решава три отделни проблема в ETL идентичността, и трите добре мотивирани и документирани в SQL коментарите:

• #194 — каноничното име на възложителя вече е MODE (най-честият етикет) на ЕИК вместо MIN() (азбучно първи), с детерминистични tiebreak-ове (mixed-case > ALL-CAPS, после по-дълго, после лексикографски).
• #195 — въведена е реална проверка на контролната цифра на българския ЕИК/Булстат (9- и 13-цифрен), която замества чисто синтактичната „9/13 цифри" проверка и разделя погрешно слети доставчици.
• #196 — guard за multi-ЕИК списъци (NOT LIKE '%;%') при съвместни поръчки, който изключва фантомните авторитети.

Проверки по дименсии:

• Сигурност (Phase 0 + agent-level): CLEAN. Няма секрети, URL промени, зависимости или инжекция. SQL-ът е статичен (без интерполиран вход). ✅
• Коректност: Алгоритъмът за контролна цифра е коректен (9-цифрен: тегла 1..8, при остатък 10 → тегла 3..10, при втори 10 → 0; 13-цифрен изисква валиден водещ 9-цифрен + тегла 2,7,3,5 / fallback 4,9,5,7). Тестовите ЕИК-та са коректно обновени към валидни по checksum стойности (111111113, 222222226 — проверих ръчно, минават контролната цифра). ✅
• Тестове: Обновяването на refresh-slice.test.ts е задължително, защото старите фиктивни ЕИК-та (111111111/222222222) вече се отхвърлят от checksum-а. Има spomenat parity тест между пълния rebuild и инкременталния път. ✅
• Производителност: Добавени са индекси за корелирания MODE подзаявка — виж бележката по-долу за проверка дали реално се използват.
• Поддръжимост: Основната ми забележка е дублирането на checksum блока (виж inline коментар).

Композитна оценка: ~8.7/10. Логиката е коректна и добре обоснована. Не блокирам за сигурност, но повдигам две неблокиращи забележки (дублиране на код и потвърждаване на плана на заявката за MODE подзаявката) преди merge. Препоръка: COMMENT — адресирайте забележките или потвърдете, че са приемлив компромис.

Забележки

1. Дублиране на код (CLAUDE.md: NO CODE DUPLICATION). Идентичният ~30-редов checksum CASE блок се повтаря 7 пъти между normalize-raw.sql и refresh-slice.sql. Паритетът се пази само ръчно + parity тест. Тъй като чист SQLite скрипт не поддържа функции, разгледайте генериране на SQL от шаблон или регистрирана UDF, за да няма риск от разминаване при бъдеща корекция.
2. Производителност (за потвърждение). Корелираната MODE подзаявка стъпва върху UNION ALL derived table; уверете се с EXPLAIN QUERY PLAN, че добавените индекси реално се използват, а не се re-materializira обединението за всяка група.
3. Пълнота на данните (#196). Guard-ът мълчаливо изключва ~405 съвместни поръчки от всички downstream rollup-и — документирано и умишлено (deferred), но добавете метрика/лог за броя изключени редове, за да е видимо.

[lyubomir-bozhinov]

ЕИК checksum-ът (#195) е верен — валидирах го емпирично, не само по спецификация. Извадих eik_valid CASE-а точно както е тук и го пуснах в sqlite3 срещу реални 13-значни ЕИК от данните на ЦАИС ЕОП:

ЕИК	eik_valid
0006723430010 (реален клон), …030, …078, …097	1
0000934421416, 0001931150243 (реални)	1
000672343 (9-зн. база), 000695089 (ИО)	1
0006723430011 (счупен d13)	0
0006723439010 (счупен клон-digit)	0
0006723440010 (счупена база)	0
-0416.453.068 (malformed)	0

Ръчна кръстосана проверка на 0006723430010: база 000672343 → пас1 124 % 11 = 3 = d9 ✓; клон позиции 9–12 = 3,0,0,1 → (2·3 + 5·1) = 11, 11 % 11 = 0 = d13 ✓. Тежестите (9-зн. 1..8 / 3..10; клон 2,7,3,5 / fallback 4,9,5,7 върху позиции 9–12), изискването база-да-е-валидна преди клона и ELSE 0 fallback-ите са коректни спрямо официалния Булстат алгоритъм. Без false-accept на едно-цифрени повреди. Отлична работа тук.

За скоупа (затова коментар, не одобрение):

1. #194 (каноничното име) — този PR се застъпва със #215 на същите SQL блокове (normalize-raw.sql + refresh-slice.sql); само единият влиза. Караме по #215 за #194 (носи ADR-0007 + curated override).
2. value-нормализация — PR-ът носи и over-valuation repair към procedure estimate (value_suspect). Това пипа суми и е отделна грижа от идентичността; влиза под value-integrity бара и заслужава собствен review + accept тест.
3. #196 остава отворен — тук е DEFERRED (редовете стават traceable, но не се сплитват по реален ЕИК).

Предложение: извади checksum-а (#195) в самостоятелен тесен PR върху текущия main — чист win, лесен за merge; #194 остава на #215; value-repair-ът в трети PR.

[StanislavBG]

Addressed all 3 threads:

• ЕИК checksum duplication — the 7 copies are gone; normalize-raw.sql/refresh-slice.sql are now generated (pnpm generate:sql) from *.template.sql + -- @include markers expanding a single scripts/lib/eik-valid.fragment.sql. A vitest test regenerates and diffs against the checked-in output so drift fails CI. (b4fecad)
• Canonical-name subquery perf — confirmed with EXPLAIN QUERY PLAN against a 4874-authority fixture: the correlated subquery was NOT index-seeking (SCAN raw_contracts/SCAN raw_tenders inside CORRELATED SCALAR SUBQUERY, despite the index) — ~46s on the fixture. Replaced with a precomputed GROUP BY + ROW_NUMBER() mode-name table joined in (SEARCH ... USING INDEX ... LEFT-JOIN); same fixture now runs in well under 100ms. Applied identically to both scripts. (b4fecad)
• Multi-ЕИК exclusion observability (fix(etl): многоЕИК-ови стойности в authority_eik се ключат като един несъществуващ възложител (~406 реда) #196) — the per-refresh anomaly report (already wired into both full and slice derive) now logs the count of excluded authority_eik lists + raw rows every run, so growth is visible. (935d59f)

Verified: pnpm --filter ./packages/db test (186/186) and pnpm --filter ./packages/db typecheck both green; pnpm lint/pnpm format clean.

[ydimitrof]

Ревю на PR #194/#195/#196 — authority/executor identity (обобщено)

Какво прави PR-ът

PR-ът консолидира идентичността на възложителите/изпълнителите в ETL слоя през три тикета:

• #194 — канонично име чрез MODE (най-често срещаното) вместо MIN, реализирано с оконна функция (ROW_NUMBER()) и детерминистични tie-break-ове; ключът по ЕИК остава непроменен, така че слуговете/URL-ите не се влияят.
• #195 — валидация на ЕИК/Булстат контролна сума (9- и 13-цифрена), с падане към ключ по име при невалиден код, за да се разделят погрешно слети субекти.
• #196 — консервативен multi-ЕИК guard (изключване на съвместните поръчки) + нова наблюдаемост в отчета за аномалии; пълното разпределяне е умишлено отложено за proposal 1.

Въведен е и единен източник на истина за контролната сума (scripts/lib/eik-valid.fragment.sql + generate:sql + тест за синхрон), който предпазва от дрейф между дублираните места.

Сигурност — ЧИСТО ✅

И трите партиди потвърждават: няма зашити тайни (курсът 1.95583 и публичните ЕИК на държавни субекти не са credentials), няма нови URL адреси или промени в зависимости, няма backdoor/обфускация. SQL-ът е изцяло статичен; конкатенациите вграждат стойности от колони, а @include е build-time — няма повърхност за SQL инжекция. Единствената защита в дълбочина: валидиране на името на фрагмента срещу path traversal в generate-sql.mjs (нисък риск).

Основни находки за потвърждение

Блокиращи / за изясняване преди merge:

1. Възможна тиха загуба на данни при канонично име (#194): новото име може да е NULL там, където старият MIN() даваше празен низ; при NOT NULL колона + INSERT OR IGNORE това може тихо да отпадне възложител и каскадно неговите поръчки/договори. Потвърдете дали authorities.name е nullable, или добавете fallback.
2. „EOP wins" защита колабира при NULL contract_number: COALESCE(...,'') дава '' = '' и подтиска всеки OCDS договор без номер, ако в прозореца съществува кой да е EOP ред без номер (същото в c2/c3). Потвърдете, че огледва normalize-raw.sql, или добавете тест за случая.
3. Липсващ индекс raw_contracts(contract_number): коментарът твърди, че idx_raw_cnum „кара seek-а", но такъв индекс не се създава в този файл — рискът е деградация до O(n²). Предлага се добавяне на CREATE INDEX IF NOT EXISTS.
4. Scoping vs. „SCOPED" декларация: няколко UPDATE-а (напр. bidders.ownership_kind без WHERE, enrich/touch-entities) минават през цялата таблица при всеки refresh. Ако raw_* е кумулативна, това е сериозна write-amplification и на практика отменя scoping-а. Потвърдете семантиката на raw_* (прозоречни vs. кумулативни).
5. Multi-ЕИК guard (#196): authority_eik NOT LIKE '%;%' изхвърля съвместните поръчки от authorities → риск от осиротели референции при downstream join; освен това хваща само разделител ;. Потвърдете продуктово, че пълното изключване (а не разпределяне) е приемливо до proposal 1 и че downstream логиката го понася.
6. Корелирана подзаявка за bidder_key: реферира външен c.contractor_eik от вложен FROM (SELECT …) без таблица. Уверете се, че версията на SQLite в D1 я поддържа и че parity тестът покрива именно този път (не само INSERT).

По-малки бележки

• Дублиране: ~15-редовият чексум израз е копиран 4 пъти (+ в normalize-raw.sql); паритетът се пази само от тест — оправдано от липсата на SQL UDF в SQLite/D1, но кандидат за генериране от единен шаблон.
• home_totals.bidders брои company_totals, което изключва bidders с amount_eur IS NULL — вероятно по замисъл.
• Пълни DELETE+recompute на flow_pairs, data_freshness, home_totals и др. — умишлено, но кандидат за бъдеща оптимизация при растеж.
• Дивизиите са коректно защитени (signing_value > 0, NULLIF(...,0)).
• Уверете се, че fx_rates(base_currency, rate_date) е индексиран заради повтарящите се FX подзаявки.
• Семантична бележка за етикета на брояча в anomaly-report.mjs (само наблюдаемост).

Качество / CLAUDE.md

Тестовете са смислени и консистентно обновени (тестовите ЕИК сменени на валидни по контролната сума — 111111113/222222226); няма cheater-тестове, мъртъв код или TODO-та; отложеното е ясно документирано като продуктово решение. Пълната преценка за паритет с normalize-raw.sql зависи от изпълнението на packages/db/src/refresh-slice.test.ts, което не бе възможно локално в рамките на партидите.

Обща оценка

Няма открити блокиращи проблеми по сигурност или зловреден код. Вердиктите по партидите варират от COMMENT до REQUEST_CHANGES заради нерешените въпроси по целостта на данните и производителността. Препоръка: изчистете точки 1–3 (тиха загуба на данни и липсващият индекс са с най-висок приоритет) и потвърдете точки 4–6 преди merge; останалите са за поддръжка/наблюдаемост.

[ydimitrof]

Ревю на PR #194/#195/#196 — обобщение

fix(etl): идентичност на възложител/изпълнител — каноничнo име, ЕИК контролна сума, multi-ЕИК guard

ВЕРДИКТ: КОМЕНТАР — няма блокиращи проблеми по сигурността, но има един въпрос за интегритет на данните (двойно броене) и един пропуск в тестовото покритие, които трябва да се изяснят преди финално одобрение. (Синтезът покрива партиди 1 и 2; окончателното одобрение изисква и партида 3.)

---

Какво прави PR-ът

Подобрява идентичността на възложители и изпълнители в ETL пайплайна чрез три свързани промени:

• #194 — каноничнo име: извеждане на канонично име на възложителя чрез MODE + прозоречен ROW_NUMBER с детерминистични tiebreak-ове и precompute във authority_canonical_name, което заменя скъпата correlated subquery; COALESCE(canonical_name, eik) пази NOT NULL и не изпуска редове.
• #195 — контролна сума на ЕИК/Булстат: валидиране на 9- и 13-цифрени ЕИК по българския стандарт; невалидните кодове (напр. фалшивия 000000001 и ~595 „не се публикува") се връщат към name-базиран ключ, за да не се слеят несвързани изпълнители.
• #196 — multi-ЕИК guard: изключване на списъчните authority_eik (NOT LIKE '%;%') от всички rollup-и, като raw staging остава непокътнат и проследим; консистентно с брояча FETCH_MULTI_EIK_EXCLUDED в anomaly-report.mjs.

Сигурност — ЧИСТО ✅

• Няма твърдо кодирани тайни, нови зависимости или нови/променени URL адреси (само промяна на референция към документация).
• Няма backdoor, обфускация или инжекция. SQL е статичен и се изпълнява като атомарен D1 batch върху staging таблици; стойностите от данните се записват като данни, а не се конкатенират в изпълним SQL — няма вектор за SQL injection (A03 неприложимо).
• Обхождане на пътища в generate-sql.mjs е коректно затворено (защита срещу /, \, ..) и покрито с регресионен тест.
• Алгоритъмът за контролна цифра е потвърден като коректен спрямо официалния стандарт.

Основни находки

1. (Изисква изясняване) Възможно двойно броене при EOP редове с NULL contract_number — normalize-raw.template.sql, dedup стъпка.
Дедупът на кумулативните дневни EOP кофи разчита на a.contract_number = c.contract_number. Коментарът твърди, че contract_number е гарантирано non-null от base keep-filter, но такъв филтър не се вижда в скрипта. Ако EOP ред има NULL contract_number, NOT EXISTS е винаги истина → кумулативните дубликати оцеляват и се броят многократно в EUR сумите. Моля потвърдете с COUNT(*) WHERE source LIKE 'eop:%' AND contract_number IS NULL; ако не е 0 — добавете явен NULL guard. (Забележка: смяната към bare = в refresh-slice.sql за OCDS↔EOP е коректна — там NULL≠NULL е желаното поведение.)

2. Липсва отрицателен тест за същината на #195. Тестовете упражняват валидния eik: път, но не и fallback-а при невалиден checksum (напр. 000000001 да НЕ колабира върху един bidder). Ако такъв тест не е в друга партида, препоръчвам изричен регресионен тест с таблица известни-валидни/невалидни ЕИК.

3. Дублиране на код (CLAUDE.md: NO CODE DUPLICATION). В refresh-slice.sql ~30-редовата checksum логика е инлайн-повторена поне 3 пъти, а големият value_flag CASE е дублиран между главния INSERT и pipeline_stats. Този файл не е template и няма @include, въпреки коментар обратното. Риск от разминаване при бъдеща промяна — препоръчва се същият генериран fragment механизъм за единен източник на истина. (За генерираните шаблони този проблем вече е адресиран и покрит с sync-тест.)

Второстепенни наблюдения

• Производителност: checksum и value_low клонът преизчисляват correlated под-заявки на ред (checksum двойно, fx_rates до ~3 пъти); за ~380k реда си струва проверка с EXPLAIN QUERY PLAN. Добавените индекси (idx_raw_cnum, idx_raw_tenders_tender_id, idx_raw_contracts_authority_eik) са уместни.
• Асиметрия при валидацията: изпълнителите минават през checksum, но възложителите се ключоват на 'auth:' || authority_eik без валидация — вероятно умишлено, но си струва да се потвърди в тикета.
• Остатъчна scratch таблица: authority_canonical_name е non-TEMP (обяснено с D1 batch поведение) и не се изтрива — приемливо, но заслужава коментар за почистване.

Заключение

Много добре изпълнена, документирана и тествана работа. Преди финално одобрение препоръчвам да се адресират: (1) потвърждаване/затваряне на риска от двойно броене при NULL contract_number и (2) регресионният тест за невалиден checksum.

[ydimitrof]

Ревю на PR — fix(etl): authority/executor identity (#194 #195 #196)

ВЕРДИКТ: COMMENT — няма блокиращи проблеми по сигурност или цялост. Има няколко въпроса за потвърждение преди merge.

---

Какво прави PR-ът

Уеднаквява идентичността на възложители/изпълнители в ETL пайплайна чрез три свързани промени:

• #194 — канонично име: извежда се като MODE per ЕИК през ROW_NUMBER() с детерминистични tiebreak-ове (вместо корелиран subquery) — по-коректно и с реално performance подобрение (нов idx_raw_cnum).
• #195 — ЕИК checksum: валидацията на контролната цифра е изнесена в scripts/lib/eik-valid.fragment.sql и се включва навсякъде чрез @include, което пази паритет с normalize-raw. Подходът „една дефиниция на едно място" е правилен.
• #196 — multi-ЕИК guard: authority_eik NOT LIKE '%;%' изключва съвместните поръчки от downstream агрегациите.

Кодът е с високо качество, много добре коментиран и покрит с тестове.

Сигурност (Phase 0 — ЧИСТО и в трите batch-а)

• Няма хардкоднати тайни, ключове, пароли или токени.
• Няма URL/мрежови промени (единствената „URL" промяна е доку-път в коментар) и няма нови зависимости.
• Целият SQL е статичен/шаблонен, върху вече заредени staging таблици — няма повърхност за SQL инжекция. Стойностите в state_owned_eik са референтни данни, не credentials.
• Path-traversal защитата за @include в generate-sql.mjs е солидна и директно покрита с тестове. Няма backdoor/обфускация/инжектиране на код.

Коректност — потвърдени силни страни

• ЕИК checksum алгоритъмът (9- и 13-цифрен) е независимо преизчислен срещу реални кодове и съвпада точно с fragment-а и инлайн копията.
• Фикстурите са уеднаквени с новата валидация (невалидните 111111111/222222222 → валидните 111111113/222222226).
• COALESCE(acn.canonical_name, s.authority_eik) предпазва от тихо изпускане на authority ред (и FK-свързаните tenders/contracts) при NULL име.
• Замяната на COALESCE(x,'')=COALESCE(y,'') с гола равенственост и премахването на NULL/NULL UNION-клона в refresh-slice е реална поправка на бъг — два различни договора с NULL contract_number вече не се сливат погрешно като дубликат.
• Паритетът между пълния rebuild и инкременталния път е поддържан и покрит с тест.

Въпроси за потвърждение преди merge (не блокиращи, но важни)

1. EOP dedup — риск от двойно броене (batch 2): кумулативната дедупликация в normalize-raw (стъпка 5) ключова е на a.contract_number = c.contract_number. Ако EOP ред може да носи NULL contract_number, редът избягва дедупликацията и сумите му се броят двойно. Коментарът твърди non-null „base keep-filter", но такъв не се вижда в файла. Моля потвърдете инварианта; ако NULL е възможен, добавете композитен fallback ключ (unp+lot+contractor) за NULL случая.

2. Multi-ЕИК guard — тиха загуба на покритие (#196): изключва ~405 съвместни поръчки и транзитивно техните tenders/contracts от всяка downstream агрегация. Умишлено е и вече е наблюдаемо чрез multiEikExcluded/anomaly-report. Потвърдете продуктово, че отложеното разделяне е приемливо за момента.

3. Подвеждащ обхват на enrichment (batch 3): enrich-authorities/enrich-bidders казват в коментара „touched by refreshed staging", но условието WHERE EXISTS (... parties p ...) обновява ВСИЧКИ entity с party — пълно сканиране при всеки refresh. Идемпотентно е, но коригирайте коментара или скопирайте по refresh_touched_*.

4. authority-region може да изтрие стойност: SET region = (SELECT ...) презаписва region с NULL, когато nuts е NULL или липсва в nuts_regions — на инкременталния път губи вече попълнен region. Обмислете COALESCE(..., region).

5. ownership_kind: нулира се на NULL при липса на съвпадение в state_owned_eik. Коректно е, ако allowlist-ът е единственият източник — моля потвърдете.

По-малки бележки

• Паритет на source-филтъра: normalize събира canonical name без source филтър, а refresh-slice филтрира eop:%/ocds:% — уеднаквете или потвърдете, че е без значение.
• authority_canonical_name е персистентна (не-TEMP) scratch таблица — документирайте я като служебна.
• Ред на изпълнение: state_owned_eik/seed трябва да е изпълнен преди normalize, иначе ownership_kind остава NULL.
• flow_pairs и search_index се преизчисляват изцяло при всеки refresh — най-скъпата част при растеж на корпуса; струва си мониторинг/SLA.
• Tiebreak GLOB '*[a-zа-я]*' не покрива цялата кирилска малка азбука (напр. ё) — само display, пренебрежимо.
• Per-row корелираният checksum subquery е по-скъп от стария синтактичен чек — струва си да се следи на пълния корпус.

Тестове

Покритието в batch 1 е много добро и смислено (NULL име на authority, два договора с NULL contract_number, скаларен bidder_id subquery, scope на ownership_kind, sync на генериран SQL спрямо шаблони, path-traversal guard-ове). Препоръка: добавете тест за идемпотентност (двукратно пускане на един прозорец → еднакви домейн редове). Общото покритие ≥90% не може да се потвърди от отделните batch-ове.

Заключение: Няма блокери по сигурност или цялост. Препоръчвам потвърждение по въпроси 1 и 2 (и по възможност 3–5) преди merge.

[ydimitrof]

Обобщено ревю на PR: fix(etl): authority/executor identity — canonical name, EIK checksum, multi-ЕИК guard (#194 #195 #196)

ВЕРДИКТ: COMMENT / необходими потвърждения преди merge — сигурността е напълно чиста, но има една потенциално блокираща бележка за целостта на данните (двойно броене) и няколко точки за потвърждение около инкременталния refresh.

Какво прави PR-ът

Уеднаквява идентичността на органи/изпълнители в ETL по три тикета:

• #194 — детерминистично каноничнo име на изпълнителния орган (MODE вместо MIN, с tie-break: cnt DESC → mixed-case → дължина → лексикографски).
• #195 — валидиране на ЕИК с контролна сума по стандарта БУЛСТАТ (9- и 13-цифрен алгоритъм с re-weight при остатък 10).
• #196 — консервативен guard срещу multi-ЕИК стойности (изключване на списъчните authority_eik).

Логиката на checksum-а е изведена в единствен източник (eik-valid.fragment.sql + pnpm generate:sql) и се вмъква build-time в SQL артефактите, със sync/паритет тест, който проваля CI при разминаване — добро решение срещу дублиране на кода на ~7 места.

Фаза 0 — Сигурност: ЧИСТО ✅ (и в трите партиди)

• Няма hardcoded тайни, нови/променени URL адреси или промени в зависимости.
• Няма SQL injection повърхност — всичкият продукционен SQL е статичен/генериран; @include е build-time вмъкване, не runtime конкатенация; стойностите идват от staging таблици.
• Path traversal в generate-sql.mjs е коректно адресиран (отхвърляне при /, \, .., само-точкови имена, basename(x) !== x), с негативни тестове.
• DELETE операциите в refresh са скоупнати (c:[eo]:* GLOB + refresh_touched_*), админските c: редове се пазят; bare-equality вместо COALESCE(...,'') избягва NULL-collapse при изтриване.
• Няма backdoor/обфускация/code injection. OWASP: няма релевантни находки.

Най-важна находка (потенциално блокираща)

Двойно броене при EOP dedup (Висок). В normalize-raw.template.sql (стъпка 5, EOP клон) и в EOP клона на refresh-slice дедупът разчита на голо равенство a.contract_number = c.contract_number, а коментарът предполага непразен contract_number, без явен филтър в самия файл. При NULL/празен contract_number условието NOT EXISTS е вакуумно вярно за всички копия от кумулативните дневни кофи → редовете не се дедуплицират → дублирани договори и двойно броене на EUR сумите (clean_total_eur_bn, authority_totals, company_totals). Моля потвърдете, че load-eop.mjs гарантира непразен contract_number; ако не — добавете резервен dedup (unp+lot+contractor+date) или WHERE contract_number IS NOT NULL.

Точки за потвърждение преди APPROVE

1. Скоуп на enrich в refresh (регресия при мащаб). enrich-authorities/enrich-bidders UPDATE използват WHERE EXISTS (parties p WHERE p.eik = ...), което обхваща всеки орган/участник с ред в постоянната parties таблица, изпълнявайки по 5 корелирани ORDER-BY подзаявки върху цялата таблица при всеки refresh — противоречи на инкременталната цел. Ограничете до refresh_touched_* или потвърдете умисъла.
2. OCDS lot_id не се нормализира за разлика от EOP (lot_norm). Ако OCDS raw_contracts.lot_id идва като LOT-3, FK към lots.id (lot:unp:3) няма да съвпадне → висящи lot референции; @refresh-batch lots вкарва редове само от raw_tenders. Потвърдете нормализация нагоре по потока или изравнете с EOP.
3. state_owned_eik се създава празна и не се seed-ва тук — ownership_kind зависи изцяло от нея; потвърдете, че се попълва от миграция/seed, иначе винаги ще е NULL.
4. Multi-ЕИК guard намалява покритието — филтрира само по ; и премахва ~405 списъчни ЕИК заедно с транзитивно свързаните tenders/contracts. Потвърдете, че ; е единственият разделител в източника (иначе фантомни authority редове при ,//), и логвайте отпадналия брой за проследимост.
5. ownership_kind scoping в refresh — за недържавен bidder от партидата подзаявката връща NULL; потвърдете, че предходно класифициран bidder не може да бъде „нулиран“ при повторно докосване.
6. Твърди прагове/пълни преизчисления — home_totals.first_date >= '2020-01-01' изключва договори преди 2020 (документирайте, ако е умишлено); flow_pairs се преизчислява изцяло при всеки refresh (потвърдете SLA при растеж).
7. Твърда зависимост от raw staging в anomaly-report (чете raw_contracts/raw_tenders) — buildAnomalyReport ще хвърля върху БД без raw staging (напр. публикуван slice); потвърдете контекста на runner-а.

По-малки бележки

• Scratch таблица: authority_canonical_name се създава, но не се трие — обмислете финален DROP (обоснованието защо е plain, а не TEMP, заради D1 batch, е прието).
• Производителност: checksum-ът за bidder_key е корелирана скаларна подзаявка на ред (2 места); pipeline_stats удвоява най-тежкото сканиране — приемливо за reconciliation, заслужава наблюдение. Precompute на mode чрез GROUP BY + ROW_NUMBER() е добро подобрение.
• amount_eur/NULL fx_rate: редът се вкарва, но се изключва от rollup сумите (amount_eur IS NOT NULL) — прието поведение.
• CI/паритет: осигурете, че генерираните .sql артефакти са регенерирани и committed, а паритет-тестовете (refresh-slice.test.ts) покриват ВСИЧКИ разгънати места на checksum-а (bidder INSERT, двата bidder_key, touched-bidders) и са зелени, с покритие ≥90% за новата логика.

Обща оценка

Висококачествена, добре документирана и добре тествана промяна с ясна проследимост към тикетите. Сигурността е чиста. Merge се препоръчва след адресиране на EOP dedup риска (двойно броене) и потвърждаване на точките около инкременталния refresh (scoping на enrich, OCDS lot нормализация, seed на state_owned_eik) при зелен паритет-тест.

Забележка: част от claim-овете (покритие на тестове, CI паритет) не можаха да бъдат изпълнени локално и трябва да се потвърдят в CI.

[ydimitrof]

Тази нова заявка въвежда твърда зависимост от raw_contracts/raw_tenders. buildAnomalyReport вече ще хвърли изключение (и ще счупи целия отчет), ако бъде извикан върху БД, в която raw staging таблиците не съществуват — напр. публикуван slice. Моля потвърдете, че anomaly report се изпълнява само в контекст, където raw таблиците присъстват; в противен случай обвийте заявката така, че липсата им да връща {authorities: 0, rows: 0} вместо да проваля отчета.

[ydimitrof]

authority_canonical_name е plain (не TEMP) scratch таблица — създава се тук, но не се трие в края на скрипта, така че остава в БД до следващото изпълнение (когато DROP TABLE IF EXISTS я пресъздава). Обосновката за plain таблица заради D1 batch е разумна, но обмислете финален DROP TABLE authority_canonical_name; след като бъде join-ната в authorities, за да не се разнася временно състояние в изходната/публикуваната БД.

[ydimitrof]

Multi-ЕИК guard-ът (WHERE s.authority_eik NOT LIKE '%;%') филтрира само по разделител ;. Ако изходните данни някога съдържат списъци с друг разделител (напр. , или /), такъв ред ще премине guard-а и ще създаде един фантомен authority за целия списък — точно регресията, която #196 цели да предотврати. Моля потвърдете, че ; е единственият разделител на съвместни поръчки в източника, или разширете условието.

[ydimitrof]

Голото равенство тук е коректно за посоката „OCDS отстъпва на EOP“ (NULL contract_number в OCDS ред → редът се запазва). Обърнете внимание обаче на симетричния случай в normalize-raw.template.sql стъпка 5 (EOP self-dedup): там същият модел с гола равенство разчита EOP contract_number да е винаги непразен, иначе кумулативните дневни копия не се дедуплицират и сумите се броят двойно. Моля потвърдете, че load-eop.mjs гарантира непразен contract_number, или добавете резервен dedup за NULL случая.

[todorkolev]

Благодаря, @StanislavBG. Затварям този PR, но не защото работата е отпаднала - напротив, тя влезе в основата на решението.

Какво стана. #203 обединяваше три отделни проблема (#194 име по мода, #195 контролен код по ЕИК, #196 многоЕИК). Разделихме ги на по един PR на проблем, за да може всеки да се прегледа и върне поотделно:

• feat(etl): канонична идентичност на субектите — име и вид по мода #251 - канонична идентичност: име и вид по мода (fix(etl): каноничното име на възложителя (MIN на името) подвежда при споделен ЕИК — поръчки на МОН излизат под училище #194)
• feat(etl): контролен код по Булстат за идентичност по ЕИК #252 - контролен код по Булстат за идентичност по ЕИК (fix(etl): eik_valid е само синтактична проверка без контролна сума — служебни/сгрешени ЕИК обединяват несвързани изпълнители #195)
• feat(etl): съвместни поръчки — приписване към всеки съвъзложител #253 - съвместни поръчки, приписване към всеки съвъзложител (fix(etl): многоЕИК-ови стойности в authority_eik се ключат като един несъществуващ възложител (~406 реда) #196)

Какво от тук е взето директно. Правилото за подредба при равен брой в #251 е твоето от този PR: първо с малки букви пред изцяло главни (през GLOB диапазон *[a-zа-я]*, защото UPPER()/LOWER() в SQLite не сгъват кирилица), после по-дългият надпис, после по азбучен ред. Диагнозата ти по случая с ЕИК 000695114 - че губи заглавието си рядък вариант, а не че връзките са сгрешени - е точна и е причината поправката да е само на надписа, без да се пипа ключът.

Извеждането на контролния код в един общ фрагмент вместо седем копия също остана като идея - в #252 то е направено през междинна таблица contractor_identity, която смята кода веднъж на двойка ЕИК+име, вместо през генериране на SQL от шаблони. Различен механизъм, същата цел: една дефиниция, а не седем.

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

Извинявам се, че отговорът дойде толкова късно спрямо отварянето на PR-а.