Skip to content

docs(web): методологията описва точно таблата — формули, обхват, изключения - #193

Open
StanislavBG wants to merge 39 commits into
midt-bg:mainfrom
StanislavBG:docs/methodology-dashboards
Open

docs(web): методологията описва точно таблата — формули, обхват, изключения#193
StanislavBG wants to merge 39 commits into
midt-bg:mainfrom
StanislavBG:docs/methodology-dashboards

Conversation

@StanislavBG

@StanislavBG StanislavBG commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Какво

Разширява /methodology с нов раздел „7. Аналитичните изгледи: какво показват и как се смятат" — по едно описание за всяко аналитично табло, сверено ред по ред срещу кода на loader-ите и заявките, а не преразказано по памет.

За всяко табло: какво показва визуалът, формулата с думи, обхватът и изключенията, и честните уговорки — в съществуващия тон (неутрален, описателен, „ориентир, не заключение"), на български, с препратки към речника и раздела за стойността.

Обхванати табла: Анализи (индекс), Договори — обзор (три ъгъла + типични цени по CPV), Конкуренция (една оферта + HHI), Раздуване след анекси, Потоци, Карта, Мрежа, Договори.

Ключови сверявания срещу кода:

  • Стойностна основа = изчистената стойност в евро (текуща при законен анекс, иначе при сключване); непотвърдените стойности се изключват от сумите.
  • Обзорът показва само реални стойности — без прогноза; текущият непълен период е скрит по подразбиране; YoY само между съседни години.
  • „Спрямо типичното" = стойност спрямо медианата на CPV класа (процентил, не z-score).
  • Раздуване = поне един анекс ∧ текуща > подписана ∧ подписана ≥ 1 000 €; растежът е претеглен по евро.
  • Конкуренция: една оферта (bids_received = 1, само с известен брой оферти, ≥ 20 договора); HHI = сбор от квадратите на дяловете (0–1, ≥ 0,25 висока концентрация, ≥ 2 доставчика).
  • Мрежа: шест най-големи преки контрагенти по стойност + по една втора връзка (фокусирана околност, не целият граф).
  • Карта: област по адреса на институцията (NUTS), непосочените — извън картата.

Обхват

Основно документация — methodology.tsx е единственият файл, променен от първоначалния commit.
Ревюто по-долу добави осем дребни code fixes в извън-documentation файлове, открити при
сверката с кода (виж „Ревю"); никой от тях не променя поведение извън описаните поправки.

Проверки

  • pnpm --filter @sigma/web typecheck — зелено
  • prettier --check . — зелено
  • pnpm --filter @sigma/web test — 377 теста зелени

Ревю (ydimitrof, 2026-07-20)

12 коментара — всички разгледани и затворени:

  • 9 приети като code fixes, слети в 38847f9 (isomorphic layout-effect в MetricInfo, коректно
    третиране на partial точка на индекс 0 в ComboTrendChart, консистентен || fallback за ЕИК
    в overruns.tsx, защита от деление на нула и dedup на CPV групите в trends.tsx, поправени
    CSS маркер/дублиран селектор, премахнат неизползван import в cache-key.ts) и aeb950b
    (именуван authorityEik alias на authoritySlug в identity.ts за яснота — стойността остава
    непроменена).
  • 3 потвърдени вече коректни без промяна в кода (unit-конверсията в analytics-stats.ts,
    агрегацията година↔месец в estimateYoyGrowth, анкорите #source/#money в
    methodology.tsx) — обяснени в отговор на съответните нишки.

Всичките 12 нишки са resolved.

Зависимости

Draft: зависи от стека #169#172, защото описва таблата, които те въвеждат (петте карти на /analytics, обзорът на /trends, /overruns). Ще се извади от draft, след като стекът се слее.

The popover often renders inside a thead th whose white-space: nowrap is
inherited by every line of the card, so long summaries and the mono
readout ran past the right edge. Reset wrapping on .metric-info-pop
(white-space: normal + overflow-wrap: anywhere), widen the card to
320px clamped to the viewport (min(320px, 100vw - 16px)), and add the
JS shift-into-viewport + coarse-pointer 44px hit area so all copies of
the component behave identically.
The обзор cross lens now re-runs the combo chart, year cards and totals
server-side for the ticked CPV groups (repeatable ?cpv, validated
5-digit codes, deduped, capped at 10), and the chart stretches to fill
its card against the CPV list. One aggregate scan via an OR of half-open
cpv_code index ranges; ?cpv stays keyed in the edge cache (CWE-349) with
UI-canonicalized ordering, and selection changes are announced via an
sr-only status line.
The by-sector aggregate grouped on substr(t.cpv_code, 1, 2) and then ran
cpvDivision() over the already-truncated prefix, so a dirty code like
' 45000000' resolved to division '4' there while the leaderboard
(cpvDivision over the full code) resolved it to 45 — the same contract
landed in different CPV sectors on the two surfaces.

Select the full t.cpv_code, GROUP BY it in SQL, and run cpvDivision()
on the full code in JS exactly like the leaderboard mapping; the
existing merge folds all codes onto their canonical division and
re-applies secLimit post-merge. Test covers a dirty leading char
flowing through both surfaces to the same division.
…he full cpv_code

Grouping by the full cpv_code fixed the dirty-code divergence but returned
one row per distinct 8-digit code — 582 rows on the local corpus (thousands
at scale) shipped out of D1 for a 15-row table. SECTOR_KEY_SQL strips the
separator characters real codes carry and takes substr(clean, 1, 2) only
when the cleaned code provably starts with two digits (in which case it IS
cpvDivision(code)); anything else falls through as the full raw code for
the JS re-key to fold in. Exact cpvDivision semantics, division-sized
result set, still no pre-merge LIMIT truncation.

Adds a sqlite3-CLI integration test pinning SECTOR_KEY_SQL ≡ cpvDivision
across the dirt corpus, and a unit test pinning the GROUP BY key against
both traps (naive substr, full-code blowup).
Every column header in „Кои институции раздуват най-много" and „Раздуване
по сектори" now carries the same ⓘ MetricInfo popover the headline KPIs
use: what the metric is, how it is computed (grounded in the SQL — growth
is SUM(delta)/SUM(signing), €-weighted, not an average of percents), and
the honest inclusion caveat (annex_count > 0, current > signing,
signing ≥ €1 000). Table header cells get white-space: nowrap so the ⓘ
never wraps the label.

Also: the „Договори по мащаб" leaderboard rows gain right-side breathing
room (row padding-right var(--s-3)) so value text and truncated titles no
longer touch the card edge; the inset lives on the row, so the selected
highlight still reads full-width and the proportional bar-track math is
untouched in normal and fullscreen mode.
…ючения

Добавя раздел „Аналитичните изгледи" в /methodology с описание на всяко
табло (Анализи, Обзор, Конкуренция, Раздуване, Потоци, Карта, Мрежа,
Договори): какво показва визуалът, формулата с думи, обхватът и
изключенията, и честните уговорки — сверено ред по ред срещу кода.

Само документация; без промяна в поведението.
@StanislavBG

Copy link
Copy Markdown
Contributor Author

@todorkolev готов за ревю 🙏 — rebase-нат на main, CI зелен, prettier-чист, CSS промените в styles/* (app.css само @import). Резолвнати нишки. Approve-ни когато ти е удобно.

@ydimitrof

Copy link
Copy Markdown
Contributor

Проверих PR #193 внимателно — целият собствен diff, всяко фактическо твърдение в текста срещу кода на loader-ите и заявките, плюс сигурност и цялост на данните.

Обхват на промяната

Въпреки че diff-ът срещу main показва 41 файла и ~9 600 реда, това е стековащ се PR: реалната собствена промяна е един-единствен комит (7413f87) и един файл — apps/web/app/routes/methodology.tsx (+147/−4). Останалото е стекът #169#172, върху който клонът е базиран и който все още не е слят. Ревюто по-долу е за собствената промяна; сигурността на подлежащия стек е покрита в ревютата на съответните PR-и.

Сигурност и цялост на данните

  • Собствената промяна е чист JSX текст — няма SQL, няма изпълним код, няма мрежови повиквания, няма нови зависимости, няма четене на вход. Повърхността за SQL-инжекция, XSS или друг exploit е нулева. Целият текст е статичен и React-escape-ва по подразбиране.
  • Няма тайни, URL промени, обфускация или каквито и да е злонамерени модели. OWASP: неприложимо на ниво код за този diff, нищо тревожно.
  • Пренномерирането на секциите (7→8, 8→9, 9→10, 10→11) е коректно и консистентно — id-тата, aria-labelledby и TOC записите съвпадат.

Сверяване на твърденията срещу кода (цялост)

Тъй като PR-ът обещава „сверено ред по ред срещу кода", проверих локално всяко число и формула. Всички съвпадат точно:

  • РаздуванеOVERRUN_MIN_SIGNING_EUR = 1000, annex_count > 0, current > signing; растежът е SUM(delta)/SUM(signing) (претеглен по евро, не средно на процентите) — overruns.ts:153–163. ✔
  • Конкуренцияbids_received = 1, праг DEFAULT_MIN_CONTRACTS = 20, HHI = сбор от квадратите на дяловете, suppliers >= 2, отбелязване при hhi >= 0.25competition.ts:31,68,183,186 + competition.tsx:95. ✔
  • ОбзорSTART = '2020-01-01', бъдещи периоди изключени, текущият частичен период скрит по подразбиране с opt-in toggle, YoY само между съседни години и потиснат за непълната година, без прогноза — trend.ts:52,166–233. ✔
  • „Спрямо типичното"mult >= 1.3 → над, <= 0.75 → под; съотношение спрямо медианата на CPV класа, не z-score — trends.tsx:105–107. ✔
  • Цени по CPV — медиана + диапазон p10–p90, петцифрен клас, топ-N по брой — trend.ts:324–338. ✔
  • МрежаHOP1 = 6 преки контрагенти + по една втора връзка (LIMIT 1 на съсед) — network.ts:25,175. ✔
  • Потоци — топ 20/50 двойки — flows.ts:181 + flows.tsx:97–99. ✔
  • Карта — 28 NUTS3 региона, област от authorities.region (адрес/NUTS), непосочените в отделен bucket извън картата, покритието се отчита — regions.ts:5,52,66. ✔

Не открих нито едно разминаване между текста и поведението на кода. Уговорките („ориентир, не заключение", „ръстът не е нередност", „типичната цена е само ориентир") са честни и точно поставени — точно това, което искаме от методологична страница пред граждани.

Бележки (незадължителни, не блокиращи)

  • PR-ът е draft и mergeStateStatus: BLOCKED — по дизайн зависи от стека feat(web): dashboard design-system base (MetricInfo, fullscreen, tokens, overrun index) #169feat(web): analyze index — five equal analysis cards #172 (описва таблата, които те въвеждат). Трябва да се слее след стека, иначе /methodology ще описва изгледи, които още ги няма. Регресията с .lens-* стиловете от feat(web): dashboard design-system base (MetricInfo, fullscreen, tokens, overrun index) #169 е разрешена вътре в този стек (5f64191, c0521bd, eaeeb2c).
  • Дребна езикова прецизност: текстът нарича „спрямо типичното" процентил, а веднага след това коректно го описва като съотношение спрямо медианата. Двете не са едно и също понятие; кодът прави съотношение-спрямо-медиана. Не е грешка по същество (описанието е вярно), но думата „процентил" може да се изпусне за яснота.

CI е зелен, prettier/typecheck минават, 377 теста зелени (документационна промяна не добавя тестова повърхност — приемливо тук).

Благодаря за изрядно свършената работа — рядко се вижда методология, сверена буквално срещу заявките.

ВЕРДИКТ: Approve на същество — да се слее само след стека #169#172 (draft, blocked по дизайн). Няма проблеми със сигурността или целостта на данните.

@ydimitrof

Copy link
Copy Markdown
Contributor

Одобрявам

Ревю на PR #193 — „методологията описва точно таблата"

Прегледах промяната строго, с акцент върху сигурност, цялост на данните и точност спрямо кода. Разграничих собствената промяна на PR-а от подлежащия стек.

Обхват — какво реално въвежда този PR

Дифът показва ~9,6 хил. реда, но собствената промяна е един-единствен комит (7413f87) само върху apps/web/app/routes/methodology.tsx (+147/−4). Останалото е стекът #169#172 / #192, който все още не е слят и затова се появява в дифа спрямо main — тези файлове са ревюирани отделно в своите PR-и и не ги преразглеждам тук. Правилно е обозначено като Draft със зависимост от стека.

Промяната е само документация: нов раздел „7. Аналитичните изгледи" в /methodology плюс коректно преномериране на следващите раздели (7→8, 8→9, 9→10, 10→11) и запис в TOC. Няма промяна в поведението.

Сигурност / OWASP

Чисто — тук няма атакуема повърхност:

  • Целият добавен JSX е статичен, вкоден текст. Няма dangerouslySetInnerHTML, няма интерполация на потребителски вход, няма нови href към външни адреси (само вътрешни <Link to="/…"> и котви #…).
  • Няма SQL, няма заявки, няма нови зависимости, няма тайни/.env.
  • A03 Injection / A07 / XSS: неприложими — React екранира текстовите възли; всички връзки са относителни и литерални.

Цялост на данните — сверих всяко твърдение ред по ред срещу кода

Това е същината при документация, която обещава „сверено срещу кода". Всички числени твърдения съвпадат точно:

  • Раздуванеannex_count > 0 ∧ current > signing ∧ signing ≥ €1 000 (overruns.ts:153–158, OVERRUN_MIN_SIGNING_EUR = 1000); растежът е SUM(delta)/SUM(signing), претеглен по евро (overruns.ts:106,162–164). ✔
  • Конкуренция — една оферта = bids_received = 1 спрямо договори с известен брой (competition.ts:68,73); праг ≥ 20 договора (DEFAULT_MIN_CONTRACTS = 20); HHI = сбор от квадрати на дяловете, ≥ 2 доставчика (competition.ts:183,186); отбелязване при ≥ 0,25 (competition.tsx:95). ✔
  • Обзор — текущият непълен период е скрит по подразбиране с opt-in toggle (trend.ts:186–190), YoY само между съседни години и пропуснат за непълната (trend.ts:220–233), начало 2020 г. (trend.ts:52), без прогноза. ✔
  • „Спрямо типичното"≥ 1,3× → над, ≤ 0,75× → под (trends.tsx:105–106); коректно описано като процентил, не z-score. ✔
  • Мрежа — шест преки контрагенти + по една втора връзка (network.ts:25 HOP1 = 6, ред 196). ✔
  • Карта — NUTS3, 28 области, изведени от адреса на институцията (~половината), с „unattributed" кофа и покритие (regions.ts:1–5). ✔
  • Потоци — top-N 20/50 двойки (flows.ts). ✔

Не открих нито едно разминаване между текста и поведението на кода.

Съответствие с описанието на PR-а

Описанието твърди „единственият променен файл е methodology.tsx" и „само документация" — потвърдено от собствения комит. CI е зелен (check pass, 1m51s), typecheck/prettier/тестове зелени според описанието.

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

  • Разделът дублира по смисъл праговете, които вече живеят в кода (напр. 1 000 €, 0,25, 1,3×/0,75×). Това е присъщо на документацията, но ако някой праг се промени в кода, този текст ще трябва да се обнови ръчно — няма автоматична връзка. Струва си да се спомене в PR-описанието като поддръжков дълг.
  • Тъй като е Draft, зависещ от стека: слейте едва след feat(web): dashboard design-system base (MetricInfo, fullscreen, tokens, overrun index) #169feat(web): analyze index — five equal analysis cards #172, за да не сочат котвите/описанията към табла, които още не са на main.

Отлична, честна и точна работа — тонът „ориентир, не заключение" е спазен навсякъде.

Одобрявам — на съществото; сливане след стека #169#172.

@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Ре-проверих раздел 7 срещу кода — една точна неточност по стойностната база (иначе HHI/конкуренция и праговете за раздуване са верни):

Формулировките, че договорите с непотвърдена стойност „се изключват от сумите" (увод на раздел 7, ~ред 408) и „не се сумират" („Договори — обзор", ~ред 536), са неверни за сайт-широките суми в евро:

  • normalize-raw.sql:357WHEN value_flag = 'value_suspect' THEN x.proc_est_eur: value_suspect редовете се repair-ват към прогнозната стойност (ненулев amount_eur), не се зануляват.
  • trend.ts:72,146 (WHERE amount_eur IS NOT NULLSUM(amount_eur), без value_flag='ok' филтър), competition.ts и home_totals ги сумират → влизат във всеки евро-тотал при прогнозната си стойност.

Т.е. договор с типо €5 млрд, маркиран value_suspect, НЕ се вади от /trends, /contracts, /competition, /regions — влиза при proc estimate. Читателят е упътен обратното. Коригирайте двете места: value_suspect договорите се включват при прогнозната си стойност (или „суровата стойност се заменя с прогнозна", ако това е замисълът).

NB: за „Раздуване след анекси" (~ред 482) твърдението е вярно — там annex_suspect има NULL current_value_eur и наистина отпада. Само сайт-широките суми са грешно описани.

Both the section-7 intro and the /contracts description said implausible-value contracts are
excluded from site-wide EUR sums. They're actually repaired to the procedure's estimated value and
included (normalize-raw.sql:357) — trend/competition/home totals sum them at that value. Only the
overrun view is correctly described as excluding them (no current value to compare against).
@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Благодаря — §7 вече е точен: „…влизат в сумите по прогнозната стойност на процедурата, не по подадената — с изключение на „Раздуване след анекси"" (ред 409-410). Това затваря основната находка.

Едно остатъчно разминаване: §6 редът „Непотвърдени стойности" (ред 210) още казва „стойността им се изключва от сумите", което сега противоречи на §7 (влизат по прогнозна). Читател само на §6 остава подведен. Изравни §6 — напр. „суровата стойност се заменя с прогнозната, която влиза в сумите" вместо „се изключва".

(Ред 397 — чужда валута без намерен курс → NULL amount_eur — си е коректно „изключени".)

… section

Replaces statistical/technical jargon (HHI, percentile, DOJ/FTC citation,
raw field names like bids_received, NUTS3) with accessible descriptions
of the same underlying methodology, for a general public audience rather
than a technical one. No change in claimed behavior or accuracy — only
the vocabulary.
nikimilenkov added a commit to nikimilenkov/sigma that referenced this pull request Jul 16, 2026
… review)

The 14 subject-risk columns were added by editing 0000_init.sql. The served D1
is persistent and `wrangler d1 migrations apply` is filename-tracked, so an
edit to the already-applied 0000_init reaches the work DB (green local CI) but
never prod — precompute's UPDATE would then fail on missing columns (the
midt-bg#188/midt-bg#239 trap the reviewer reproduced).

Extract the columns into 0006_subject_risk_columns.sql (ALTER TABLE ADD
COLUMN) and remove them from 0000_init (SQLite has no ADD COLUMN IF NOT EXISTS,
so a fresh DB can't carry both). 0006 clears the 0002 claimants
(midt-bg#226/midt-bg#193/midt-bg#172) to avoid a duplicate-version at apply. The precompute
CREATE TABLE IF NOT EXISTS mirror stays a no-op on the existing tables. The
four risk tests now apply 0006 after 0000_init, mirroring the served-D1 chain.

Reword the 0000_init header to distinguish the rebuilt-every-import work DB
from the persistent served D1, so the next person adds objects via a new
migration instead of editing 0000_init. Also document that value_flag='ok' is
the exact complement of the hidden suspect set.
nikimilenkov added a commit to nikimilenkov/sigma that referenced this pull request Jul 17, 2026
… review)

The 14 subject-risk columns were added by editing 0000_init.sql. The served D1
is persistent and `wrangler d1 migrations apply` is filename-tracked, so an
edit to the already-applied 0000_init reaches the work DB (green local CI) but
never prod — precompute's UPDATE would then fail on missing columns (the
midt-bg#188/midt-bg#239 trap the reviewer reproduced).

Extract the columns into 0006_subject_risk_columns.sql (ALTER TABLE ADD
COLUMN) and remove them from 0000_init (SQLite has no ADD COLUMN IF NOT EXISTS,
so a fresh DB can't carry both). 0006 clears the 0002 claimants
(midt-bg#226/midt-bg#193/midt-bg#172) to avoid a duplicate-version at apply. The precompute
CREATE TABLE IF NOT EXISTS mirror stays a no-op on the existing tables. The
four risk tests now apply 0006 after 0000_init, mirroring the served-D1 chain.

Reword the 0000_init header to distinguish the rebuilt-every-import work DB
from the persistent served D1, so the next person adds objects via a new
migration instead of editing 0000_init. Also document that value_flag='ok' is
the exact complement of the hidden suspect set.
…boards

# Conflicts:
#	apps/web/app/lib/filters.test.ts
#	apps/web/app/styles/components.css
#	apps/web/workers/cache-key.test.ts
#	apps/web/workers/cache-key.ts
#	packages/config/src/index.test.ts
@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Забелязвам разминаване между рамката на PR-а и състоянието му. Тялото казва „Само документация — methodology.tsx е единственият променен файл" и „ще се извади от draft след merge на стека", но PR-ът не е draft (draft: false, base main, mergeable). Собствените ти комити наистина пипат само методологията, но клонът е стекнат върху #169#172, тъй че diff-ът срещу main носи целия стек + новото табло /overruns — 41 файла, ~9000 реда, вкл. миграция 0002_contracts_overrun_index.sql. Ако някой се довери на „docs-only" рамката и merge-не сега, влиза целият неслят стек под docs PR.

Две опции: (1) върни го в Draft докато стекът се слее (както описва секцията „Зависимости"), или (2) пренасочи base към стек-клона, не към main. И бележка за координация: 0002_*.sql е зает от четири независими PR-а (overrun_index #169#172/#193, current_value_currency #257, related_persons_foundation #226, contract_co_authorities #253) — първият слят печели 0002, останалите се преномерират.

@ydimitrof ydimitrof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Обобщен преглед на PR: „docs(web): методологията описва точно таблата — формули, обхват, изключения"

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

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

Въпреки заглавието docs(...), PR-ът е значителна функционална промяна, придружена от актуализация на методологията, така че документацията да описва точно таблата (формули, обхват, изключения). Основни части:

  • Нов маршрут /overruns („Раздуване") с loader, класация, scatter/облак, инспектор и таблици по сектори и институции.
  • Пренаписване на /trends („Договори — обзор") с три ъгъла на гледане (време / CPV / кръстосано), изцяло SSR/no-JS съвместимо.
  • Рефакторинг на /analytics — хиро-карти (AnalyzeCard) с нови „headline" заявки, водещи към конкретните изгледи.
  • Методология (methodology.tsx) — нов раздел 7 „Аналитичните изгледи" с формули, обхват и изключения; пренномериране на следващите раздели.
  • DB слой — нови/разширени заявки (overruns, regions, trend, flows, competition, headline-функции), миграция 0002 (частичен индекс за overrun), нови конфигурации (cpvDivision/cpvBucket), cache-key защити и обширни тестове.
  • Стилове — ~910 реда за таблото „trends" (components.css/layout.css), голяма промяна в pages.css (+3080/-3), нов токен в tokens.css; премахнати стилове analytics-lenses/lens-*.

Сигурност (Фаза 0 / OWASP) — ЧИСТО за целия PR

Няма твърдо кодирани тайни, нови зависимости или нови/променени външни URL адреси; няма eval, dangerouslySetInnerHTML, обфускация или задни вратички. Всички SQL заявки са статични или параметризирани чрез .bind(). Потребителският вход е строго валидиран (5-цифрени CPV кодове /^\d{5}$/, allowlist-ове, clampLimit, типизирани съюзи за ORDER BY), а CWE-349 (замърсяване на cache-ключа) е добре адресиран с осмислени тестове за ?cur=1 и мулти-селект ?cpv. React екранира всички изведени стойности. Няма установени рискове от A01/A03/A08.

Блокиращо / изисква потвърждение преди merge

  1. Потенциален неизползван import (cache-key.ts, партида 7) — добавен е INTENTIONALLY_UNKEYED, но не изглежда използван. При noUnusedLocals / eslint no-unused-vars това би счупило build/линт и нарушава правилото „NO DEAD CODE". Да се потвърди срещу пълния файл.
  2. authorityEik == authoritySlug (overruns.ts, партида 8) — за възложителя не се селектира отделна ЕИК колона (за bidder-ите има b.eik_normalized). Да се потвърди, че authoritySlug(authority_id) връща ЕИК цифрите за реда „Възложител · ЕИК", а не route slug — иначе е разминаване в показваните данни.
  3. Гранулярност на YoY (analytics.tsx, партида 2) — loader-ът вече иска getSpendingTrend(..., { granularity: 'month' }) (преди 'year') и подава месечни точки на estimateYoyGrowth, докато картата твърди „Същата стойност като на страницата „Тренд"". Да се потвърди, че стойностите не се разминават с /trends.

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

  • Обхват vs. заглавие: заглавието е docs(...), но PR-ът добавя реална функционалност (нови маршрути, компоненти, заявки, CSS). Отбелязано в почти всяка партида — уеднаквете заглавието с функционалната промяна или разделете промяната.
  • trends.tsx: деление на нула в relLabel (ред 105); възможни дубликати в missing (ред 90); подвеждащ надпис „(показани първите 24)" при точно 24 договора.
  • CSS: несъответстващ маркер на секция (trends-dashboardend overruns-dashboard); дублиран селектор .trend-chart-panel--full .trend-chart-body; повтарящ се литерал 'IBM Plex Mono' (кандидат за токен). Преди сливане потвърдете, че няма осиротели референции към премахнатите lens-* класове. pages.css (+3080) е без patch — не е проверен ред по ред за дублиран/мъртъв CSS.
  • overruns.tsx: непоследователна обработка на ЕИК (authorityEik || '—' срещу bidderEik ?? 'непотвърден'); scatter кръговете не са фокусируеми от клавиатура (приемливо — списъкът носи данните).
  • Миграция 0002: няма down-миграция (опишете DROP INDEX idx_contracts_overrun в плана за откат); частичният индекс по annex_count е perf-нюанс.
  • cpvBucket: бъдеща service-дивизия, добавена в CPV_SECTORS без CPV_BUCKET_SERVICES, тихо ще падне в „goods" (maintenance риск).
  • tokens.css: --paper-raised: #ffffff е суров hex, докато коментарът твърди, че палитрата е централизирана в OKLch.

Качество и тестове

Кодът е чист, добре структуриран и коментиран, с последователно именуване, без TODO/мъртъв код (извън точка 1) и без дублиране. Заявките са ограничени (без N+1, без непараметризиран вход), с внимателна обработка на липсващи данни и числова безопасност (защита от деление на нула, NaN/Infinity). Тестовете са смислени и нетривиални (гранични случаи, партидни периоди, cache drift-guard, инвариант „сума по области == сума по райони == общо"). Презентационните и docs партидите нямат тестове, което е приемливо; покритието на новата loader-логика в trends.tsx следва да се потвърди спрямо гейта ≥90%.

Препоръка: изяснете неизползвания import и трите въпроса за коректност; адресирайте дребните CSS/UX забележки по преценка — след което PR-ът е готов за одобрение.

Comment thread apps/web/app/components/MetricInfo.tsx
Comment thread apps/web/app/lib/analytics-stats.ts
Comment thread apps/web/app/components/ComboTrendChart.tsx Outdated
Comment thread apps/web/app/routes/analytics.tsx
Comment thread apps/web/app/routes/methodology.tsx
Comment thread apps/web/app/routes/trends.tsx
Comment thread apps/web/app/styles/components.css
Comment thread apps/web/app/styles/components.css Outdated
Comment thread apps/web/workers/cache-key.ts Outdated
Comment thread packages/db/src/queries/overruns.ts
…artial-index, css markers

- MetricInfo: use an isomorphic layout effect so SSR doesn't emit a useLayoutEffect warning
- ComboTrendChart: hasPartial now correctly treats a partial point at index 0, and the
  solid/dashed path segments are guarded so a leading partial doesn't compute a negative
  solidEnd or render a malformed dashed segment
- overruns.tsx: unify the authority/bidder EIK fallback to the || operator on both lines,
  since authorityEik is a non-nullable string and an empty value should still fall back
- trends.tsx: guard relLabel against a zero/NaN medianEur (would otherwise divide to
  Infinity/NaN), and dedup the missing CPV-group list before querying medians
- components.css: fix the trends-dashboard section's mismatched closing marker comment, and
  drop the duplicate .trend-chart-panel--full .trend-chart-body rule in favor of the one
  fullscreen-modal rule that already carries flex + min-height
- cache-key.ts: drop the unused INTENTIONALLY_UNKEYED import
…view)

authorityEik was populated with authoritySlug(), which is correct (auth: ids strip to the
bare EIK) but misleading — a field named for a URL slug feeding an EIK column. Add an
authorityEik alias of authoritySlug in identity.ts and route the overruns.ts assignment
through it. Emitted value is unchanged.

@ydimitrof ydimitrof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Обобщен преглед на PR — docs(web): методологията описва точно таблата — формули, обхват, изключения

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

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

Промяната пренаписва и разширява аналитичната част на apps/web заедно със съответните заявки и документация. Основните елементи:

  • Нови/преработени маршрути: нова страница /overruns („Раздуване"), пълен редизайн на /trends (от „Тренд във времето" към „Договори — обзор" с три ъгъла), пренаписана начална страница на „Анализи" (analytics.tsx) с пет hero-карти.
  • Нови компоненти и помощни функции: SSR-безопасни ComboTrendChart, FullscreenButton, MetricInfo; тествани помощници за форматиране/геометрия (analytics-stats.ts, overruns-chart.ts, overruns-inspector.ts).
  • Документация: нов раздел 7 „Аналитичните изгледи" в methodology.tsx с коректно преномериране на следващите раздели и валидни вътрешни котви; съдържанието съответства на праговете в UI.
  • Данни/заявки: нови заявки за overruns/competition/flows/regions/trend, нова идемпотентна DB миграция (0002, частичен индекс), нови типове в api-contract, CPV конфигурация (cpvDivision/cpvBucket), разширяване на канона за query параметри и кеш-ключове.
  • Стилове: голямо разширение на CSS (components.css, pages.css), премахване на старите .lens-* правила от layout.css.

Сигурност (Фаза 0 / OWASP) — ЧИСТО във всички партиди

  • Няма твърдо кодирани тайни, ключове или пароли (PEG = 1.95583 е официалният курс BGN→EUR).
  • Няма нови външни URL адреси, нови зависимости, backdoor-и, обфускация или инжекционни шаблони.
  • SQL: всички заявки са параметризирани (.bind(...)), с интерполация само на модулни константи и строго типизирани enum-и; потребителският вход е валидиран преди употреба (cpvGroupSelection с /^\d{5}$/, whitelist за ъгли/сортиране, строг регекс за година, clampLimit). Кеш-ключовете покриват отговор-афектиращите параметри (CWE-349). Еквивалентността SQL↔JS е тествана в реален SQLite.
  • XSS: целият изход минава като JSX текст/атрибути, които React екранира; няма суров HTML. CSS-ът е чисто декларативен (без url(), @import, expression).

Цялост на данните и коректност — добре обмислена

Крайните случаи са внимателно покрити: празни серии връщат честен празен изглед без NaN, клампване на растежа, защита срещу деление на нула, споделен OVERRUN_WHERE за единна дефиниция на „раздут", коректна медиана за четен/нечетен брой, изключване на текущия непълен период, YoY само срещу предходната година. Валута NULL→BGN е документирана; чужди валути без курс се пропускат честно вместо да фабрикуват данни.

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

  1. [Партида 8] Лимит на bound-параметрите на D1 в getOverrunAnnexes. IN (${placeholders}) може да достигне 200 плейсхолдъра (MAX_LIMIT = 200), а Cloudflare D1 има документиран таван от 100 параметъра — при leaderboardLimit > 100 заявката може да гръмне по време на изпълнение. По подразбиране (50) е безопасно. Да се капира/chunk-не до ≤100 или да се потвърди, че маршрутът никога не подава >100 id-та. Това е единствената реална техническа рискова точка.
  2. [Партида 6] Неверифицируем диф на pages.css. PR отчита +3080 реда, но патчът не е достъпен, а локалното копие е само 653 реда. Нужен е достъпен текстов диф за пълен ред-по-ред преглед и повторно сканиране на HEAD версията за външни url()/@import.

Съответствие заглавие/обхват — повтарящ се въпрос (партиди 1, 4, 7, 8)

Заглавието е docs(web): …, но PR-ът съдържа съществена продуктова логика (нови маршрути, компоненти, заявки, SQL миграция, типове, ~2000+ реда функционален код и CSS). Препоръчва се преетикетиране (напр. feat) или разделяне на документационната част, за да е диф-ът атомарен и коректно версиониран (Conventional Commits).

Тестово покритие — да се потвърди

Помощните функции, regions.ts, trend.ts, миграциите и CPV конфигурацията имат солидни, смислени тестове (включително враждебни входове). Липсват обаче тестове за няколко нови повърхности — компонентните тестове за overruns.tsx, чистите функции в trends.tsx (pick, multText, jitter, logMax и др.) и overruns.ts (561 реда). Ако не са в друга партида, покритието не отговаря на прага ≥90% за нов код.

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

  • [Партида 4] Off-by-one индикатор „(показани първите 24)" — появява се дори когато договорите са точно 24 без отрязване; заявете limit + 1 и показвайте при > 24.
  • [Партида 4] hrefWith пренася непознати query параметри в <Link> и кеш-ключа — ниска сериозност, но неограничава пространството на кеш-ключове.
  • [Партида 1] chooseXTicks реже до 5 стъпала — при широк логаритмичен диапазон дясната част на оста остава без етикети.
  • [Партиди 2, 3] Достъпност: точките в scatter диаграмите са кликаеми само с мишка (има еквивалент чрез бутоните в класацията); да се провери, че „stretched link" на AnalyzeCard не прехваща кликовете върху бутона ⓘ.
  • [Партида 5] CSS: потвърдете, че премахнатите .lens-* класове вече не се използват; .fs-btn/.trend-fs-btn са почти дублирани (общ базов клас); проверете WCAG контраста на ~9–10px caption текстове (--ink-soft върху --paper-warm).
  • [Партиди 2, 8] Консистентност: да се провери, че подсказката „Същата стойност като на страницата „Тренд"" не подвежда след премахването на прогнозата; null-проверки за headline агрегатите (overruns, flows, region); капиране на limit в getCpvGroupStats.

Заключение

Кодът е с високо качество, добре документиран, коректно параметризиран и чист откъм сигурност. Положителните акценти включват отличната валидация на cpvGroupSelection, затегнатите кеш-ключове и внимателно обмислената цялост на данните. Преди сливане: адресирайте тавана на D1 параметрите, осигурете видимост върху pages.css, потвърдете тестовото покритие и изяснете съответствието на префикса docs с реалния обхват.

Comment thread apps/web/app/lib/overruns-chart.ts
Comment thread apps/web/app/routes/analytics.tsx
Comment thread apps/web/app/routes/analytics.tsx
Comment thread apps/web/app/routes/overruns.tsx
Comment thread apps/web/app/routes/trends.tsx
Comment thread packages/db/migrations/0003_contracts_overrun_index.sql
Comment thread apps/web/app/styles/tokens.css Outdated
Comment thread packages/db/src/queries/overruns.ts Outdated

@lyubomir-bozhinov lyubomir-bozhinov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Прегледах и съдържанието на методологията срещу кода (не само рамката, за която коментирах по-рано). Формулите съвпадат ред по ред:

  • „Раздуване" = поне един анекс ∧ крайна > първоначална ∧ първоначална ≥ 1 000 €, а числото е „сборът от надбавките (крайна − първоначална)" — точно OVERRUN_WHERE + SUM(current−signing) в overruns.ts; annex_suspect коректно „не участва, защото няма надеждна текуща стойност" (NULL current_value_eur).
  • „Дял с една оферта" = bids_received = 1 върху поръчки с известен брой кандидати; концентрацията (HHI) иска ≥ 2 доставчика — както в competition.ts.
  • Стойностната база = изчистена стойност (текуща при законен анекс), прозорец 2020→днес без бъдещи дати — както в trend.ts.

Неутралната рамка („увеличенията по анекси често са напълно законни") е добра. Текстът е верен.

Две зависимости (не са дефект на текста): (1) описаните числа за „Раздуване" стъпват на current_value_eur, който е коректен едва след #257 (евро-анекси) — дотогава live-стойностите подбиват; (2) остава бележката ми за стекването — PR-ът все още носи целия стек #169#172 срещу main, редно е да влезе след него. Одобрявам съдържанието.

getOverrunAnnexes bound one id per SQL parameter with no cap, but the
leaderboard limit can reach 200 ids while Cloudflare D1 rejects queries
past 100 bound parameters ("too many SQL variables"). Chunk the id list
at the D1 cap and merge the per-chunk results instead.
The loader queried limit: 24 and showed "(показани първите 24)" whenever
contracts.length === 24, which fired even when exactly 24 contracts
existed untruncated. Query one extra row and only flag truncation when
it's actually present.
chooseXTicks took the lowest 5 stops from the in-range tick ladder, so a
wide log range (e.g. 25%–4800%) left the upper ~40% of the axis without
labels even though points were plotted there. Sample evenly across the
in-range ladder instead, always keeping the first and last stops.
The card's hint claimed avgYoy is "the same value as on /трендs", but
/trends never displays this figure (methodology.tsx documents /trends as
forecast-free) — the metric is analytics-only, computed from the same
underlying monthly series. Reword the hint to state that accurately.
The file's own header states the palette lives in exactly one place
(OKLch); --paper-raised: #ffffff was the one raw-hex exception.
…ly CSRF

postcss 8.5.15 (GHSA-r28c-9q8g-f849) and valibot 1.4.0 (GHSA-5qjj-4xww-7phc)
are patch-level, non-breaking bumps to 8.5.18+ and 1.4.2+ via pnpm-workspace
overrides.

react-router 7.18.0 is flagged by GHSA-qwww-vcr4-c8h2, a CSRF issue confined
to the unstable RSC APIs (verified via repo-wide grep: zero RSC usage). No
fix exists in the 7.x line; the minimum fixed version is 8.3.0, a major
breaking bump out of scope here. Suppressed via osv-scanner.toml, following
the same time-boxed pattern as the existing sharp (GHSA-f88m-g3jw-g9cj)
entry, reviewable by 2026-10-01.
…nflicts

- osv-scanner.toml: kept the react-router RSC suppression (this branch);
  dropped main's sharp suppression since this branch already bumped sharp
  to ^0.35.0 (5f94c94), which resolves GHSA-f88m-g3jw-g9cj outright
- packages/db/migrations: renumbered this branch's overrun-index migration
  from 0002 to 0003 to avoid colliding with main's already-merged
  0002_current_value_currency.sql; updated migrations.test.ts to apply both
  in sequence
- apps/web/app/routes/analytics.tsx, trends.tsx: kept this branch's
  redesigned loaders/queries (analytics hub, obzor three-lens view), applied
  main's getDb() read-only D1 chokepoint (midt-bg#199) instead of raw env.DB
- apps/web/app/routes/overruns.tsx: not itself conflicted, but also routed
  through getDb() to satisfy the chokepoint guard test added by main
  (readonly-db-chokepoint.test.ts)
- pnpm-lock.yaml: regenerated via pnpm install
@nedda76

nedda76 commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Този клон е в конфликт с main, тъй че към момента не може да се ревюира — дифът, който GitHub показва, вече не отговаря на това, което би влязло. Ще го пребазираш ли върху актуалния main (или merge на main в клона) и да разрешиш конфликтите? След това веднага го поглеждам. Благодаря! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants