Skip to content

docs: codify review standards into docs/ guides and index them - #143

Merged
todorkolev merged 3 commits into
midt-bg:mainfrom
StanislavBG:docs/codify-review-standards
Jul 3, 2026
Merged

docs: codify review standards into docs/ guides and index them#143
todorkolev merged 3 commits into
midt-bg:mainfrom
StanislavBG:docs/codify-review-standards

Conversation

@StanislavBG

Copy link
Copy Markdown
Contributor

Какво и защо

Събира повтарящите се бележки от ревютата в пет кратки ръководства в docs/, за да може новите
контрибутори да покриват очакванията на първи опит, а ревюто да се повтаря по-малко. Само
документация — без промени по кода.

Новите файлове:

  • docs/review-accuracy.md — точност и коректност (блокер за merge): единна база за стойността
    (amount_eur IS NOT NULL), непълни периоди + YoY, 404 за несъществуващи обекти.
  • docs/review-accessibility.md — достъпност и UI: sr-only role="status" за авто-submit филтри
    (flows.tsx), палитрени токени, графики/SVG fallback.
  • docs/review-security.md — Cloudflare/кеш/сигурност: ключове за кеш (CWE-349), rate limiting,
    CSP, валидация, D1 индекси, AI асистент.
  • docs/review-testing.md — тестове и CI: pnpm typecheck vs vitest, регресионни тестове,
    pnpm audit, integrity gate.
  • docs/review-code-and-process.md — структура на кода и PR процес: преизползване, SQL в
    @sigma/db, координация на merge.

Индексът docs/README.md става пълен (съществуващи + нови), а коренният README.md сочи към него.

Бележка: правилата са изведени от реални ревюта; част от тях описват поведение, което все още е в
отворени PR-и (#131 езиков префикс в кеш-ключа, #132 cache-key drift guard). Може да изчака след
тях при нужда — нищо в този PR не докосва техните файлове.

Свързан issue

Няма — само документация. Радвам се да отворя issue, ако предпочитате да се проследи.

Вид промяна

  • docs — документация

Как е тествано

  • pnpm lint (prettier --check) — чисто за всички нови и променени файлове.
  • Без промени по кода, затова typecheck/test не са приложими.

Чеклист

  • Комитите следват conventional commits и нямат Co-Authored-By: trailer
  • PR-ът е с един логически обхват и е от форк към midt-bg/sigma:main
  • pnpm typecheck минава (неприложимо — без код; не е счупено)
  • pnpm test (поне за засегнатите пакети) минава (неприложимо — без код)
  • pnpm lint е чисто
  • Няма комитнати тайни, .env* или .dev.vars
  • Документацията в docs/ е обновена (това е целта на PR-а)

Codify recurring maintainer review feedback into five docs/ guides
(accuracy, accessibility, security, testing, code-and-process), make
docs/README.md the complete index of all docs (existing + new), and
point the root README at that index, so contributors meet the bar on
the first pass.

@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.

Полезен PR! Проверих конкретните твърдения срещу реалния код и преобладаващо са точни: базата amount_eur IS NOT NULL за сумите, integrity gate-ът, vitest конфигурацията (environment:'node' + app/**/*.test.ts), rate-limit assert-ът, unit тестовете на helper-ите, имената на пакетите. Това е трудната част в такъв документ и е свършена добре.

Дребно (LOW): в review-accuracy.md „slug на възложител" — възложителите се map-ват по ЕИК (params.eik в authority.tsx), не по slug; примерите с ЕИК и graph-node са си верни.

@StanislavBG

Copy link
Copy Markdown
Contributor Author

Addressed the wording note in b815c7e: review-accuracy.md now says authorities key on ЕИК (params.eik in authority.tsx), not slug — and lists slug as the company identifier.

@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Прегледах петте docs (@ 537744e) срещу кода. review-accuracy.md го сверих ред по ред: единната стойностна база (amount_eur IS NOT NULL = rollup-ите), непълните периоди (YoY=null), 404 вместо кеширана празна 200 — точно инвариантите, които налагаме, вярно описани; бележката за остарели DTO doc-коментари (ред 39–41) кодифицира урока от #182. Останалите четири прочетох като проза — чисти. Approve. (Triage.)

@todorkolev
todorkolev merged commit 75adbb2 into midt-bg:main Jul 3, 2026
@ydimitrof

Copy link
Copy Markdown
Contributor

Проверих PR #143 изцяло — целия diff (259 добавени реда, 0 изтрити), описанието, всички коментари по ревюто и сверих ключовите твърдения срещу живия код.

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

PR-ът е само документация: пет нови ръководства в docs/ (review-accuracy.md, review-accessibility.md, review-security.md, review-testing.md, review-code-and-process.md), плюс редакция в docs/README.md (индекс) и един ред в кореновия README.md. Няма промени по изпълним код, конфигурация, миграции, workflow-и или зависимости. Обхватът е един логически (кодификация на повтарящи се ревю-бележки) и съответства на описанието — „само документация, без промени по кода“.

Сигурност и data integrity (OWASP)

  • Тайни: няма ключове, токени, пароли или частни ключове в новите файлове (сканирано).
  • Инжекции (SQL/XSS): няма изпълним код, SQL, шаблони или dangerouslySetInnerHTML в diff-а — нулева attack surface. Иронично, самите документи налагат правилните мерки (bound параметри в D1, .replace(/</g, '\\u003c') преди dangerouslySetInnerHTML, валидация на ЕИК + encodeURIComponent), което е в духа на OWASP A03.
  • Външни връзки / supply chain: новите файлове съдържат само относителни вътрешни линкове — никакви външни URL, скриптове или обфускиран payload. Единствените абсолютни URL в README.md (sigma.midt.bg, storage.eop.bg, localhost) са заварени и не са пипани от този PR.
  • Малициозен код: няма backdoor, code injection, замаскирано съдържание или каквото и да е, което да застраши проекта. Markdown-ът е инертен.

Сверка с кода (точност на твърденията)

Понеже документ, който описва невярно поведение, сам по себе си е дефект, проверих каноничните примери на място — всички са коректни:

  • authority.tsx:44,55throw new Response('Not Found', { status: 404 }) върху params.eik, точно както твърди review-accuracy.md.
  • flows.tsx:105<p className="sr-only" role="status">, каноничният авто-submit шаблон от review-accessibility.md.
  • amount_eur IS NOT NULL е реалната единна база в packages/db/src/queries/* (authorities, companies, flows, regions, competition, trend).

Бележката за ЕИК-vs-slug от @lyubomir-bozhinov е адресирана коректно в b815c7e; двата follow-up коментара (авторитетите се ключват по ЕИК; „single-offer“ базата е узаконено изключение) са отразени в текста.

Дребни, незадължителни наблюдения (non-blocking)

Заключение

Чиста, добре структурирана и фактологически проверена документация. Нулев риск за сигурност или интегритет на данните; напълно OWASP-съвместимо по подразбиране (няма код за атакуване). Съответства на описанието и на приложимите ревю-стандарти. Благодаря за прегледната работа — това реално сваля товара от бъдещите ревюта.

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

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