docs: ADR директория, речник на данните и docs-integrity проверка (#101, #102, #75) - #182
Conversation
nedda76
left a comment
There was a problem hiding this comment.
Прегледах PR-а. Консолидацията на docs backlog-а е добра и точна — проверих, че „само документация“ важи (всички не-doc промени са само коментари), а преформулираните коментари за value_suspect/amount_eur в packages/db/migrations/0000_init.sql са коректни: съвпадат с scripts/normalize-raw.sql:303-320 (value_suspect се поправя до procedure estimate и се сумира), старият „NULL, never summed“ коментар беше остарял. docs/etl.md съществува, репойнтите резолват.
Единствените находки са в новия checker scripts/check-docs.mjs — три места, където може да счупи CI фалшиво или тихо да не провери нищо (детайли по редовете).
За CI: червеното е от RiskIndicators.tsx / riskLogic.test.ts — заварен prettier дълг на main (оправя се в #174), не от този PR. Собствените му файлове са чисти; ще позеленее след merge на #174 или rebase.
midt-bg#182 review found the docs gate itself had three bugs a happy-path run could not surface: - findOrphanDocs treated any text file under docs/ as a doc to index, so a docs/*.json|yaml would false-fail CI. Now filters to .md. - DOCS_REF matched docs/*.md inside external URLs, so a URL in a comment would false-fail CI. Now skips refs preceded by '/' (a real ref is root-relative). - the file://${argv} main-module guard mismatched on encoded paths (space / non-ASCII), so the gate silently exited 0 without checking. Now pathToFileURL. The matching logic is now pure and covered by scripts/check-docs.test.mjs (node:test), wired into CI ahead of the gate so the checker is itself gated. Test files are excluded from the ref scan so their fixture paths aren't read as references.
|
Благодаря — и трите са поправени в
Писането на тестовете извади и 4-ти проблем: gate-ът четеше собствените си fixture низове в |
|
Всички проверки минаха. PR-ът е docs-only плюс една CI проверка; единственият изпълним код ( Прегледах PR #182 внимателно — целия diff (759/-79 през 22 файла), всички коментари по нишката, трите inline находки на @nedda76, новия чекер и неговите тестове, както и сверих документните твърдения ред по ред срещу реалния SQL. Благодаря за изчистената, добре аргументирана работа. Обхват и съответствие с issue-тата
Сигурност / OWASP / data integrity
Чекерът — трите находки на @nedda76 са реално затворени и покрити с тестове: Незадължителни бележки (не блокират):
Verdict: APPROVE ✅ |
Consolidates the documentation backlog and adds a CI check that keeps it honest. - ADR process (midt-bg#101): docs/adr/ with template + index; ADR-0001 moved out of architecture.md; backfilled ADRs for D1, value_flag, style-src, blue/green rollback, and the EOP-wins dedup. architecture.md is now a system overview plus an ADR map. - Data dictionary + dangling refs (midt-bg#102): docs/core-scope.md (domain model, value_flag/date_flag, amount_eur semantics, rollups, sectors) and docs/v1-implementation-plan.md (precompute + keyset pagination). Repoint the five source comments that referenced non-existent docs. - README heading term (#75): use „хранилище" consistently. - scripts/check-docs.mjs + pnpm check:docs + CI step: fail on any dangling docs/*.md reference in source or any unindexed doc. Closes midt-bg#101 Closes midt-bg#102 Closes #75
Review found core-scope.md and ADR-0003 stated the value basis backwards — propagated from a stale schema comment in 0000_init.sql. Against scripts/normalize-raw.sql and scripts/precompute.sql: - value_flag has FIVE values (added value_low); all five are summed — amount_eur is populated for every flag (value_suspect is repaired to the procedure estimate, annex_suspect falls back to signing/current). - amount_eur is NULL only when there is no trustworthy EUR figure: an FX-rateless foreign row, a value_suspect row without a procedure estimate, or a row with no signing/current value. NOT "NULL iff value_suspect". - home_totals.suspect counts value_suspect rows (a KPI); those rows are themselves summed — distinct from the NULL-amount_eur rows excluded from sums. - Fixed the stale 0000_init.sql schema comments (value_flag enum, amount_eur, suspect) that were the source of the error. Also: ADR-0002 reworded (consolidated baseline + thin index migration, not a single file) and "bulk снимки" -> "bulk зареждания".
midt-bg#182 review found the docs gate itself had three bugs a happy-path run could not surface: - findOrphanDocs treated any text file under docs/ as a doc to index, so a docs/*.json|yaml would false-fail CI. Now filters to .md. - DOCS_REF matched docs/*.md inside external URLs, so a URL in a comment would false-fail CI. Now skips refs preceded by '/' (a real ref is root-relative). - the file://${argv} main-module guard mismatched on encoded paths (space / non-ASCII), so the gate silently exited 0 without checking. Now pathToFileURL. The matching logic is now pure and covered by scripts/check-docs.test.mjs (node:test), wired into CI ahead of the gate so the checker is itself gated. Test files are excluded from the ref scan so their fixture paths aren't read as references.
Strict cell-by-cell re-review found more docs asserting invariants that did not match the code — same failure mode as the value_flag fix (trusting stale schema comments over the computation): - core-scope.md claimed all rollup sums filter amount_eur IS NOT NULL and that home_totals.contracts/value_eur cover one set. precompute.sql:97 computes home_totals.contracts = COUNT(*) over ALL contracts (corpus record count) and facet_counts has no amount_eur filter; the corpus count and the clean sum do NOT cover one set (precompute.sql:16-19). Corrected, and fixed the stale 0000_init.sql:201-202 comments that were the source. - v1-implementation-plan.md claimed "no cheap COUNT(*)" limits page N-of-M. companies.ts:185 / authorities.ts:145 run COUNT(*) over the rollup per request and return total; pageCursors gives page jumps, filterSignature resets the cursor. Corrected. - v1-implementation-plan.md conflated NULL amount_eur with value_suspect in the keyset sort note; NULL rows are FX-rateless etc., not value_suspect. Fixed. - Minor: contracts id is a c:e:/c:o: composite, not "c: + staging row id".
…nk parsing Independent adversarial pass over the PR returned a clean verdict (no blocker/major, all prior findings re-confirmed closed against code). Three items closed: - ADR-0001: corpus count "129 хил." -> "190 хил." to match core-scope/deploy/ v1-plan (it was moved verbatim from the old architecture.md, never re-derived). - 0000_init.sql suspect comment: note the value_suspect-without-estimate case (amount_eur NULL -> excluded), so "summed" is not overstated. - check-docs linkTargets: also parse titled (](x.md "t")) and reference-style ([a]: x.md) links so a doc linked those ways is not false-flagged orphan; covered by an expanded unit test.
fcfb08a to
390c506
Compare
todorkolev
left a comment
There was a problem hiding this comment.
Прегледано - чисто и безопасно за мърдж. Промяната по 0000_init.sql е само коментари (без DDL/DML), а репото нарочно държи схемата в един редактируем файл (pre-production, свежа база при всеки импорт), така че не е риск за възпроизводимостта. CI минава.
Две дребни, неблокиращи неща за отделно после: несъответствието за facet_counts в docs/core-scope.md:119 (year се смята на живо, не се попълва) и че check-docs.mjs е проверка за съществуване на референции, а не пълен link checker.
…mber worker adr to 0008 Rebase of midt-bg#183 onto upstream/main (post-midt-bg#182 ADR reorganization) restructured the privacy-policy and worker-level X-Robots-Tag ADRs to live in docs/adr/ rather than inline in docs/architecture.md: - New docs/adr/0007-privacy-masking.md — content extracted from the inline ADR-0002 in architecture.md; relative paths adjusted (../ → ../../) for the new adr/ location; cross-link to the worker ADR now points to 0008. - docs/adr/0003-centralized-x-robots-tag-worker.md → docs/adr/0008-centralized-x-robots-tag-worker.md — renumbered to free the 0003 slot taken by upstream's value-flag ADR; internal cross-link from architecture.md#adr-0002-... to 0007-privacy-masking.md. - docs/adr/README.md — index extended with the two new entries. - docs/architecture.md — adopted upstream's short summary form; the inline ADR-0001+0002 contents are removed (the rendering ADR lives at adr/0001-rendering-and-security.md and the privacy policy at adr/0007-privacy-masking.md); Решения (ADR) section now also points to 0007 and 0008. - docs/privacy-masking.md — cross-link from architecture.md#adr-0002-... to adr/0007-privacy-masking.md; ADR-0003 to ADR-0008. No code changes; verified pnpm check:docs (docs-integrity gate from midt-bg#182) passes.
…mber worker adr to 0008 Rebase of midt-bg#183 onto upstream/main (post-midt-bg#182 ADR reorganization) restructured the privacy-policy and worker-level X-Robots-Tag ADRs to live in docs/adr/ rather than inline in docs/architecture.md: - New docs/adr/0007-privacy-masking.md — content extracted from the inline ADR-0002 in architecture.md; relative paths adjusted (../ → ../../) for the new adr/ location; cross-link to the worker ADR now points to 0008. - docs/adr/0003-centralized-x-robots-tag-worker.md → docs/adr/0008-centralized-x-robots-tag-worker.md — renumbered to free the 0003 slot taken by upstream's value-flag ADR; internal cross-link from architecture.md#adr-0002-... to 0007-privacy-masking.md. - docs/adr/README.md — index extended with the two new entries. - docs/architecture.md — adopted upstream's short summary form; the inline ADR-0001+0002 contents are removed (the rendering ADR lives at adr/0001-rendering-and-security.md and the privacy policy at adr/0007-privacy-masking.md); Решения (ADR) section now also points to 0007 and 0008. - docs/privacy-masking.md — cross-link from architecture.md#adr-0002-... to adr/0007-privacy-masking.md; ADR-0003 to ADR-0008. No code changes; verified pnpm check:docs (docs-integrity gate from midt-bg#182) passes.
…mber worker adr to 0008 Rebase of midt-bg#183 onto upstream/main (post-midt-bg#182 ADR reorganization) restructured the privacy-policy and worker-level X-Robots-Tag ADRs to live in docs/adr/ rather than inline in docs/architecture.md: - New docs/adr/0007-privacy-masking.md — content extracted from the inline ADR-0002 in architecture.md; relative paths adjusted (../ → ../../) for the new adr/ location; cross-link to the worker ADR now points to 0008. - docs/adr/0003-centralized-x-robots-tag-worker.md → docs/adr/0008-centralized-x-robots-tag-worker.md — renumbered to free the 0003 slot taken by upstream's value-flag ADR; internal cross-link from architecture.md#adr-0002-... to 0007-privacy-masking.md. - docs/adr/README.md — index extended with the two new entries. - docs/architecture.md — adopted upstream's short summary form; the inline ADR-0001+0002 contents are removed (the rendering ADR lives at adr/0001-rendering-and-security.md and the privacy policy at adr/0007-privacy-masking.md); Решения (ADR) section now also points to 0007 and 0008. - docs/privacy-masking.md — cross-link from architecture.md#adr-0002-... to adr/0007-privacy-masking.md; ADR-0003 to ADR-0008. No code changes; verified pnpm check:docs (docs-integrity gate from midt-bg#182) passes.
Консолидира docs backlog-а и добавя CI проверка, която го държи честен.
Само документация + един check — продуктовият код не е пипан (репойнтите са само в коментари).
#101 — ADR процес
docs/adr/с шаблон (_template.md) + индекс (README.md).architecture.md(git mv, историята запазена).value_flag,style-src 'unsafe-inline', blue/green rollback, EOP-wins dedup.architecture.mdвече е overview на системата + карта към ADR-ите; всички входящи „виж architecture.md" връзки остават валидни.#102 — речник на данните + висящи препратки
docs/core-scope.md— domain модел и речник: таблици, rollup-и,value_flag/date_flag, семантиката наamount_eur(единна базаamount_eur IS NOT NULL), сектори.docs/v1-implementation-plan.md— precompute слой + keyset пагинация.etl-pipeline.md,mock-coverage.md), са пренасочени към реалните файлове.#75 — терминология
Проверката
scripts/check-docs.mjs+pnpm check:docs+ CI стъпка: fail при висящаdocs/*.mdпрепратка в кода ИЛИ неиндексиран doc — в идиома наintegrity-gate.mjs. Прави bug class-а на Документация: липсващитеdocs/etl-pipeline.mdиdocs/core-scope.md+ речник на данните #102 невъзможен за повторно внасяне.Странични находки (follow-up, не пипнати тук)
etl-pipeline-state.md,spec/assistant-contracts.md) — индексирах ги.etl-pipeline-state.mdе „анализ на текущото състояние" (point-in-time) — кандидат за преместване/архивиране отделно.Closes #101
Closes #102
Closes #75