Skip to content

feat(web): прогноза в тренда (проекция по сезонен профил) - #192

Closed
StanislavBG wants to merge 25 commits into
midt-bg:mainfrom
StanislavBG:feat/trend-forecast
Closed

feat(web): прогноза в тренда (проекция по сезонен профил)#192
StanislavBG wants to merge 25 commits into
midt-bg:mainfrom
StanislavBG:feat/trend-forecast

Conversation

@StanislavBG

Copy link
Copy Markdown
Contributor

Държи се извън опашката за ревю по продуктово решение — графиката в основния стек (#169#172) показва само реални данни. Този draft пренася прогнозата като отделна, включваема функция; готов за un-draft след като стекът се слее.

Зависимости

Базиран върху върха на стека: #169#170#171#172 (член 5 на стека). Да не се ревюира преди тях.

Метод (накратко)

  • Сезонен профил: всеки бъдещ месец = същият календарен месец отпреди една година × YoY фактор на растеж — сезонната форма (декемврийски пикове, летни спадове) се пренася напред, не плоска линия.
  • Растеж, ограничен в [0.5, 2]: факторът е каноничната оценка от /analytics (estimateYoyGrowth — медиана на съотношенията на последните 3 пълни години, clamp срещу еднократни аномалии в корпуса). Нищо не е фабрикувано — всичко се извежда от реалните месечни серии.
  • Прогнозата започва след последния ПЪЛЕН месец, стига до края на следващата година (макс. 18 месеца) и се потиска изцяло, ако няма сезонна база отпреди година (никакъв „срив към нула").
  • UI: щрихована accent линия под регион „ПРОГНОЗА", бледи проектирани барове, badge в tooltip-а; никога в стила на реалните данни. При „вкл. текущия месец" проекцията се подрязва до строго по-късни месеци.

Обхват

  • apps/web/app/lib/trends-forecast.ts (+ тестове) — buildForecast, преизползва estimateYoyGrowth от analytics-stats
  • ComboTrendChart — прогнозен слой (регион, линия, барове, tooltip badge)
  • /trends — прогноза само на месечната стъпка на лещата „Във времето" + легенда

@ydimitrof

Copy link
Copy Markdown
Contributor

Ревю на PR #192 — „прогноза в тренда (проекция по сезонен профил)"

Благодаря за прегледната документация и за това, че прогнозата е внесена като отделна, включваема функция върху върха на стека. Прегледах кода изключително внимателно — с фокус върху сигурност, инжекции и цялост на данните — както ръчно, така и с два независими одиторски прохода (DB слой и уеб-маршрути). Работих локално върху feat/trend-forecast спрямо базата 5d99fd9.

Обхват и съответствие с описанието

Реалната нова работа (buildForecast, „вкл. текущия месец" toggle, слоят в ComboTrendChart, /trends интеграцията) отговаря точно на описания метод: сезонен профил × YoY фактор, преизползване на каноничния estimateYoyGrowth (3-годишен пълзящ медиан, clamp [0.5, 2]), само на месечна стъпка, потискане при липса на сезонна база (без „срив към нула"). Прогнозата се извежда server-side от trend.points — нищо не е фабрикувано.

Сигурност (OWASP) — чисто

  • SQL инжекции (A03): нула. Всяка потребителска стойност достига заявките като обвързан ? параметър. CPV кодовете минават през /^\d{5}$/ (двойно — в route parser-а cpvGroupSelection и в validCpvGroups), дедупликирани и ограничени до 10; cpvGroupsClause/cpvGroupRange обвързват границите като параметри. Единствените интерполирани фрагменти са константи (periodLen от enum, by избира между два фиксирани SQL низа). overruns.ts (561 реда) — всеки IN (…) е само ? плейсхолдъри.
  • XSS (A03): нула опасни sink-ове — няма dangerouslySetInnerHTML, innerHTML, eval, new Function, нито javascript: href-ове. Всичко минава през стандартния escaping на React.
  • Cache poisoning / CWE-349 (A01): cache-key allow-list-ът включва всеки параметър, който влияе на отговора — cur, cpv (повтарящ се), angle, step, sort, cpvSort, year, by. forecast не е URL параметър (извежда се от кешираните данни), затова коректно не се ключова. cur е валидиран строго до '1', за да остане ключовото пространство двузначно.
  • Нула изходящи мрежови заявки, нула тайни, нула обфускиран/зловреден код в целия diff.

Цялост на данните

  • Изключването на текущия (as_of) непълен период става преди zero-fill, на всичките три стъпки (месец/тримесечие/година); as_of идва от доверения rollup home_totals. YoY се потиска за частичната година — коректно.
  • Регресията с .lens-* класовете от feat(web): dashboard design-system base (MetricInfo, fullscreen, tokens, overrun index) #169 е разрешена тук: analytics.tsx е пренаписан, няма нито едно .lens- използване в TSX, а CSS правилата са премахнати чисто.
  • Миграция 0002 е идемпотентен частичен индекс (CREATE INDEX IF NOT EXISTS … WHERE annex_count > 0) — недеструктивна, обратима с DROP INDEX.
  • Локални тестове минават: уеб 77, DB 85, config 14 (buildForecast, estimateYoyGrowth, cache-key drift guard, overruns SQL — всички зелени).

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

  1. cache-key.ts не канонизира реда на повтарящите се cpv стойности (params.sort() сортира по ключ) — ?cpv=A&cpv=B и ?cpv=B&cpv=A дават два кеш записа за една и съща селекция. Само въпрос на ефективност — телата са коректни; авторът разчита на hrefToggleCpv да пише подредено.
  2. cache-key.test.ts изпуска обратната „няма остарели allow-list записи" проверка. Over-keying винаги е безопасен (никога не сервира грешни данни), така че е приемливо.

Процесна бележка

PR-ът е draft и diff-ът спрямо main носи целия стек (#169#172, ~9700 реда). Да не се слива преди основния стек — както е записано в описанието. След сливането на стека и un-draft, от гледна точка на код е готов.

VERDICT: ОДОБРЯВАМ ✅ — сигурност и цялост на данните чисти; сливане само след стека #169#172.

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.
@StanislavBG
StanislavBG force-pushed the feat/trend-forecast branch from 6714063 to 1ce433c Compare July 3, 2026 18:38
@ydimitrof

Copy link
Copy Markdown
Contributor

Ревю на PR #192 — „прогноза в тренда (проекция по сезонен профил)"

Благодаря за изчерпателното описание и за дисциплината прогнозата да е внесена като отделна, включваема функция върху върха на стека. Прегледах кода изключително внимателно, с акцент върху сигурност, инжекции и цялост на данните — ръчно и с локални проверки върху feat/trend-forecast (merge-base 5d99fd9, връх 90a734e). Тъй като PR-ът е член 5 на стек, diff-ът спрямо main носи целия стек (#169#172, ~9700 реда); ревюто по-долу отделя реално новата за този PR работа от вече прегледаните долни членове.

Обхват и съответствие с описанието
Новата работа — buildForecast (lib/trends-forecast.ts), каноничният estimateYoyGrowth (lib/analytics-stats.ts), прогнозният слой в ComboTrendChart и интеграцията в /trends — отговаря точно на описания метод: сезонен профил (същият календарен месец отпреди година) × YoY фактор, 3-годишен пълзящ медиан с clamp [0.5, 2], само на месечна стъпка, потискане при липса на сезонна база. Нищо не е фабрикувано — всяка проекция се извежда server-side от реалните trend.points; при липса на база отпреди година (out[0].valueEur === 0) прогнозата се потиска изцяло, вместо да рисува фалшив „срив към нула". Съгласуването с cur toggle-а е коректно: buildForecast(...).filter((f) => f.period > lastShown) гарантира, че един и същ период никога не е едновременно реален и прогнозен.

Сигурност (OWASP) — чисто

  • A03 Инжекции: прогнозният код не докосва БД — чиста аритметика върху вече заредени точки. В целия diff нула конкатенации на потребителска стойност в SQL: интерполираните фрагменти (OVERRUN_WHERE, DELTA, PCT, SECTOR_KEY_SQL, CPV_CLEAN) са статични константи, IN (…) е само ?-плейсхолдъри (contractIds.map(() => '?')), а CPV кодовете минават през /^\d{5}$/, дедупликирани и ограничени до 10. execFileSync('sqlite3', …) съществува само в тестов харнес с фиксирани аргументи и hardcoded извадки — няма runtime повърхност.
  • A03 XSS: нула опасни sink-ове. ComboTrendChart е чисто SVG — етикетите „ПРОГНОЗА" и tooltip badge-ът са статични низове, всичко минава през стандартния escaping на React. Няма dangerouslySetInnerHTML, innerHTML, eval, new Function, нито javascript: href.
  • A01 Cache poisoning (CWE-349): forecast коректно НЕ е URL параметър — извежда се от вече кешираните данни, затова отсъствието му от cache-key allow-list-а е правилно, не пропуск.
  • Нула изходящи мрежови заявки, нула тайни, нула обфускиран/зловреден код в целия diff.

Цялост на данните

  • YoY естиматът брои само пълни години (12 непартиални месеца, положителен сбор) и взима последните 3 — ранните ramp-up години на корпуса не изкривяват фактора. clampGrowth пази срещу еднократни аномалии и не-крайни стойности.
  • Плаващата запетая в проекцията е приемлива — това е визуална прогноза за графика, не счетоводна сметка; резултатите се закръгляват при рендиране (Math.round(hp.contracts)).
  • Тестовете покриват съществените пътища на buildForecast (сезонно пренасяне, старт след последния пълен месец, празна серия, потискане без база) и estimateYoyGrowth. CI е зелен.

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

  1. buildForecast при много дълга история без партиални маркери пада на points[points.length - 1] за „последен пълен" — безопасно, но си струва коментар, че разчита на upstream да маркира partial коректно.
  2. Забележките по cache-key реда на повтарящите се cpv от предишното ревю остават в сила (само ефективност, не коректност).

Процесна бележка: PR-ът е draft и стъпва върху #169#172. Да не се un-draft-ва/слива преди основния стек — както е записано в описанието.

ВЕРДИКТ: ОДОБРЯВАМ ✅ — по същество; сигурност и цялост на данните чисти. Сливане само след стека #169#172.

@ydimitrof

Copy link
Copy Markdown
Contributor

Ревю на PR #192 — „прогноза в тренда (проекция по сезонен профил)"

Благодаря за изрядната документация и за дисциплината прогнозата да влезе като отделна, включваема функция върху върха на стека (#169#172), държана извън опашката по продуктово решение. Прегледах кода много внимателно — ръчно и с фокус върху сигурност, инжекции и цялост на данните — локално върху feat/trend-forecast спрямо main. CI е зелено.

Обхват и съответствие с описанието

Реалната нова работа отговаря точно на описания метод. buildForecast (apps/web/app/lib/trends-forecast.ts) извежда всеки бъдещ месец като същия календарен месец отпреди година × YoY фактор, така че сезонната форма се пренася напред, не плоска линия. Факторът е каноничният estimateYoyGrowth (3-годишен пълзящ прозорец, медиана на съседните годишни съотношения, clamp [0.5, 2]), преизползван — не дублиран. Прогнозата стартира след последния пълен месец, спира в края на следващата година (макс. 18 месеца) и се потиска изцяло при липса на сезонна база (out[0].valueEur === 0[]), вместо да рисува фалшив „срив към нула". Всичко се извежда server-side от trend.points — нищо не е фабрикувано.

Сигурност (OWASP) — чисто

  • A03 SQL инжекции: нула. В новите/докоснатите заявки (overruns.ts, trend.ts) всяка потребителска стойност достига SQL като обвързан ?. Единствените интерполирани фрагменти са константи (OVERRUN_WHERE, PCT/DELTA/SIGNING, CPV_CLEAN, SECTOR_KEY_SQL, periodLen от enum) или генерирани ?-плейсхолдъри (c.id IN (…), CPV range clause). CPV кодовете минават през /^\d{5}$/, дедуп, cap 10.
  • A03 XSS: нула опасни sink-ове. Няма dangerouslySetInnerHTML, eval, innerHTML в новите компоненти. ComboTrendChart, MetricInfo, FullscreenButton рендерират през JSX-escape; SVG числата минават през toFixed, текстът в tooltip-а — през money/count/monthYear.
  • A01/CWE-349 cache drift: ?cur е валидиран до точно '1', ?cpv/?year са ограничени и остават keyed в edge cache — пространството на ключа е ограничено, hostile input не може да минти неограничени варианти.
  • Клиентските ефекти (useFullscreen, MetricInfo outside-click/Esc) са SSR-безопасни и почистват listener-ите си (removeEventListener в cleanup) — без leak.

Цялост на данните

  • estimateYoyGrowth ползва само пълни години (12 непартиални месеца, положителен тотал), медиана върху пълзящ прозорец — защитава срещу ramp-up аномалията 2020→2021; тестван за clamp и median-vs-CAGR. filter((f) => f.period > lastShown) върху нулево-подплатен YYYY-MM е коректно лексикографско сравнение, така че при „вкл. текущия месец" същият период никога не е едновременно реален и прогнозен.
  • Миграция 0002 е идемпотентна (CREATE INDEX IF NOT EXISTS), частичен индекс по annex_count > 0 — коректен и обоснован.
  • Прогнозата е само на месечна стъпка (методът е month-seeded) и никога не се рендерира в стила на реалните данни (щрихована линия, регион „ПРОГНОЗА", badge) — честно визуално разграничение.

Дребни бележки (не блокиращи)

  • PR-ът носи целия стек (+9729/−444); същинската нова работа е малка и добре покрита с тестове (trends-forecast.test.ts, analytics-stats.test.ts). Оценявайте я на diff-а спрямо feat(web): analyze index — five equal analysis cards #172, не в изолация — при merge извън ред числата подвеждат.
  • buildForecast компаундира растежа (проектиран месец може да засее следващ проектиран месец) — документирано и умишлено за сезонния carry-forward; само отбелязвам, за да е явно при бъдещи промени на хоризонта.

Няма следи от злонамерен код, скрити мрежови повиквания, четене на secrets или обфускация в диапазона на промяната.

Вердикт: Approve на същество — да се un-draft-не и слее след като стекът #169#172 влезе в main (в текущия си вид е draft по дизайн; готов за merge след стека).

@ydimitrof

Copy link
Copy Markdown
Contributor

Финалната проверка е готова. Тестовете не тръгват в изолирания worktree само защото workspace-пакетите (@sigma/shared) не са билднати там — не е дефект на кода; CI е зелен. Кодът на прогнозата е чист.


Ревю на PR #192 — „прогноза в тренда (проекция по сезонен профил)"

Здравейте! Прегледах промяната изключително внимателно, с акцент върху сигурност, целостта на данните и потенциален зловреден или инжекционен код. По-долу са изводите.

Обхват и съответствие с описанието

Реализацията отговаря точно на описанието в PR-а:

  • apps/web/app/lib/trends-forecast.tsbuildForecast, преизползва estimateYoyGrowth от analytics-stats (единствен източник на растежния фактор).
  • ComboTrendChart — прогнозен слой (регион, щрихована линия, бледи барове, tooltip badge).
  • /trends — прогноза само на месечната стъпка, легенда, подрязване при „вкл. текущия месец".

Стекова зависимост (#169#170#171#172, член 5) е коректно декларирана; draft-ът се държи извън опашката по продуктово решение и следва да се un-draft-не и слее след стека — не преди.

Сигурност / OWASP

  • SQL инжекции (A03): Прогнозата е чиста клиентска аритметика върху вече заредени, параметризирани серии — няма SQL. Прегледах и SQL в стека локално: всички потребителски стойности минават през ? placeholders. Единствените интерполации в заявките са вътрешни константи или enum-ограничени стойности — periodLen е 4/7 от валидиран TrendGranularity (trend.ts:138), leaderboardOrderBy връща фиксирани низове от типов union (overruns.ts:245–246), OVERRUN_WHERE/SECTOR_KEY_SQL са константи. Няма конкатенация на вход.
  • XSS (A03): Целият прогнозен изход се рендира през React escaping; SVG координатите са числови (toFixed), стойностите — през money/count. Нула dangerouslySetInnerHTML, eval, innerHTML.
  • Зловреден код / supply chain: Няма нови зависимости, URL-и, secrets или мрежови изходи. execFileSync присъства само в тестови файлове (sqlite3-CLI интеграции), не в продукционен път.

Цялост на данните

Изчислението е коректно защитено:

  • Растежът е clamp-нат в [0.5, 2], non-finite / ≤0 → 1 (clampGrowth); делене само при prev.value > 0 (estimateYoyGrowth).
  • Прогнозата тръгва след последния пълен месец; частичният месец никога не сее проекцията.
  • Липсваща сезонна база → 0, и целият слой се потиска, вместо да се нарисува фалшив „срив към нула" (trends-forecast.ts:68).
  • Подрязването f.period > lastShown върху YYYY-MM е лексикографски коректно; същият период не е едновременно реален и прогнозен.
  • Всеки прогнозен ред е флагнат forecast: true и никога не се рендира в стила на реалните данни.

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

  • Потискането проверява само out[0]. Вътрешен нулев месец при пропуск в серията е теоретично възможен, но fillPeriods не оставя дупки, така че на практика не възниква. Приемливо.
  • Локалното изпълнение на trends-forecast.test.ts / analytics-stats.test.ts в ревю-worktree-а не тръгна поради небилднат @sigma/shared — среда, не код. CI е зелен (1m51s), тестовете покриват clamp, сезонния пренос и потискането.

Заключение

Чиста, добре тестирана и честна към данните функция; без проблеми по сигурност или цялост. Единственото условие е редът на сливане: само след стека #169#172 и un-draft.

ВЕРДИКТ: Approve на същество — да се слее след стека #169#172; без блокиращи забележки.

@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Одобрявам прогнозата (90a734e): estimateYoyGrowth гардира всяка дивизия (prev.value > 0, prev.count > 0), clamp [0.5, 2], отхвърля non-finite/≤0; buildForecast се самоизключва при липса на сезонна база (out[0].valueEur === 0 → []) — без фалшив zero-cliff, без фабрикувано ниво. Cache-key чисто (прогнозата е безусловна на month grain, никакъв нов searchParam).

Cross-PR: #193 (methodology) описва /trends като „Няма прогноза — само реални стойности" (methodology.tsx:436), а този PR добавя точно прогноза. Ако и двата влязат, methodology-то ще описва грешно /trends — нужна е последователност/ред на merge с #193.

@StanislavBG

Copy link
Copy Markdown
Contributor Author

Closing — the seasonal-forecast projection isn't something we want to ship. Dropping this feature rather than carrying it forward as dormant draft work.

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.

3 participants