fix(etl): canonical authority name by frequency-mode + curated override, not MIN() (#194)

[cefothe]

Closes #194.

Проблем

Възложителите се дедупликират по ЕИК (коректно), но изписваното име се избираше с MIN(authority_name) — азбучно най-ранния низ. При споделен ЕИК това дава грешен надпис: под ЕИК 000695114 (МОН) „БСУ „Д-р Петър Берон"" изпреварва „Министерство…" (в кирилицата „Б“ е преди „М“), затова профилът на МОН (285 договора, ~296 млн. EUR) излизаше като чуждестранно училище — при положение, че министерското име доминира по брой редове (620 договорни / 444 обявления).

Решение

• normalize-raw.sql (стъпка 1) + refresh-slice.sql: каноничното име е честотната мода (най-често записаният вариант), а не MIN(). Тайбрек: по-кратко име (концизната форма пред композита „дете – родител“), после лексикографски → детерминистично.
• seed-authority-names.sql: курирана таблица authority_name_overrides (по модел на state_owned_eik), която пинва авторитетното име за споделен-ЕИК случаите; заредена и в трите пътя на import.mjs преди normalize/refresh. Прагматичен заместител, докато Търговският регистър не влезе.
• Refresh път: override-ът коригира и съществуващи редове и маркира засегнатия възложител като „touched“, за да се пресметнат наново rollup + FTS.
• Само надпис: id (auth:'||ЕИК) и URL на профила остават стабилни; подобрява и търсенето (Качество на търсенето: възложител с дълъг списък имена изпреварва точните съвпадения #25).
• Документация: docs/etl.md + ADR-0007.

Компромиси / обхват

• Регистров lookup (option 2) и под-профили (option 4) са отложени — източникът raw_tr_companies не е зареден; училището в чужбина легитимно няма собствен ЕИК.
• MAX(authority_type) остава произволен-но-стабилен (храни само type_group) — извън обхвата.
• Родствените fix(etl): eik_valid е само синтактична проверка без контролна сума — служебни/сгрешени ЕИК обединяват несвързани изпълнители #195 / fix(etl): многоЕИК-ови стойности в authority_eik се ключат като един несъществуващ възложител (~406 реда) #196 нарочно не са част от този PR.

Проверка

• SQLite fixture-и: модата избира министерството, тайбрекът отхвърля композита, all-null имена не чупят, override-ът се прилага, а touched-маркирането в refresh е scoped + идемпотентно.
• check-docs.mjs минава; load-eop + check-docs unit тестове 16/16; node --check и Prettier чисти.
• Само-надпис → authority_totals/flow_pairs се ключат по authority_id, integrity gate не се променя.

[cefothe]

Ревю — fix(etl): canonical authority name by frequency-mode + curated override (#194) — ✅ Одобрявам

Обхват: normalize-raw.sql, refresh-slice.sql, нов seed-authority-names.sql, свързване в import.mjs (и трите пътя), ADR-0007 + etl.md. Един логически проблем, промяна само на надписа.

Сигурност

Чисто — без тайни, без URL-и, без нови зависимости. SQL е статичен (без интерполация на потребителски вход); бележката в seed-а ползва типографски кавички „…", така че няма проблем с escaping.

Проверено чрез изпълнение

Тъй като node_modules не е инсталиран, валидирах реалния SQL срещу същия sqlite3 3.51, който тестовете ползват, възпроизвеждайки казуса със споделен ЕИК:

Проверка	Резултат
Честотната мода бие MIN() — МОН (620 реда) печели пред „БСУ…" (Б<М)	✅
Празни низове се филтрират от модата (777 → „Реално име", не '')	✅ подобрение спрямо стария MIN()
ЕИК само с NULL имена се пропуска без срив (INSERT OR IGNORE)	✅
Override пинва името; IS NOT guard-ът в refresh обновява само различаващия се ред	✅
Touched-маркирането обхваща само коригираните от override редове	✅

Проследих разпространението и по двата пътя: full (normalize мода+override → precompute пре-строи authority_totals/flow_pairs/search_index от authorities.name) и slice (refresh прилага override върху съществуващи редове → маркира touched → authority_totals за touched → search_index изцяло от totals; flow_pairs се пре-строи изцяло). Всеки надпис надолу стига до новото име — touched-маркирането на ред 69 е необходимата връзка и е коректно ограничено/самоограничаващо се.

Силни страни

• Детерминистичен избор чрез пълна наредба (COUNT(*) DESC, LENGTH ASC, name ASC) — стабилен между rebuild-и, идентично правило в двата пътя.
• Самодостатъчен SQL: CREATE TABLE IF NOT EXISTS authority_name_overrides и в двата скрипта — точно това пази refresh-slice.test.ts зелен, ако seed-ът е пропуснат.
• Коректен рефактор на src филтъра (source LIKE … вкаран във всеки UNION клон — семантично еквивалентен на стария post-union WHERE).
• Отлично ADR-0007 — документира отхвърлените алтернативи (регистров lookup недостъпен; под-профили искат ключ отвъд ЕИК; case-fold tiebreak ненадежден заради ASCII-only UPPER), и коректно отбелязва, че промяната е само на надписа, така че integrity gate-ът минава без промяна.

Незадължителни бележки

1. Няма отделен регресионен тест за fix(etl): каноничното име на възложителя (MIN на името) подвежда при споделен ЕИК — поръчки на МОН излизат под училище #194. Съществуващият тест пази изпълнението на batch-овете, но не пинва поведението мода>MIN. Малък тест със споделен ЕИК, който проверява печелившото име, би заключил поправката за в бъдеще.
2. Корелирана подзаявка в mode INSERT (COALESCE((SELECT … rn=1), MIN(…))) прави по едно търсене на всяка група; LEFT JOIN name_mode … AND rn=1 е един pass. Работи приемливо на реалния корпус (цитирани 620/444) — бъдещо опростяване, не дефект.
3. MIN() fallback теоретично може да върне '' при ЕИК само с празни низове — козметично, невероятно, не по-зле от досегашното.

Заключение: Солидна, добре документирана поправка само на надписа с коректно разпространение надолу. Препоръчвам добавяне на един насочен тест за случая със споделен ЕИК, преди да стане критична зависимост.

[ydimitrof]

Преглед на PR #194 — канонично име на възложителя (мода + курирана override, вместо MIN())

Обобщение

Много добре обоснована и добре документирана промяна. Проблемът е реален (MIN() над кирилски текст избира азбучно най-ранния низ и при споделен ЕИК име на второстепенен разпоредител измества родителския орган), а решението — честотна мода с детерминистичен тайбрек (COUNT(*) DESC, LENGTH ASC, name ASC) плюс курирана override таблица — е коректно и прагматично. ADR-0007, docs/etl.md и seed скриптът дават отличен одит на решението и на отхвърлените алтернативи.

Сигурност (Фаза 0)

• Няма hardcoded secrets, нови зависимости или промени по URL. Единствената „твърдо кодирана“ стойност е ЕИК 000695114 (МОН) в seed таблицата — публична данна, не тайна. Няма SQL injection повърхност (статичен SQL, без вход от потребител). CLEAN — не блокира.

Коректност

• SQL логиката за модата е коректна и идентична между normalize-raw.sql (стъпка 1) и refresh-slice.sql. COALESCE(...MIN()) fallback покрива ЕИК само с празни/NULL имена. id (auth:<ЕИК>) и профилният URL остават стабилни — промяната е наистина само на надписа.
• Виж инлайн бележки за: (1) INNER JOIN към authority_totals при маркирането на „touched“ в refresh; (2) фрагментиране на гласовете при варианти, различаващи се само по регистър/интервали; (3) дублиране на mode-логиката и CREATE TABLE в три файла (риск от разминаване).

Тестове

• В PR-а липсват автоматизирани тестове, които да фиксират новото поведение (мода печели пред MIN, override пинва името, стабилност на id/URL, детерминизъм между rebuild-и). ADR се позовава на integrity gate-а, но той проверява редове/EUR, не избора на име. Препоръка: fixture с споделен ЕИК + assert, че каноничното име е модата/override-а. Това е основната причина да не давам APPROVE спрямо изискването за покритие.

Изход

COMMENT — промяната е издържана и готова по същество; моля адресирайте бележката за INNER JOIN (реален, макар граничен пропуск за FTS refresh) и добавете поне един целеви тест.

[ydimitrof]

Възможен пропуск при маркиране на „touched“. INNER JOIN authority_totals at означава, че възложител, който няма ред в authority_totals (напр. без сумирани договори/обявления), няма да бъде маркиран като touched дори когато override-ът току-що е сменил името му. Коментарът по-горе обещава да се пресметнат наново „rollup + FTS“, но search_index/FTS редът за такъв възложител няма да се опресни. Ако инвариантът е, че всеки възложител в authorities винаги има ред в authority_totals, това е безопасно — но си струва да се потвърди, или да се използва LEFT JOIN / отделно маркиране на override-натите редове директно (те вече са известни от предходния UPDATE).

[ydimitrof]

Фрагментиране на гласовете при модата. GROUP BY authority_eik, authority_name брои всеки различаващ се низ като отделен вариант, затова именувания, различаващи се само по регистър или интервали (напр. МОН срещу мон, или trailing spaces), разделят гласа и може да занижат истинската доминираща форма. За текущия казус (#194) курираната override таблица компенсира, но си струва бележка/follow-up за нормализация на белите полета/регистъра преди преброяването, за да е модата по-устойчива в общия случай.

[ydimitrof]

Дублиране на дефиницията и на mode-логиката. CREATE TABLE IF NOT EXISTS authority_name_overrides и целият mode SQL сега съществуват в три файла (seed-authority-names.sql, normalize-raw.sql, тук). Дублирането е съзнателно (self-sufficiency), но носи риск от разминаване — ако тайбрекът/схемата се промени на едно място. Ако проектът има начин за споделен SQL fragment/include, обмислете го; иначе поне добавете кръстосана бележка, че трите копия трябва да се редактират заедно (частично покрито от коментарите).

[lyubomir-bozhinov]

За #194 сравних този PR със #203 (двата се застъпват на същите SQL блокове — само единият може да влезе). Този е по-чистият избор и е коректен: честотен режим с детерминиран tiebreak (ROW_NUMBER() OVER (PARTITION BY authority_eik ORDER BY COUNT(*) DESC, LENGTH(authority_name) ASC, authority_name ASC)), закърпени са и normalize-raw.sql, и refresh-slice.sql — daily refresh пътят носеше същия MIN() bug, иначе имената пак се чупят на следващия cron — плюс ADR-0007 и authority_name_overrides таблица.

Дребно (не блокира): финалният COALESCE(..., MIN(s.authority_name)) fallback е практически недостижим при непразно име (name_mode винаги дава rn = 1 ред) — ОК като защита.

Издържано — това е линията за #194. (Забележка: ЕИК checksum-ът за #195 от #203 е верен и заслужава да се пренесе в самостоятелен PR — виж коментара ми там.)

[todorkolev]

@cefothe - затварям този PR, и започвам с това, което е наш пропуск: по #194 стояха отворени три PR-а едновременно - твоят #215, #203 на @StanislavBG и #251 от мен. Не сме ги свързали навреме и си писал срещу проблем, по който вече е имало работа. Съжалявам за загубеното време - това е наша организационна грешка, не твоя.

Какво влиза вместо него. #251 (канонична идентичност - име и вид по мода) е част от поредица #251 → #252 → #253, която пипа същите места в normalize-raw.sql и refresh-slice.sql и решава още два свързани проблема. Не можем да държим два различни избора за едно и също име, докато строим базата върху единия.

Два въпроса по същество, където се разминаваме - казвам ги открито, защото на единия може да си прав:

1. Подредбата при равен брой. Ти взимаш по-краткия надпис (LENGTH ASC), #251 взима по-дългия и предпочита с малки букви пред изцяло главни. Влиза в сила само при равенство, но дава различно име. Ако смяташ, че съкратената форма е по-добра за показване, кажи го - това е редакторски избор, не техническа истина, и ако имаш аргумент, ще коригираме #251.

2. ADR-0007 отхвърля точно подхода, който ползваме - но на основание, което е заобиколено. В документа пишеш, че тайбрек „предпочети смесен регистър" е ненадежден, защото UPPER/LOWER в SQLite са само-ASCII и не сгъват кирилица. Това е вярно за UPPER. #251 обаче не вика UPPER изобщо - засича малки букви през GLOB диапазон *[a-zа-я]*, който работи и за кирилица. Тоест пречката, заради която си отхвърлил варианта, не съществува при това изпълнение.

За таблицата с презаписи. Единственият запис в seed-authority-names.sql е МОН, а по твоята собствена бележка модът вече го възстановява (620 договорни реда срещу 444 обявления). Тоест днес тя пинва нещо, което и без нея излиза вярно. Оправданието ѝ е за случаите, където модът още бърка - но такъв случай още не е показан, а и не може да се измери преди пресъздаването на базата. Затова я оставяме настрана засега: ако след пресъздаването излязат ЕИК-ове, където модът дава второстепенния разпоредител вместо органа, механизмът ти е готовият отговор и ще го върнем.

Отделно: #251 добавя ordering_unit_name - оригиналното име от документа се пази винаги и се показва под каноничното. Така при споделен ЕИК името на второстепенния разпоредител не се губи, а се вижда. Това покрива другата половина на проблема, който описваш.

Какво остава от работата ти. ADR-ът трябва да се напише за решението, което наистина пускаме, и ще го напиша аз - но описанието на случая с ЕИК 000695114 и анализът на отхвърлените варианти са твои и ще влязат с позоваване. Двата аргумента там са силни и ги приемам изцяло: регистровият lookup отпада, защото raw_tr_companies не е зареден и бюджетните органи често липсват и в регистъра; под-профилите на второстепенните разпоредители отпадат, защото искат ключ отвъд ЕИК, а училище в чужбина легитимно няма собствен ЕИК. Това е анализ, който не бях направил.

Ако предпочиташ сам да напишеш ADR-а за окончателния вид - кажи и е твой.