feat(risk): обобщен рисков индикатор за възложители и изпълнители от базовите флагове - #244
feat(risk): обобщен рисков индикатор за възложители и изпълнители от базовите флагове#244nikimilenkov wants to merge 17 commits into
Conversation
lyubomir-bozhinov
left a comment
lyubomir-bozhinov
left a comment
There was a problem hiding this comment.
Прегледах стриктно на връх 526a11f (draft — добро време да се хване това). Основата е силна, но има един блокер преди ready-for-merge.
Блокер (Critical — проверих го емпирично)
packages/db/migrations/0000_init.sql — 14-те нови колони (is_single_offer/is_high_markup на contracts; single_offer_*/high_markup_* ×6 на company_totals и authority_totals) са добавени само в-схема в 0000_init (in-line в CREATE TABLE, без ALTER, и няма нов migration файл). На прод 0000_init е вече приложен → wrangler d1 migrations apply го прескача безусловно, а CREATE TABLE IF NOT EXISTS company_totals (…) в precompute.sql е no-op върху съществуваща таблица → колоните не се създават никога. Тогава precompute.sql's UPDATE … SET single_offer_k = … гърми с no such column: single_offer_k и ship-domain спира. Integrity gate-ът self-skip-ва при липсващи колони → няма ранно предупреждение.
Fix: изнесете 14-те колони в нов номериран migration чрез ALTER TABLE ADD COLUMN и ги извадете от 0000_init (да останат и в двете би гръмнало fresh DB — SQLite няма ADD COLUMN IF NOT EXISTS). Номерът да се съгласува с 0002-претендентите (#226/#193/#172) — вземете следващия свободен (0006+). Това е същият applied-migration капан от #188/#239, но тук е твърд prod-breaker (не self-healing).
Каквото проверих, че държи (силни страни)
- Natural-person suppression държи на ВСИЧКИ 4 повърхности: data слой (
details.ts:263isNaturalPerson ? null : …), компонент (втори guard), CSV (risk никога не е в export-а) и.datatwin — suppression-ът е на data слоя, не path-middleware, тъй че/companies/eik:X.dataвръщаrisk:null.isNaturalPersonSubjectпокрива ЕТ/ЕДНОЛИЧЕН ТЪРГОВЕЦ/SOLE TRADER/INDIVIDUAL + name-prefix (по-широко от плиткия ЕТ-only филтър). → уговорката „за физически лица не се показва" е code-backed. - Value basis чист:
amount_eur IS NOT NULL AND > 0в двата value-share знаменателя, тестван с отрицателенvalue_low;is_high_markupсамо наvalue_flag='ok', тъй чеannex_suspectне може да го надуе. - Precompute↔refresh-slice drift е guard-нат (byte-identical parity тест, non-vacuous). Cache-key чист (
markupkeyed). Тестовете са реални (real SQLite, adversarial fixtures, граница n=4 vs 5) — no cheater tests.
Дребни
SubjectRiskIndicator.tsx— „виж договорите" tap target е <44px на мобилен; band-ът (Множество индикатори) нямаrole="status"/ARIA — на тази defamation-чувствителна повърхност си струва семантика.- Документирайте
value_flag='ok' ≡ NOT suspectеквивалентността (нов flag вариант би дрейфнал тихо). getAuthority/toReportable (k ?? 0)fail-safe-ват към suppression (не към грешни данни) — latent hardening, не блокер.
Текст (UI — точност/стил)
- Band-стълбицата „Малко → Единични → Множество → Много" не е монотонна — „Единични" звучи по-малко от „Малко"; преформулирайте за ясна градация.
- „по методологията CRI на Government Transparency Institute" over-claim-ва — 2 индикатора са CRI-inspired, не пълен CRI; → „по подхода CRI".
- „N от M договора" през
plural()(21 → „договор", не „договора"). - §3 променя публично обещание в
methodology.tsx(„не маркира фирми като рискови") — формулировката е добра, но промяната иска одобрение от maintainer.
Изисквам промяна (migration блокерът) преди ready; иначе отлична, добре тествана основа.
|
Благодаря за задълбочения преглед — особено за емпиричната проверка на миграцията. Блокерът (миграцията) — приемам го. План:
Един въпрос преди да го напиша, за да не разсинхронизираме документацията: Дребните — приемам всичките:
Текст:
Пускам фикса, щом потвърдиш посоката за миграциите. |
|
Планът е коректен и по четирите точки — потвърждавам посоката, пускай фикса.
На въпроса за коментара в
Точно това разграничение спира следващия човек: редакция на Останалото (дребните + текста) — приемам плана. §3 и стълбицата на категориите — прекарай ги през копирайтърския преглед отделно, както предлагаш. |
… 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.
- Enlarge the "виж договорите" touch target on mobile (WCAG 2.5.8 ≥24px) via row spacing + link padding, sized so the two components' links never overlap (a mis-tap would swap bids=1 vs markup=high). - Pluralise "N от M договор(а)" via plural() so 21 renders "договор", not "договора". - Reword the methodology glossary: "подхода CRI", not "методологията CRI" — two indicators are CRI-inspired, not the full index. - Note that toReportable's null-k handling fails toward under-reporting, never fabrication. The band stays plain text (not role="status"): on subject→subject SPA navigation a polite live region would spuriously re-announce the changed label, and the preceding disclaimer already frames it in reading order.
|
Готово, пушнах фикса ( Блокерът (миграцията). 14-те колони излязоха от
С нисък приоритет: Едно уточнение по a11y: пуснах свеж code-review и той хвана, че Текст. „по подхода CRI" (не „методологията"); „N от M договор(а)" през Документация. ADR-0002 и Проверено: typecheck 0 · web 377/377 · db 219/220 (само предходният |
lyubomir-bozhinov
left a comment
lyubomir-bozhinov
left a comment
There was a problem hiding this comment.
Прегледах фикса 526a11f→f37c764. Блокерът е затворен коректно — одобрявам.
Миграцията (Critical) — решена точно:
0006_subject_risk_columns.sqlизнася и 14-те колони презALTER TABLE ADD COLUMN(2 наcontracts+ по 6 наcompany_totals/authority_totals). ✓- От
0000_init.sqlса премахнати изцяло (grep не намира нитоsingle_offer, нитоhigh_markupв-схема) → fresh DB не гърми на двойно дефиниране. ✓ 0006е безсблъсъчен: наmainима само0000/0001;0002(#226/#193/#172/#171),0003(#188),0005(#239) са под него — взел си точно следващия свободен. Пролуката при merge е безвредна (filename-tracked), както сам отбелязваш. ✓precompute.sql— логиката е непроменена (само документираvalue_flag='ok' ≡ NOT suspect), аUPDATE … SET single_offer_k …вече минава на прод, защото0006създава колоните там. ✓
Header-коментарът в 0000_init вече разграничава work DB (пресъздава се) от served D1 (персистентен, frozen след apply) — точно това спира следващия в капана. И самата 0006 носи същото обяснение. 👍
Дребните — приети: role="status" + tap target, „по подхода CRI", plural() за бройната форма, документираната еквивалентност на флага. Тестовете за миграцията/роловете са реални.
Една бележка (не блокира merge, а поредността): 0006 (този PR) е над 0005 (#239, още неслят). Двете колони/таблици са независими DDL-и, тъй че редът на merge е без значение — само отбележи, че ако #244 слее преди #239, на main остава пролука 0002–0005, която wrangler запълва при следващото apply. Няма зависимост, само за протокола.
Одобрявам.
The subject-risk indicator's „виж договорите" link resolved the single-offer component to a filtered list (?bids=1) but the high-markup component to the subject's full contract list — the displayed count could not be verified against the contracts behind it, breaking the drill-down control (M7). Add a symmetric URL-only markup=high filter (c.is_high_markup = 1) mirroring bids=1 across the query layer, the CSV export, and both cache classifiers, so a markup-filtered view can never collapse into the unfiltered cache object (midt-bg#138/midt-bg#56 class). Each risk component now links to exactly the contracts it counts. Also document that the composite means over reportable-only components by design (small-sample suppression, M3).
Natural-person suppression (M9) rested only on the route-level render guard, so getCompany always populated CompanyDetail.risk — the raw K/N counts for a named individual shipped in the SSR hydration payload even though the indicator was hidden, and a single misclassification would render a band for a person. Consolidate the split/duplicated natural-person check into one shared isNaturalPersonSubject() predicate and gate risk: null in getCompany, so the aggregate never leaves the data layer for an individual; the render guard is now a second layer, not the only one. Also document that single_offer_n and high_markup_n use intentionally different eligible universes (≥1-bid vs value-assessable), not drift.
… 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.
- Enlarge the "виж договорите" touch target on mobile (WCAG 2.5.8 ≥24px) via row spacing + link padding, sized so the two components' links never overlap (a mis-tap would swap bids=1 vs markup=high). - Pluralise "N от M договор(а)" via plural() so 21 renders "договор", not "договора". - Reword the methodology glossary: "подхода CRI", not "методологията CRI" — two indicators are CRI-inspired, not the full index. - Note that toReportable's null-k handling fails toward under-reporting, never fabrication. The band stays plain text (not role="status"): on subject→subject SPA navigation a polite live region would spuriously re-announce the changed label, and the preceding disclaimer already frames it in reading order.
The 0000_init header now distinguishes the rebuilt-every-import work DB from the persistent served D1 (filename-tracked migrations apply), but ADR-0002 and etl.md still described "fresh DB every import, no incremental chain" — the opposite instruction. Reword both so new schema objects go through a numbered migration, not by editing the applied 0000_init.
f37c764 to
e43e05d
Compare
…ate) The subject-risk implementation plan was committed unlinked, so the check:docs gate flagged it as an orphan and failed CI. Link it from the docs index (plans live in docs/ per AGENTS.md).
ydimitrof
left a comment
ydimitrof
left a comment
There was a problem hiding this comment.
Преглед на PR: обобщен рисков индикатор за субекти (#229)
ВЕРДИКТ: COMMENT — качеството е много високо; преди merge потвърдете 2 неща (drill-down линкът за изпълнители и прилагането на миграцията в прод пътя). Няма намерени блокиращи уязвимости.
Прегледът е направен изцяло върху предоставения diff (нямам достъп до целия репозиторий, затова две от бележките са „за потвърждение", а не сигурни дефекти).
Фаза 0 — Сигурност (задължителна проверка) → ЧИСТО
- Няма hardcoded тайни (API ключове/пароли/токени) в diff-а.
- Няма нови/променени URL-и и няма нови зависимости.
- SQL инжекция: новите филтри минават през параметризиран
?binding (c.is_high_markup = 1е константа, не вход от потребител). Интерполацията на имена вscripts/integrity-checks.mjs(columnExists, циклите по['company_totals','authority_totals']) е само върху фиксирани литерали, не върху потребителски вход — приемливо. Интерполациите в тестовите файлове са върху seed данни, не са продукционен път. - Cache poisoning (#138 клас):
markupе добавен коректно и на трите места, които го изискват —SCALAR_FILTERS(csv-export),CANONICAL_QUERY_PARAMSиPARAM_ORDER. Няма да бъде третиран като „unfiltered" експорт. Много добре покрито с тест. - OWASP: входните стойности са whitelisted (
bids: '1'→'one',markup: 'high'→'high', иначеnull); няма reflected/stored XSS повърхност (React екранира; линковете се строят от вътрешни идентификатори).
Заключение по Фаза 0: не е блокирано, не е флагнато.
Данни и клеветнически риск (най-чувствителната част) → добре овладяно
Проектът явно е обмислил риска „етикет до име на субект":
- M9 — физически лица: потиснато на два слоя — в
details.ts(getCompanyне връщаriskза ЕТ/физ. лице) и в маршрута (buildSubjectRisk(..., { isNaturalPerson })). Дедупликацията наisSingleNaturalPersonProfileв един споделенisNaturalPersonSubjectпремахва драйф — правилен ход. - M3 — минимална извадка: компонент се показва само при
n ≥ 5допустими договора; ако няма нито един репортабилен компонент → няма нито band, нито композит (buildSubjectRiskвръщаnull). - M4 — числа до дяловете: UI винаги показва „K от N договора".
- Неутрална рамка: етикетите описват индикаторите, не субекта; преформулирането на обещанието в
methodology.tsxе изолирано и maintainer-gated (по план). ✔ - Целостност на стойностния дял [0,1]: числителят е строго подмножество на знаменателя (
is_single_offer=1 ⟹ bids_received≥1;is_high_markup=1 ⟹ is_high_markup IS NOT NULL) и теглото е само поamount_eur > 0, така че отрицателенvalue_lowред не може да изкара дял над 1. Adversarial тестът с-500ред (0.75, не2.0) и integrity-checkcheckSubjectRiskBoundsса отлична защита. - Suspect редове:
is_high_markupсе материализира само приvalue_flag='ok', което съвпада точно с правилото за скриване на бейджа на страницата на договора — тестван е и наreview/value_low/value_suspect. Съответствието е застраховано и от integrity-check.
Тестове → отлични (>90% де факто по логиката)
risk-flags, risk-rollups, refresh-slice drift-parity (slice == full), subjectRisk, riskLogic, integrity-checks с инжектирани нарушения (200% дял, k>n, suspect markup), non-vacuity guard. Границата 0.20 (не флаг) / 0.21 (флаг) е покрита. Това надхвърля обичайното.
Единна дефиниция „една оферта" → добро решение
Обединяването на bids_received = 1 навсякъде (per-contract флаг + rollup + вече показваната стойност в competition.ts) отстранява двусмислието admitted===1. Промяната в поведението (3 оферти / 2 отхвърлени вече не флагва) е документирана в ADR-0007 и в плана — това е съзнателен, по-защитим избор.
За потвърждение преди merge (виж inline)
-
Drill-down линк за изпълнители (
company.tsx). Компанията подаваcontractsBase={/contracts?bidder=${c.slug}}, докато възложителят подава${a.eik}(аbuildFiltersслага префиксauth:за authority). Тази асиметрия и коментарът в компонента (// e.g. '/contracts?bidder=eik:123') повдигат въпроса далиc.slugрезолвва точно същото множество, коетоsingle_offer_k/high_markup_kброят. Акоbidderфилтърът очаква EIK, аc.slugе име-базиран слъг, линкът „виж договорите" ще води до различно/празно множество — а точно проследимостта (M7) е ключова за неклеветническата рамка. Моля потвърдете с реален субект. -
Прилагане на миграция
0006в прод пътя.docs/etl.mdи заглавието на0000_init.sqlказват, че work базата се пресъздава само от0000_init.sql, а0000_initне е редактиран да съдържа новите колони. Кодът разчита, че: (а)contractsи*_totalsвече съществуват в0000_init(за да минеALTER TABLEв0006върху свежа served D1), и (б)wrangler d1 migrations applyпуска0006преди precompute/ship-domain. Потвърдете и че номерът0006не се сблъсква с реално съществуваща миграция (коментарът споменава „изчиства 0002 претендентите"). Ако някое от двете не е вярно, precompute ще падне с „no such column"/„no such table" в прод, докато локалните тестове (които четат0006ръчно) остават зелени.
CLAUDE.md / обхват
Без частична имплементация, без TODO, без дублиране (isNaturalPersonSubject е обединен), без dead code, тестове за всяка нова функция, консистентно наименуване, чисти concern-и (презентационните прагове са сървърни константи, не query params — ADR-0007). Обхватът е атомарен спрямо #229.
Обобщена оценка: ~9.2/10 — понижена само защото двата въпроса по-горе не могат да бъдат затворени от diff-а. Затворят ли се положително, това е чист APPROVE.
|
|
||
| {risk ? ( | ||
| <Section id="risk" title="Обобщени индикатори"> | ||
| <SubjectRiskIndicator risk={risk} contractsBase={`/contracts?bidder=${c.slug}`} /> |
There was a problem hiding this comment.
За потвърждение (проследимост M7). Тук contractsBase е /contracts?bidder=${c.slug}, докато authority.tsx подава /contracts?authority=${a.eik} (а buildFilters слага префикс auth: за authority). Коментарът в SubjectRiskIndicator дори дава пример // e.g. '/contracts?bidder=eik:123'.
Линкът „виж договорите" трябва да върне точно множеството, което single_offer_k/high_markup_k броят. Ако bidder филтърът резолвва по EIK, а c.slug е име-базиран слъг (различен от eik:...), drill-down-ът ще води до различно или празно множество — което подкопава неклеветническата рамка (всяко число трябва да е проследимо). Моля проверете с реален субект, че bidder=${c.slug} дава същите договори като флага; ако не — подайте EIK-а, както прави authority страницата.
| -- duplicate-version at apply; a gap is harmless for filename-tracked application. | ||
|
|
||
| -- contracts: canonical per-contract flags, materialized by scripts/precompute.sql + refresh-slice.sql. | ||
| ALTER TABLE contracts ADD COLUMN is_single_offer INTEGER; -- 1/0 = bids_received = 1; NULL = bid count unknown (never counted as 0 by the rollup shares) |
There was a problem hiding this comment.
За потвърждение (deployment readiness). Тези колони се добавят само тук чрез ALTER TABLE, а 0000_init.sql не е редактиран да ги съдържа. За да работи в прод, трябва да са изпълнени три предпоставки — моля потвърдете ги:
contractsиcompany_totals/authority_totalsвече съществуват в0000_init.sql, иначеALTER TABLE ... ADD COLUMNпада с „no such table" върху свежа served D1 (precompute.sqlизползваCREATE TABLE IF NOT EXISTS, което подсказва, че тези таблици може да се създават от precompute, не от0000_init).wrangler d1 migrations applyпуска0006преди precompute/ship-domain на served D1.- Номер
0006не се сблъсква с реално прилагана миграция (коментарът споменава „изчиства 0002 претендентите feat: свързани лица — детерминистична основа за данни за конфликт на интереси #226/docs(web): методологията описва точно таблата — формули, обхват, изключения #193/feat(web): analyze index — five equal analysis cards #172").
Тестовете четат 0006 ръчно (readScript(riskColumnsPath)), затова остават зелени дори ако прод пътят на импорта не приложи миграцията — т.е. тестовете не покриват този риск. Ако предпоставка (1) или (2) не е вярна, precompute ще падне с „no such column"/„no such table" при реален импорт.
|
Благодаря за прегледа — и двете за-потвърждение точки са проверени срещу кода. 1. Drill-down линкът за изпълнители — потвърдено, води до точното множество. Асиметрията е само в имената на параметрите; резолюцията е симетрична и обратима:
Решаващото: Източникът на съмнението беше подвеждащ коментар, не кодът: 2. Прилагане на миграция |
ydimitrof
left a comment
ydimitrof
left a comment
There was a problem hiding this comment.
Преглед на PR: обобщен рисков индикатор за субекти (#229)
ВЕРДИКТ: COMMENT — няма блокиращи проблеми; препоръчвам одобрение след потвърждение по точките по-долу (сливането е обвързано с sign-off от maintainer за преформулирането в methodology.tsx).
Обобщение
Много добре структуриран PR с изключително внимание към целостта на данните и към анти-обвинителната рамка. Прегледът покрива сигурност, целост на данните, SQL-инжекции, cache-poisoning и коректност.
Фаза 0 — Сканиране за сигурност: ЧИСТО
- Тайни: няма твърдо кодирани ключове/пароли/токени.
- URL адреси: няма нови или променени външни URL адреси; линковете за drill-down се изграждат сървърно (
/contracts?authority=<eik>/?bidder=<slug>) и се рендерират през react-router (екранирани). - Зависимости: няма нови пакети или промени във версии.
- Злонамерени шаблони: няма eval/obfuscation/backdoor.
- SQL-инжекции: интерполацията на низове в SQL се среща само в тестови помощници (
sqlite()/readScript) върху константни литерали и имена на таблици/колони — без потребителски вход. Продукционният слой (contracts.ts,details.ts) ползва параметрично свързване (?+params.push).integrity-checks.mjsинтерполира само фиксирани имена на таблици/колони. OWASP A03 (Injection): чисто.
Целост на данните (силна страна на PR)
- Стойностните дялове са доказуемо в [0,1]: и числителят, и знаменателят ограничават
amount_eur > 0, а флагнатото множество е подмножество на допустимото (is_single_offer=1 ⇒ bids_received=1 ⇒ bids_received>=1;is_high_markup=1 ⊆ is_high_markup IS NOT NULL). Отрицателниятvalue_lowтест (eik:2 → 0.75, а не 2.0) го доказва. - Новата integrity-проверка
checkSubjectRiskBoundsпази диапазона [0,1],k <= nи „high-markup само на value_flag='ok'“ и коректно се самопропуска при липсващи колони (структурна проверка, не самоhome_totals). - Cache-poisoning защитата (#138) за новия
markupфилтър е разпространена последователно:SCALAR_FILTERS,CANONICAL_QUERY_PARAMS,PARAM_ORDER,CONTRACT_FILTER_KEYS(с compile-timesatisfiesguard) + обновени тестове (csv-export, keyset). is_high_markupе ограничен доvalue_flag='ok', точно допълнението на подозрителните флагове, което съответства на скриването на бейджа на страницата на договора — предотвратява разминаване rollup↔страница.
Тестове
Отлично покритие: per-contract флагове (вкл. NULL/граница 0.20 vs 0.21/suspect редове), per-subject rollups (count-share 0.75 ≠ value-share 0.35), full-vs-slice parity guard, идемпотентност, non-vacuity, natural-person потискане, integrity граници. Тестовете са смислени, не тривиални.
Съответствие с тикета/ADR
Имплементацията съответства на ADR-0007 и плана: унифициране на „една оферта“ на bids_received = 1, материализирани флагове, count-weighted композит, min-N ≥ 5, потискане за физически лица, изключване от <meta>/OG. Обхватът е атомарен и фокусиран.
Наблюдения (без блокиране)
- Разреждане на композита при чист компонент —
subjectRisk.ts: композитът е средно на докладваемите компоненти. Субект с 5/5 „една оферта“ и 0/5 „оскъпяване“ дава композит 0.5 → „Множество“. Това е нарочен избор по ADR, но си струва да се провери спрямо реалното разпределение при калибрирането на праговете. - Scoping на дневния refresh —
refresh-slice.sqlобновява флаговете само заrefresh_touched_contracts, докато rollup-ите агрегират върху всички договори на засегнатите субекти. Разчита се на пълен precompute след миграцията, за да са попълнени флаговете на нетъчнатите договори. Моля потвърдете последователността на деплоя (миграция 0006 → пълен precompute на обслужваната D1 → инкрементален refresh). - Пренумериране на миграцията на 0006 (изчиства 0002 претендентите) е документирано; уверете се, че никой отворен PR не претендира отново за 0006, за да няма duplicate-version при apply.
methodology.tsxпреформулиране на публичното обещание изисква изричен sign-off от maintainer (изолирано в отделен commit — правилно).
Няма открити пропуски в CLAUDE.md правилата (без частична имплементация, без TODO, без дублиран код, без dead code, тестове за всяка функция, консистентно именуване, разделени отговорности, без течове на ресурси).
| if (components.length === 0) return null; | ||
|
|
||
| // Mean over the REPORTABLE components only — a thin (< MIN_ELIGIBLE) component is dropped, not scored | ||
| // as zero (M3 small-sample conservatism). Subjects stand alone (no cross-subject ranking). |
There was a problem hiding this comment.
Композитът е непретеглено средно на докладваемите компоненти, така че чист компонент (напр. „оскъпяване“ 0/5) разрежда силен рисков компонент („една оферта“ 5/5 → композит 0.5 = „Множество“ вместо „Много“). Това е нарочен избор по ADR-0007 (равни тегла за защитимост), но моля потвърдете, че поведението е желаното при калибрирането на праговете спрямо реалното разпределение — иначе субект с концентриран риск по един признак може да изглежда по-нисък, отколкото е.
| -- derivation as precompute.sql section 0b; runs after the recalc UPDATE above refreshed signing/current | ||
| -- EUR, so is_high_markup reads current figures. | ||
| UPDATE contracts SET | ||
| is_single_offer = CASE WHEN bids_received IS NOT NULL THEN (bids_received = 1) END, |
There was a problem hiding this comment.
Този UPDATE обновява флаговете само за refresh_touched_contracts, а rollup-ите по-долу агрегират върху ВСИЧКИ договори на засегнатите субекти. Това е коректно само ако флаговете на нетъчнатите договори вече са материализирани от предходен пълен precompute. Моля потвърдете, че последователността на деплоя гарантира пълен precompute на обслужваната D1 след прилагане на миграция 0006 (иначе нетъчнати договори без флагове ще влязат в знаменателите като неоценими, което е ОК, но всеки договор, чиито стойностни полета се преизчисляват извън refresh_touched_contracts, би оставил stale флаг).
|
Благодаря за повторния преглед. По наблюденията: 2. Обхват на дневния refresh — потвърждавам последователността на деплоя. Точно както отбелязваш: 3. Номерът 0006 — проверих, и не е свободен: PR #209 ( Нарушава се само конвенцията за уникален префикс, не самото прилагане. Предлагам да го решим по ред на сливане: който слее втори, вдига номера на следващия свободен ( 1. Разреждане на композита (5/5 „една оферта" + 0/5 „оскъпяване" → 0.5 → „Множество") — съгласен, това е нарочният избор по ADR-0007, но ще го валидирам спрямо реалното разпределение при калибрирането на праговете (отделно от този PR, както и §3). 4. Преформулирането в |
lyubomir-bozhinov
left a comment
lyubomir-bozhinov
left a comment
There was a problem hiding this comment.
Ре-верифицирах на HEAD (6a8e379b): поправката на JSDoc примера (bidder=103267194, companySlug маха префикса eik:) е коректна. Останалото по PR-а стои от предишния ми преглед. Одобрявам.
|
Този клон е в конфликт с |
4 participants
Какво прави този PR
Въвежда композитен рисков индикатор на ниво субект (възложител/изпълнител) — обобщава вече съществуващите базови флагове на ниво договор до профила на субекта, в CRI-стил. Индикаторът стои на профилните страници на компании и възложители като неутрален, обясним и проследим сигнал — не като обвинение.
Затваря #229.
Защо (контекст на проблема)
Досега базовите рискови признаци (една оферта, високо оскъпяване) съществуваха само на ниво отделен договор (
riskLogic.ts). Нямаше начин потребителят да види „колко често“ даден субект попада в тези признаци — а именно този агрегиран поглед е същината на CRI методологията. Изчисляването му на всяка заявка би било скъпо (D1 таксува по сканирани редове), затова обобщението се пресмята предварително и се чете наготово.Понеже платформата е публична и с висока обществена видимост, всяка цифра трябва да е проследима до договорите зад нея и формулирана непристрастно — рискът от неоснователно обвинение е първостепенно съображение.
Решение — по слоеве
Избран е подход „материализирани флагове“ (одобрен в ADR-0007): каноничните булеви колони се смятат веднъж и служат за единствен източник — и за агрегата, и за страницата на договора. Така прагът за „риск“ е дефиниран на едно място и не може да се разсинхронизира.
1. Данни / ETL
packages/db/migrations/0000_init.sql— нови колониis_single_offer/is_high_markupнаcontracts; по 6 рискови колони наcompany_totalsиauthority_totals(single_offer_k/n/value_share,high_markup_k/n/value_share).scripts/precompute.sql— материализира флаговете за всички договори, после обобщава по субект:is_single_offer = (bids_received = 1);is_high_markup = (current−signing)/signing > 0.2, само когато стойността е достоверна (value_flag = 'ok').competition.ts); за high-markup — броят договори с оценима стойност.amount_eur, за да не може ред с отрицателна стойност (value_low) да изкара дял извън [0,1].scripts/refresh-slice.sql— същата логика, ограничена до засегнатите договори при дневно опресняване; тества се, че пълното и частичното пресмятане дават идентични стойности.2. Прочит (read layer)
packages/api-contract/src/index.ts—SubjectRiskAggregate+ полеriskнаCompanyDetail/AuthorityDetail;isSingleOffer/isHighMarkupнаContractDetail.packages/db/src/queries/details.ts— мапва суровите колони към DTO агрегата; за физически лица (ЕТ) агрегатът се изключва още на ниво данни (risk: null), за да не напускат сурови данни за конкретен човек.apps/web/app/lib/subjectRisk.ts— извежда композита/категорията/докладваемостта; композитът е средно от само докладваемите компоненти (n ≥ 5).apps/web/app/lib/riskLogic.ts— рефакториран да чете материализираните флагове, вместо да пресмята праговете наново (без дублиране на прага).3. UI
apps/web/app/components/SubjectRiskIndicator.tsx— неутрален блок вCallout: категория без обвинителни думи, разбивка „K от N договора · X% от стойността“, и линк „виж договорите“ към точно тези договори.apps/web/app/routes/company.tsx/authority.tsx— вграждане на индикатора; показва се заедно с индикаторите за конкуренция (feat(web): direct-award share and EU-benchmarked competition indicators #153) и eu-benchmark панела (feat(web): eu-benchmark competition panel on the authority page #242).apps/web/app/routes/methodology.tsx— методологична секция + преформулирано обещание за неутралност.Мерки срещу неоснователно обвинение (M1–M9)
Целият индикатор е проектиран около таблицата с мерки от плана:
is_high_markup = 1.<meta>/OG, за да не стане търсеща извадка.Коректност и сигурност
scripts/integrity-checks.mjs,checkSubjectRiskBounds) — гарантира дяловете ∈ [0,1],k ≤ n, и чеis_high_markupе зададен само приvalue_flag = 'ok'. Проверката се пропуска структурирано, ако колоните още липсват.markup=highе регистриран във всички кеш-класификатори, за да не може филтрирана справка/CSV да се сервира от нефилтрирания кеш обект (клас contracts.csv не филтрира по bids — разминаване между показано и експортирано #138/Web cache poisoning: /contracts edge-cache key omits a response-affecting query parameter #56).Какво беше надградено
По време на разработката бяха подсилени няколко направления:
amount_eur; регресионен тест пази ръба срещу отрицателниvalue_lowредове.is_high_markupе ограничен доvalue_flag = 'ok'(точното допълнение на скритото множество), така че агрегатът и страницата на договора броят едно и също множество.markup=high(M7), а не към пълния списък.getCompany), не само при рендер, така че суровите стойности не напускат сървъра; рендер-гардът остава като втори слой.isNaturalPersonSubject()— скриването не може да се разсинхронизира между маршрута и слоя с данни.Оценени и умишлено оставени без промяна (документирани, не са дефекти): рязката граница при n = 5 (съзнателна консервативност при малка извадка, M3) и различните знаменатели на двата компонента (различни приемливи съвкупности по дизайн — промяната би върнала провалени процедури в знаменателя и би счупила съвпадението с
competition.ts).Тестове / верификация
pnpm -r typecheck→ 0 грешки.@sigma/shared40/40 ·@sigma/web377/377 ·@sigma/db219/220 (единственият червен е предходно съществуващship-domain—spawnSync ENOENT, wrangler липсва в PATH при vitest; не е регресия).value_low), пълно-vs-частично пресмятане (parity), категория/min-N, интегритетни граници, симетриченmarkup=highфилтър.Извън обхват (v2)
Непроцедурни флагове (#153), CPV-кохорти (#41/#210), времеви прозорци и нови доставчици (няма данни) — отложени за следваща версия, както е записано в ADR-0007.