feat(related-persons): регистърни доказателства за връзките (#279) - #2
feat(related-persons): регистърни доказателства за връзките (#279)#2ydimitrof wants to merge 60 commits into
Conversation
Test coverage
✅ No workspace dropped below its baseline (tolerance 0.5pp). 📈 Coverage rose by more than 1pp — run |
…tier Records the decision before any code: a link publishes only against a checkable Trade Register fact (midt-bg#279). If review rejects the grounds, the whole approach falls — which is why this ADR comes first rather than last. Seven decisions, each separately contestable: the evidence ladder (six outcomes, first match wins); a heuristic that grounds an assertion — the explicit exception to ADR-0007 decisions 2 and 3, together with what justifies it (corroboration against the official's own filing, never a standalone name match) and the filters that can only withhold; the joint-stock bar as a union of three signals, where an unknown legal-form code withholds; reconciliation of a terminated stake against the live deed, reversing ADR-0021 E11 (the "и към днешна дата" labels defer to phase 2); the deed cache, extending ADR-0010 decision 6 with a 35-day retention; monotonicity as a gate rather than a store — §8 of the issue is false as written; and the launch gates. Supersedes ADR-0009, ADR-0015, ADR-0017 (headers and index synchronised). Amends ADR-0007, ADR-0010, ADR-0021 (E11 only), ADR-0028 — accepted ADRs are not rewritten, so the amendment is recorded here. Cross-checked against issue midt-bg#279 and the earlier spike; carries the three facts the issue does not: the sustained 429 at ~50 requests, the open question of the ЕАД legal-form code, and the deliberate non-use of the beneficial-ownership fields (C-37/20).
The index has marked it superseded by ADR-0030 since PR midt-bg#226, but the file header still read "Accepted". A header/index divergence, spotted while synchronising the index for ADR-0033.
746ea13 to
4fde377
Compare
ydimitrof
changed the title
DiyanaDimitrova
left a comment
DiyanaDimitrova
left a comment
There was a problem hiding this comment.
Strict review — ADR-0033 against ticket midt-bg#279 (midt-bg/sigma). Read both in full, verified every control number and every cross-ADR reference. No blocking issues. This is an exemplary ADR: it faithfully addresses midt-bg#279's seven concerns and corrects the ticket where live evidence contradicts it, rather than silently implementing a flawed spec. Findings below are documentation-quality (MEDIUM/LOW).
Scope
In: 0033-* (new, +309), status edits to 0009/0015/0017/0023, README index row. Docs-only. Security scan clean (no secrets; the sole URL is midt-bg#279's own registry endpoint; "token" hits are linguistic tokens).
Fidelity to midt-bg#279 — verified
The 7 decisions map 1:1 to the ticket, and the numbers reconcile exactly:
- identity row
281+102+156+21+4+4 = 568✓; surface by-evidence251+78 = 329✓; "264 new named claims" =329 − 65 published-survivors✓; §12 phantom-row fix255,582 + 617 = 256,286 − 87 = 256,199✓. - Every supersede/amend target (0007, 0009, 0010, 0015, 0017, 0021, 0023, 0028) exists on base;
0033is free onmain.
Where it diverges from midt-bg#279, it does so deliberately and with argument — and this is the ADR's main value:
- §9 cadence corrected: decisions can't ride the 6-hourly Worker ETL (no
node:sqlite/corpus/winner-set in a Worker) → daily workflow + monthly lookups. - §8 monotonicity corrected: "seal forever, strictly additive" is shown false (label flips, expiring cache, court-annulled entries); reframed as a gate, seals re-derived per run.
- §3 API facts corrected by live probing (2026-08-04):
?entryDate=is honoured,/Historydoes return JSON, wrongsubUICis 500 not 404 — the opposite of what §3 asserts, which matters because a 500 must never read as "no history / outside register". - §7 present-tense labels deferred to phase 2 behind an LIA addendum (they assert a present tense about a named person on cache-bounded freshness).
- Precision-first framing throughout: tripling the named surface raises the bar, so the acceptance gate is a hand-labelled zero-error sample, not the control totals. Fail-closed everywhere (unknown
legalFormwithholds, partial cache throws, 429 ends the run, returned UIC must equal requested ЕИК).
Findings
1. [MEDIUM — traceability] Amended ADRs get no back-pointer. 0033 amends 0007 (dec 2–3), 0010 (dec 3, 6), 0021 (E11), 0028 — but none of those files, nor their README status, is touched. Superseded ADRs correctly gain a "Superseded by" line; amended ones gain nothing, so a reader of ADR-0007 (the foundational certainty-bar) never discovers that 0033 carved an explicit exception to its decisions 2–3. The stated convention ("accepted ADRs aren't rewritten") already coexists with adding a supersede back-pointer without touching the body — the same one-line "Amended by ADR-0033 (decisions 2, 3)" breadcrumb closes the gap. Recommend it at least for 0007 and 0028.
2. [MEDIUM — surfaced but unresolved] The §5/§10 arithmetic gap is a real acceptance blocker. Decision 7 correctly catches that §5's "3 по ЕИК" confirmations fall in no §10 bucket (the row sums to 568 only without them) and makes control-number reproducibility contingent on resolving it "with the author." Excellent catch — but it means the control totals are currently un-reconcilable, so the ADR's own reproducibility check is externally blocked. Worth promoting from decision-7 prose to an explicit tracked PR blocker so it isn't lost.
3. [LOW — coordination] Confirm 0033 is uncontested. Free on base main (verified), but if a parallel open PR also claims 0033, whichever merges second renumbers (same hazard as migrations). 30-second check pre-merge.
4. [LOW — link integrity] Verify non-PR references resolve. spec §3.3, ../spec/related-persons-lia.md, and the ADR-0021 E10/E11 anchors underpin decision 7's "methodology page carries the rule verbatim" and decision 4's E11 reversal — a mechanical link-check would confirm they exist and are numbered as cited. (0031/0032 confirmed present.)
5. [LOW — cosmetic] README status vocab. 0033 uses "Прието (дизайн)", a string no other row uses ("Прието" / "Заменено от…"). Harmless unless the index column is ever parsed.
Verified
- ✅ All 6 control-number identities recomputed and consistent.
- ✅ All supersede/amend targets exist; 0009/0015/0017 status + README synchronized; 0023 status fix matches the pre-existing "заменя ADR-0023" README note for 0030.
- ✅ Divergences from midt-bg#279 are each argued, not silent.
- ✅ Deliberate scope beyond midt-bg#279's literal text (correcting its API facts / cadence / monotonicity) is appropriate for an ADR and sound.
Action: no changes required to accept the design. Fold in #1 (amend back-pointers) and track #2 (the §5/§10 reconciliation) as the one genuine pre-implementation dependency.
/conflicts rendered an empty surface locally — seed.sql carried only authorities, tenders and bidders, so the свързани-лица routes had nothing to show and the feature could not be verified in a browser at all. Adds two invented officials, three invented companies and the rows the read gate actually requires (packages/db/src/queries/related-persons.ts): published status with a surfaced ownership class, a live contract for the ЕИК, and signed_at inside the declared window so the „в периода" split is exercised. One link per outcome — a self stake, a family stake, and a joint-stock link left held — so a change to the publishing rule shows up as a before/after rather than as an empty page either way. Every person and company is invented; no real declarant, no real winner. The ЕИК pass the real 9-digit checksum (normalize-raw.sql's eik_valid), so the fixture cannot teach a wrong ЕИК shape.
…capability Foundations for the Trade Register evidence leg (midt-bg#279, ADR-0033). No crawling and no publishing decision yet — this is the transport, the cache and the identifier rule, each with the refusals that make the later steps safe. scripts/tr/eik.mjs — the ЕИК checksum, which until now was callable only from SQL (normalize-raw.sql's eik_valid). The crawler has to decide in Node whether a code is worth a lookup. A test lifts the CASE expression straight out of the .sql file and runs both implementations over ~120 values through node:sqlite, so the twin cannot drift from the rule it mirrors. scripts/tr/client.mjs — 429 is never retried. The register's block is sustained (an earlier spike saw 429 at ~50 requests and then 429 to everything, with no Retry-After and no quota header), so it is an instruction to stop, not a transient. 5xx and network faults retry with growing backoff; a 429 arriving mid-retry aborts rather than letting a later 200 mask the block. A foreign host is refused before any packet is sent. Two transport decisions worth stating: this leg does NOT reuse cacbg/tls.mjs — that module pins register.cacbg.bg's leaf because that host serves a broken chain (ADR-0011), whereas this host verifies against system roots, which is stronger. And it uses node:https, not fetch: measured live, the identical request returns 500/empty via undici and 200 with the full 34,398-byte deed via node:https. scripts/tr/cache.mjs — resumability plus the PII rail. The index holds no name at all: ЕИК, dates, codes, verdicts and a body hash, never an excerpt. Any value carrying a ten-digit run is refused outright — the ЕГН shape, and sound as a check precisely because an ЕИК is 9 or 13 digits, never 10. scripts/tr/*.test.mjs matched NO CI job before this commit; the workflow globbed only scripts/ and scripts/cacbg/. Fixed here rather than later, or every test above would have been decorative. cacbg/guard.mjs's assertScratchIgnored takes the subdirectory instead of being copied — a duplicated safety rail drifts from its original. Existing callers are unchanged.
The libel-critical part of midt-bg#279. Pure and offline: deed in, verdict out. Nothing is wired into the publishing path yet — that is the next commit. deed.mjs enforces one order of operations, and it is an invariant with tests on it rather than a comment: decode entities → split on record-container/hr--report → drop erased → strip tags within the entity → match tokens within ONE entity. Field CR_F_19_L routinely holds several съдружници in a single string; matching against the whole field lets one person's given name combine with another's surname, and the output is a named public claim about the wrong human being. The test proves both directions — the frankenstein name does NOT match, the correct declarant DOES, and the naive whole-field approach WOULD have fired. Without that last assertion a matcher that always returns false would pass the suite. evidence.mjs implements the ladder: bar_joint_stock · document · confirmed · refuted · unknown · outside_tr, first match wins. What it establishes is company IDENTITY, never that the official owns anything — the ownership claim is the official's own filed declaration. So the failure mode of a wrong match is a real official attached to the wrong company's contracts. Fixtures are real markup, copied from the live deed for ЕИК 115536179. Writing this against imagined markup is how the entity-boundary bug ships, and doing so caught two things the ADR had wrong. ADR-0033 corrected on implementation, both from re-measuring the full deed: • the erasure marker is `erasure-text-inline`, NOT `field-text--erased` — that class occurs zero times, and an erased container has no field-text paragraph at all. Both spellings are now honoured; assuming one costs a wrong publish. • fieldOperation is not the erasure signal (2 on erased fields here, 1 on the erased history records W0 sampled), so it is not relied on. Guards that can only withhold: an unknown legal form never falls through to a lower rung; a two-token declarant can never earn „Документ"; a Latin homoglyph is a non-match and is counted; an empty seat never confirms; a seat registered after the declared period does not confirm; refutation never applies to a family stake and is suppressed inside the 2011-2012 re-registration window; matched_fact is a closed vocabulary that cannot carry a name.
One request per candidate ЕИК, sequential, 1 per 3 s. What it refuses to do is
the substance, and every refusal is driven offline through injected I/O.
• The pace flag cannot make it faster, only slower. Tuning around a rate
limiter empirically is what spec §3.3's „NEVER bulk-scrape" forbids, so
--min-interval-ms below the documented 3 s is rejected outright.
• A 429 ends the run with exit 2 and records NOTHING about the ЕИК that hit
it — that ЕИК is unknown, not absent, and the throttle is a fact about us,
not about the company. A later re-run resumes exactly there.
• The candidate set is closed: it never follows a link out of a deed.
• A checksum-invalid code is dropped before any request.
• The returned deed's UIC must echo the request, or it is refused and not
cached — otherwise one company's deed lands under another's ЕИК.
Two things measured against the live register during this work:
„Outside the Търговски регистър" does NOT look like midt-bg#279 §3 describes. An ЕИК
that is not a търговец answers HTTP 200 with a ZERO-BYTE body — not a 404, not
HTML. Verified on Община София (000696327): empty on two consecutive requests
while a real company returned its full deed in the same window. The rule that
keeps R6 honest is therefore the STATUS, not the emptiness: empty under 200 is
the register's answer and may be cached permanently; empty under 5xx is a
failure and stays transient. Both directions are tested. Without this the ~4
non-търговец ЕИК would never resolve and the run could never exit 0.
The breaker threshold is 5, not 10, because the unit is ЕИК and each unresolved
one costs the full 5-attempt budget. At 10 the breaker would spend ~50 requests
against an already-failing endpoint — the exact volume at which the register was
observed to begin a sustained 429, i.e. the safety mechanism would itself have
tripped the block. At 5 the worst case is ~25.
The breaker test caught itself being fake: it had been generating made-up ЕИК of
which only 1 in 10 passed the checksum, so the crawler dropped them all and the
assertion passed on zero requests. It now generates genuinely valid codes and
asserts the REQUEST budget, not just the candidate count.
A side table, not columns on interest_links, for three reasons that are easy to
forget once the code is written: SQLite's ADD COLUMN has no IF NOT EXISTS and
these migrations are applied by a bare `d1 execute --file` with no ledger, so a
re-apply must be a no-op; load.mjs rebuilds the CACBG tables from 0003 alone, so
any column added here would have to be duplicated into 0003 and kept in step
forever; and §8 describes the seal as an attached artefact of a link.
A seal is written for EVERY link, not only published ones — the seals on held and
withdrawn links are what make the review queue reviewable.
matched_fact is a closed vocabulary ('seat:<CITY>' | 'role:owner:<FIELD>' |
'role:manager:<FIELD>' | 'eik') and must never carry the matched name; the deed's
names are read only to produce a boolean and never leave git-ignored scratch. A
schema cannot enforce that, so the audit will.
Wired everywhere the 0003 table list already is, and the two FK orderings are
opposite by necessity: the seal ships AFTER interest_links and is wiped BEFORE
it. D1 enforces foreign keys, so getting either backwards breaks the re-seed.
• ship-related-persons.mjs — TABLES and WIPE_ORDER, with a test asserting both
orderings and a third asserting every shipped table is also wiped (a table
that ships but is never wiped accumulates seals for links that no longer
exist).
• related-persons.ts CONFLICT_TABLES — so an environment without 0006 degrades
to an empty surface rather than a 500.
• load.mjs — applies 0003 AND 0006, and drops the seal first.
• BOTH related-persons-data.yml and deploy.yml hardcode migration filenames;
each needed its own step. Missing either leaves a D1 without the table.
ship-reseed.test.mjs builds its fixture from the migrations, so 0006 joins it —
the new table is part of the FK graph that test exists to police.
КДА — командитно дружество с акции — issues shares like an АД, so the same two reasons apply: the shareholder book is not public, making a declared parcel unverifiable, and the parcel may be immaterial. It was absent from both JOINT_STOCK and FORM_TOKENS, so a КДА read as CLOSELY HELD and its declared holder could reach the public ownership surface as an owner — the exact class of claim the exclusion exists to prevent. Two independent effects, hence both lists: JOINT_STOCK is the materiality bar, and FORM_TOKENS feeds the content-word count in nameDistinctiveness, where a form token miscounted as a content word inflates distinctiveness toward publishing. Found while implementing midt-bg#279, whose rung 1 names КДА explicitly alongside АД and ЕАД, but the gap predates it and is a bug on its own — hence a separate commit. The mirror cases stay correct: „КДА-ТРЕЙД ЕООД" and „КДА ГРУП ООД" are ООД.
The decision swap (midt-bg#279, ADR-0033). load.mjs no longer asks whether a declared name looks distinctive; it asks what the Trade Register says about that ЕИК. publishTier() and seatConfirmed() are deleted — one production caller each — and publish_tier now carries the evidence kind. FAIL CLOSED, twice. A missing cache refuses the load outright. A PARTIAL cache refuses it too, and that is the one that matters: it does not fail loudly downstream. An 80%-restored cache yields ~80 published links, which clears the ship floor of 50, ships a decimated surface and wipes the rest of the live links. --allow-partial-tr is the deliberate override, and an uncached ЕИК under it is „unknown" (held), never a reason to publish. The workflow now passes --min-links 250 explicitly rather than relying on the default of 50. tr-census.mjs is deleted. Its promote() moved C_hold → published on pure name uniqueness — identity by name coincidence, which is what this change abolishes. Two design errors the fixtures caught, both about how far ADR-0017's gate reaches: • Gating the declared-ЕИК leg on name uniqueness would discard the strongest identifier we have exactly where the name is useless — a фирма backing two ЕИК is the case ADR-0028 was written for. Now ungated. • Gating the seat leg on nameDistinctiveness would empty the rung of its purpose: rescuing a GENERIC name is what the seat is for. Only national non-uniqueness blocks it. A consequence worth stating plainly: midt-bg#279 NARROWS the family surface. The registered owner of a family stake is the relative, whose name we deliberately never store, so rung 2 („Документ") can never fire for a family link by construction — its identity can only be confirmed by something the OFFICIAL declared, the seat or the ЕИК. ADR-0032's decision is untouched (family publishes on the named surface exactly like self); it now needs the same evidence as everything else. The fixture's seat-less family case is therefore held, and a seat-carrying one was added as the positive control, without which „family published: 0" would be indistinguishable from a dead path. Fixture declarants gained patronymics. Bulgarian names are three-part by statute (ЗГР чл. 9) and rung 2 requires the full triple, so two-token fixture names were not merely unrealistic — they could never earn „Документ" and were quietly testing the wrong path. A seal is written for every link, held and withdrawn included, so the review queue can explain itself; live_status is re-derived each run and never sealed. load.mjs also emits candidate-eiks.txt — every resolved ЕИК, not just published ones — which is the crawler's closed input set.
The read path, the DTO, the card and the audit (midt-bg#279, ADR-0033). This is what makes „every shown link explains itself" true rather than a promise: a reader sees which act was read, which entry, and when — and can open the same act. THE PREDICATE HAS THREE COPIES and all three moved together. SURFACED_OWNERSHIP in related-persons.ts is the one people find; the other two are precompute.sql and refresh-slice.sql, and the second of those runs on the 6-hourly cron feeding the officials search index. Missing it would leave officials findable whose links no longer surface — a stale index that contradicts the page it links to. The card's wording is load-bearing and tested as such. „лицето е вписано като съдружник/собственик" reports what the act RECORDS; it does not say the official owns anything, because that claim comes from their own declaration and is rendered separately as „дялово участие". „Потвърдено" says the COMPANY was identified from something the official declared — nobody was found in the act — so its label must not imply anyone was, and a test asserts it never contains „вписан" or „собственик". audit.mjs's C axis was dead code the moment the loader stopped emitting B_distinctive: it re-derived name distinctiveness, a rule no longer in force. It now re-derives the rule that IS in force — a published link must carry a seal, the seal must be a publishing rung, and the tier must agree with it — plus the PII rail as an audited invariant rather than a convention: matched_fact is a closed vocabulary, so a name reaching it is a hard finding. Five new cases, including a positive control, without which all four negatives would still pass if the axes fired unconditionally. Eight test fixtures needed 0006 or evidence seals. That is not incidental: the read gate now requires a seal, so a fixture without one renders an EMPTY surface and every assertion below it passes vacuously. related-persons-sql.test.ts seals its links with a SELECT over interest_links, so a row added later is sealed automatically and cannot silently drop off the surface. Two things the browser check caught that no unit test could. The dev server was serving pre-change HTML because my local vite config pinned __SIGMA_DEPLOY_TAG__ to a constant — the worker prefixes that tag into its edge-cache key, so the key never rotated and stale HTML survived every restart. That is exactly the failure the real vite.config.ts warns about, reproduced by ignoring it. And the row's <dt> is CSS-uppercased, so the assertion had to be case-insensitive.
midt-bg#279 §9 asks for the publishing decisions to run on a schedule and for the rule to be public. ADR-0021 E10 makes the second one a libel-defence obligation, not a nicety: a reader must be able to re-derive any link. The methodology page now carries the evidence ladder VERBATIM — all six rungs, including the two that hide a link — and, more importantly, the disclosures that make it honest rather than merely stated: • the register carries NO ЕГН, so a three-name match is not proof of identity and a namesake is possible. Said in those words, not implied. • which is why the match must be all three names inside ONE registry entry; initials and Latin-script names are refused. • seats move, so a registered seat only confirms a period it predates. • erased entries are skipped everywhere. • what the evidence proves and what it does not: the register confirms the IDENTITY OF THE COMPANY; it does not certify that the official owns it — that claim is their own declaration. This is why the card says „вписано като съдружник/собственик" and never „собственик според ТР". • the monthly cadence, the entry number and lookup date on every link, and the 35-day retention with no third-party name kept. The divestment promise is corrected too: withdrawal on a later silent filing is an INFERENCE FROM SILENCE whose commonest cause is a finished mandate, so it is now reconciled against the live act before it takes effect. Cadence, deliberately on the EXISTING workflow rather than a second one. A duplicate would carry its own copy of the credential guards, the D1-target guard and the ship floor — and the copy is the one nobody exercises. A scheduled run takes the same path, targeting STAGING (production stays manual, so a prod write remains a deliberate act) and with full_crawl false, so it never touches the register: decisions are a pure function of declarations, cached deeds and contracts, which is exactly what §9 means. Registry lookups get their own monthly workflow because they are the only step with a network budget, and separating them is what keeps the decision run zero-network by construction. It ships nothing, caches the deeds between runs so a refresh stays ~400 requests rather than a full crawl, and treats exit 2 (the rate limiter) as a WARNING, not a failure — the run stopped politely and the cache is resumable, and a red X there would train everyone to ignore the one signal that means „the register asked us to stop". The uploaded artifact is the cache INDEX only; the raw deeds carry third-party names and never leave the runner. Spec §3.3's Phase-0 is closed with what was measured, including the three facts that contradicted the issue: the sustained 429, „not in the register" being an HTTP 200 with an empty body, and fetch failing where node:https succeeds. The LIA gains an addendum for the new source and the runbook two new takedown grounds — a court-annulled entry (чл. 29 ЗТРРЮЛНЦ) and a namesake match. Neither looks like a bug: the pipeline works correctly and the claim is still wrong, so neither is fixed by a rules bump.
ydimitrof
changed the title
CI caught both; local runs could not, and the reasons are worth recording because they will recur. load-ambiguous.test.mjs ran load.mjs WITHOUT setting TR_CACHE_DB, so it fell back to the repo's real scratch/tr/tr-cache.sqlite — a file that exists on my machine only because the live smoke run created it, and that exists nowhere in CI. The test was silently running against a developer artifact. It now builds its own empty cache and passes the path explicitly, like every other loader test. refresh-officials.test.ts executes the real refresh-slice.sql, which now joins interest_link_evidence, but built its schema from 0000/0001/0003 only. Locally `pnpm test` reported 6/6 because turbo REPLAYED A CACHED RESULT for @sigma/ingest — my change was to scripts/refresh-slice.sql, which is not among that package's declared inputs, so the cache key did not move. `turbo run test --force` fails the same way CI does. Fixture now applies 0006 and seals its links. The seal is derived with a SELECT over interest_links rather than written row by row, so a link added to the fixture later is sealed automatically. That matters more than it looks: the officials batch requires a publishing rung, so an unsealed fixture indexes NOBODY and every assertion below it passes vacuously. Coverage baseline ratcheted up, as check-coverage nudged: apps/web +1.33pp (the new jsdom card tests and the evidence-label tests), packages/db +0.34pp, packages/ingest +0.57pp, packages/shared +0.83pp branches.
|
Reviewed the libel-critical core against the diff —
One non-blocking hardening note (defense-in-depth on the one path where a false match = a named-person claim): that safety rests on the fields staying name+boilerplate. The name is always the pre-first-comma segment here ( Scope: this pass covers the ladder + parser (where a false publish is born). The network/cache layer ( |
DiyanaDimitrova
left a comment
DiyanaDimitrova
left a comment
There was a problem hiding this comment.
Strict review — full ADR-0033 implementation at d50a723 (grown from the docs-only PR I reviewed at a40df20 to +4677/−494, 63 files). Reviewed across four parallel dimensions — TR network/crawler/ЕИК, deed parser + evidence ladder, cacbg integration + migration 0006, and web-surface PII + workflows + tests — then personally re-verified the two headline gaps. Scripts suite 243/243 green, web typecheck clean, security scan clean (no secrets; URLs are the documented registry API; process.env spreads are test harnesses).
Verdict: 0 CRITICAL, 1 HIGH, 4 MEDIUM. The implementation is genuinely strong and thoroughly tested; the libel-critical invariants hold. One stated safeguard is absent and should land before the crawler/ship runs against production — flagging as a comment rather than blocking the design.
HIGH — the monotonicity gate (decision 6) is specified but not implemented
scripts/cacbg/audit.mjs (no rules_version reference), scripts/cacbg/load.mjs:419
rules_version is only ever written (load.mjs:419); it is never read for a comparison. Every "§8's monotonicity gate keys on this" mention (load.mjs:44, evidence.mjs:34-35, 0006_...sql:32) is a comment pointing at code that does not exist: there is no pre-wipe export, no snapshot compare, and no "previously-published link vanished under an unchanged rules_version" hard-finding anywhere in the repo. ship-related-persons.mjs has only a count floor (assertShipFloor), which cannot catch a one-for-one swap (count stays constant while a real link is silently dropped).
Why HIGH and not CRITICAL: the failure direction is safe — a missing monotonicity gate risks silently withholding a true link (a recall regression), never fabricating a false public claim. The ADR's own maxim covers it: "a recall miss is safe; an over-merge is not." And it's a decision-6 mechanism, not one of the §7 launch gates. But it is a stated part of ADR-0033, and the surface it protects becomes 3× larger — so it must land before the first production re-run/wipe, or a rules-neutral change can quietly shrink the published surface with nothing to catch it. Fix: implement the specified gate — export {link_key, rules_version} for status='published' before the wipe; after reload, for each prior published key now absent, hard-fail if rules_version is unchanged, else emit a diff.
MEDIUM
M1 — retention is documented, purge is never enforced. scripts/tr/fetch-deeds.mjs:81, cache.mjs:145. maxAgeDays defaults to null and only makes a cached row pending again (re-fetch); nothing ever unlinks the PII-bearing scratch/tr/deeds/<eik>.json or deletes rows, and there is no 35 default. Decision 5 promises "a 35-day retention … and a purge step in the same job." Local/runner-only (CI correctly excludes deeds/*.json from artifacts), so no public exposure — hence MEDIUM — but third-party PII accumulates unbounded on disk. Fix: RETENTION_DAYS = 35 default + an actual purge (unlink raw JSON + delete rows) in the job.
M2 / M3 — deed parser hardcodes single-quoted attributes. scripts/tr/deed.mjs:67 (entity split) and :76-79 (erasure strip) both match only class='…'. If the register ever emits class="…", the entity split silently collapses two people into one block — the cross-entity "franken-name" match this file exists to prevent (fullSubsetMatch("ПЕНКО КОСТАДИНОВ ФИЛИПОВ", …) → true across a boundary), and the erasure strip raises a false drift REFUSAL. Safe today (fixtures are verbatim single-quote live markup), but the module's own R7 doctrine is "refuse loudly, never guess," so this silent-mis-split is the wrong failure mode. Fix: class=['"][^'"]*record-container across all three call sites + add a double-quoted fixture.
M4 — the ЕГН rail only guards 4 of the fields written to the index. scripts/tr/cache.mjs:66. assertNoEgnShape runs over TEXT_FIELDS (4 names) but upsertDeed binds ~7 more (seatEntryDate, latestOwnEntryDate, legalFormCode, …) unchecked. Nothing leaks today (dates aren't 10-digit runs), but the guard is only as strong as a hand-maintained list, and a future 10-digit field (e.g. a raw fieldEntryNumber) would bypass it straight into the index. Fix: run the shape check over every stringified bound value, not a subset.
LOW
- Dead
nameDistinctivenessimport (load.mjs:18) — tier-era residue; confirms distinctiveness left the publish path but muddies the read. --max-age-daysflag name implies retention but only controls re-fetch (consequence of M1).deed.mjs:231registryLegalFormgates ontypeof … === 'number'; a string-typed code degrades tounknown(safe direction, but drops one of the three joint-stock signals).- Methodology page documents 5 rungs; code has 6 (
outside_trfolded into "Неизвестна") — both held, no safety impact, but not verbatim. - Missing positive-control tests: a
role:manager:CR_F_7_Lseal passing the vocabulary check; a double-quoted deed fixture.conflicts.render.test.tsx:198toContain('—')is unscoped (passes on any dash).
Verified correct (not reported — pinned by tests / re-checked)
- Network hygiene: 429 stops the run, is never retried, never cached as negative; 5xx/timeout retried with backoff; pacing ≥3s enforced (faster is un-passable); resumable; circuit breaker sized in requests (25 < ~50 observed limit) so it can't itself trip the limiter.
- UIC-equality guard refuses a leading-zero round-trip to a different company; ЕИК is TEXT end-to-end; ЕИК checksum (9- and 13-digit) matches the algorithm and is pinned against the live SQL
eik_validover 120+ values. - Libel-critical ladder: erased records skipped at a single choke point; full three-token subset (2-of-3 refused); entity-scoped match cannot span two names (for observed markup); six-rung order with declared-ЕИК before the name-gated seat leg; joint-stock bar = union of 3 signals with unknown code withholding; "strictly-before-first-declared-year" refutation is off-by-one/timezone-safe and family stakes are structurally exempt.
- PII: removed at the type level (
api-contract) so no component can leak a relative's name; cache index carries no name column;matched_factis a closed vocabulary enforced by an independent audit re-derivation. - Fail-closed & cadence: partial cache throws (belt-and-braces per-link too);
--min-links 250floor blocks a decimated surface; registry lookups monthly vs decisions daily is separated by construction; CI excludesdeeds/*.json.
Action: not blocking the design — but land the monotonicity gate (HIGH) and the purge (M1) before the crawler/ship runs against production, and fold in the deed quote-hardening (M2/M3) since the module's own doctrine demands it. Excellent, well-tested work otherwise.
|
Second pass — the parts I flagged out-of-scope last time. One MAJOR functional bug in the crawl; the rest is sound. MAJOR —
|
DiyanaDimitrova
left a comment
DiyanaDimitrova
left a comment
There was a problem hiding this comment.
Deeper strict review — usability · security · performance · maintainability · testing (at d50a723, five parallel dimensions, findings personally reproduced/verified). This complements my earlier correctness/PII pass (monotonicity-gate HIGH + retention MEDIUM still stand). New tally: 0 CRITICAL, 2 HIGH, several MEDIUM. By the strict threshold the two HIGH robustness defects are request-changes territory — but both are cheap fixes and the input source is the (semi-trusted) government registry, so I'm leaving this a comment. Fix them before the crawler runs at scale.
HIGH — verified
H1 · Polynomial ReDoS + unbounded response body → the crawler can hang on one deed. scripts/tr/deed.mjs:67,77 + scripts/tr/client.mjs (no max-body cap). The entity-split/erasure regexes are O(n²) on a <div…erasure-text-inline…>-heavy body. Reproduced against the real entityBlocks: 34KB→2.3ms, 68KB→8.8ms, 136KB→31ms, 272KB→109ms, 544KB→439ms — doubling length ≈ 4× time. Extrapolated, a multi-MB deed costs seconds→minutes, and httpsGet buffers the whole body with no size ceiling, so nothing stops a pathological (or maliciously large) deed from stalling the run. (Note: I did not reproduce the "200KB→9.2s" magnitude a first pass suggested — my numbers are milder; the scaling is the real issue, not a hang on small input.) Fix: cap the body in httpsGet (req.destroy() on overflow) — this one chokepoint also bounds H2 — and make the entity split a linear tokenizer / anchored match.
H2 · Uncaught RangeError aborts the parse (and the crawl). scripts/tr/deed.mjs:36 — String.fromCodePoint(Number(n)) on a numeric character reference. Reproduced: � → RangeError: Invalid code point 999999999999, thrown out of decodeEntities; the crawler's parse path (fetch-deeds.mjs:~219) does not wrap it, so a single malformed deed ends the run instead of being refused-and-skipped. Fix: bounds-check the codepoint (≤ 0x10FFFF, else drop the entity) and wrap the per-ЕИК parse in the existing refuse-and-continue.
MEDIUM — new (beyond the earlier pass)
- Crawl scope isn't the stated ~400 / 20 min.
load.mjscandidate set =COUNT(DISTINCT eik)over all links (published + held), with no ceiling and no projected-wall-clock log; the "~400 ЕИК" figure is an assumption the code doesn't enforce. Add--max-candidates+ a projected-runtime log. (Perf P1) - Daily job re-runs the full corpus rebuild. The ADR frames decisions as "pure, zero-network, daily"; the workflow actually re-crawls CACBG + full
load.mjsrebuild daily. Reconcile the cadence claim or measure/record the real daily wall-clock. (Perf P2) - A1 · official page overclaims "собствен дял".
conflict.official.tsx:55,62lede + callout say the page shows only the official's own stake, butOFFICIAL_SQLhas norelation='self'filter, so ADR-0032 family cards ("деклариран дял на свързано лице") render below. The card label is honest and the relative is nameless (why MEDIUM not HIGH), but the page-level framing wrongly implies the named official personally holds every stake shown. Copy-only fix: "собствен дял или дял на свързано лице". - Rung precedence is comment-only — no test goes red on a reorder.
evidence.mjs:117-204. The single place a plausible future edit produces a libel with all tests green. Add cross-rung precedence tests (document-beats-refutation; bar-beats-everything). (Testing/Maint) - Publishing set
{document,confirmed}duplicated across 4 sites (audit.mjs:44,related-persons.ts:135,conflicts.ts,0006.sql:23) and the joint-stock regex byte-duplicated (deed.mjs:222vsclassify.mjs:80) — no "keep in step" test. A rung change touches all four; one miss is a silent mislabel/leak. (Maint) - Dev seed renders an empty
/conflicts.scripts/seed.sqldoesn't seal its published links, so under the new evidence gate the local surface goes dark — the exact "passed only due to local state" class this PR just fixed, missed in the seed. Seal seeded links + add a seed-loads-non-empty test. (Testing) matched_factname-safety has no production guard — only a test whose fixtures are always clean (ПЛОВДИВ). Feed a dirty settlement and assert the exact sealed token, or enforce the vocabulary in prod. (Testing/Security)- ЕГН rail covers 4 of ~11 index fields (from earlier pass) — worth folding in here as it's the security-relevant twin of the above.
LOW
Deed re-parsed 5–7× per link (hoist the double registryLegalForm(deed) at fetch-deeds.mjs:229-230; WeakMap-memoize allFields); fetch-deeds.mjs has no --help and dumps a raw stack on bad args; no per-ЕИК progress on a ~20-min run; retention constant drifts (35 doc / 30 default / no purge); toLink silently coerces an unknown evidence_kind to document; A2 no inline съименник/"no-ЕГН" caveat at the confirmed-claim card.
Verified strengths (kept, not re-listed from the prior review)
Read-path is correctly indexed (join on evidence PK, no N+1; LINK_CONTRACTS_LIMIT=500 DoS guard); write path is a single batched transaction; workflows SHA-pin actions, permissions: contents: read, no pull_request_target, exclude deeds/*.json from artifacts, and print reasoned ::error:: on every fail-closed path; a11y cues carry text/sr-only (not color-only); the cross-entity libel guard test is mutation-hard (proves the naive match would have fired); ЕИК SQL/JS parity pin over 120+ values; no secrets, SQLi, path traversal, prototype pollution, or TLS bypass.
Recommendation: land H1 (body cap + linear split) and H2 (codepoint guard + refuse-and-continue) before the crawler runs unattended — one bad/large deed shouldn't stall or abort a 20-minute run. The rung-precedence test and the seed-seals-links test are the two highest-value maintainability/testing adds. Everything else is fast-follow.
The entity split and the erasure strip both matched class='...' only. The live register emits single quotes today and every fixture is verbatim from a real deed, so nothing is broken now — but the failure mode if that ever changes is the worst one this module has: the split stops firing, all owners in a field collapse into one token pool, and a name assembled from two different people starts matching. That is a named public claim about someone who is not there. deed.mjs's own doctrine is "refuse loudly, never guess". Silently mis-splitting is neither, so match either quote style instead. The new split test strips the <hr> separators first. With them present it passes on the <hr> rule alone and proves nothing about the quote handling — verified by watching it pass before the fix.
assertNoEgnShape refused any value matching /\d{10}/, justified in three places
by "an ЕИК is 9 or 13 digits, never 10 — so it cannot reject a legitimate
identifier". That reasoning only holds if the run is matched as a WHOLE. A
13-digit ЕИК (клон) CONTAINS ten-digit substrings, and on the fetched path
rawPath is `<eik>.json`, derived from that very ЕИК:
/\d{10}/.test("1155361790001.json") === true
upsertDeed sits past fetch-deeds' JSON.parse/assertUicEcho try-catch, so the
throw propagated and killed the whole run on the first branch office that
returned a deed — after its paced request was spent. Fail-closed for
correctness, but the crawl is the data source for the entire feature.
The existing test asserted this exact case was safe and passed anyway:
markOutsideTr sets no rawPath, and the fetched fixture used a 9-digit ЕИК, so
the one path that throws was never exercised.
Anchor the pattern, and screen by EXCLUSION rather than a hand-maintained list
of four field names — upsertDeed binds thirteen values and the next one added
would have bypassed the rail silently.
Two fields stay exempt for structural reasons, not convenience. `eik` is
already shape-validated by safeEik. `bodySha256` is 64 chars of [0-9a-f]:
measured over 200k digests, a standalone ten-digit run occurs in 7.2% of them
(18.1% unanchored), so screening it would refuse roughly one deed in fourteen
for no privacy gain. A digest is not personal data — it exists precisely so
that no deed content reaches the index.
ADR-0033 decision 5 promises the deed cache is kept "with a 35-day retention (one refresh cycle plus slack) and a purge step in the same job". Neither existed. --max-age-days defaulted to null and only made a cached row pending again — it re-REQUESTS, it never deletes — so nothing ever unlinked the raw JSON that holds co-owner and manager names and company addresses. Third-party personal data accumulated on disk unbounded, with a documented TTL that deleted nothing. Freshness and retention are now separate flags because they are separate obligations: --max-age-days decides what gets re-requested, --retention-days decides what stops being kept. Retention defaults to the ADR's 35 days rather than to "off", so the rail holds for an operator who passes neither. purgeExpired runs in `finally`, and that placement is the point: retention is an obligation about other people's data, not a reward for a clean run, so it must also happen on the paths that leave early — a 429 (exit 2), a tripped breaker, an unresolved candidate. A failed purge is loud but never masks the run's own exit code, least of all the 429 that tells the operator to back off. It is a privacy rail, not cache eviction. Under normal operation it removes nothing, because the monthly refresh rewrites each row well inside the window; what it actually catches is residue — a company that dropped out of the candidate set, or a refresh that failed — plus orphaned raw files that no read path can reach and that are therefore pure retained personal data. purgeExpired was written before its tests, so RED was established afterwards by mutation: stubbing the expiry query kills 4 tests, disabling the orphan sweep kills 2, and moving the call out of `finally` kills the 429 case.
…fies rules_version was only ever written (load.mjs) and never read for a comparison. Every "§8's monotonicity gate keys on this" comment — in load.mjs, evidence.mjs and migration 0006 — pointed at code that did not exist: no pre-wipe export, no snapshot compare, no finding. ship-related-persons.mjs's count floor cannot stand in for it. It compares a COUNT, so a one-for-one swap — one true published link silently dropped, one gained — leaves it perfectly quiet. Two halves: load.mjs exports the currently published set with each link's rules_version immediately before it drops the CACBG tables. The rebuild is total, so that set exists only in this instant. On a first run the tables are absent and the export is an empty array — written rather than skipped, so a missing file keeps meaning "the loader never ran", not "nothing was published". Any error other than "no such table" propagates; swallowing it would turn a broken export into a permanently silent gate. audit.mjs compares against it. A link published last run and not published now under an UNCHANGED rules_version is a hard finding: nothing licensed the removal. Under a changed version it is an intentional event and degrades to a printed diff, so a rules bump still shows a human which named claims it withdrew. Held and withdrawn links are excluded from the export — they were never a public claim, so their absence is not a regression. The failure direction this protects against is recall, not libel: a missing gate risks withholding a true link, never fabricating a false one. But the surface it guards is about to triple, so a rules-neutral change must not be able to shrink it silently. Also drops the dead nameDistinctiveness import from load.mjs (referenced only in a comment since the ladder replaced it) and replaces it with a note on why it stays in classify.mjs.
Superseded ADRs gain a "Superseded by" back-pointer; amended ones gained nothing. A reader of ADR-0007 — the foundational certainty bar — had no way to discover that ADR-0033 carved an explicit exception to its decisions 2 and 3. Adds an "Amended by" line to 0007, 0010, 0021 and 0028, each naming which decisions moved. A header breadcrumb, so the convention that accepted ADRs are not rewritten still holds.
expect(text()).toContain('—') ran against the whole page, where the value and
date cells render em-dashes of their own. It would have survived the source
branch being deleted outright. Scope it to the card whose sourceUrl is null and
assert the declaration anchor is absent from it.
It was parked as "resolve with the author". It does not need the author — the issue's own numbers determine it. §5 gives each rung a control count: 4 barred + 281 document + 102 seat + 3 ЕИК + 21 refuted + 156 unknown = 567, plus the 4 links whose ЕИК is not in the register at all (§5 scopes the ladder to links "с изтеглен акт", so those reach no rung) = 571. §10's identity row is 281/102/156/21/4/4 = 568. The difference is exactly the "3 по ЕИК". The labels give the cause away. The identity row calls its second bucket "седалище" — one leg of rung 3 — while the evidence row two lines below calls the same rung "потвърдено". Rung 3 has two legs; the row tallied one and presented the result as a partition of the resolved set. A histogram that dropped a category, not a disputed measurement. The stated total 568 equals that row's sum exactly, so it is derived from the histogram rather than measured independently. Both therefore move together: the bucket becomes "потвърдено: 105", the resolved total becomes 571. F8 measures against those and reports disagreement rather than adopting whichever reading makes the reconciliation pass. The identified-vs-surfaced gap moves with it — 57 links on the corrected reading, 54 on the row as written — so that figure independently distinguishes the two, which is worth more than the correction itself. Also records what is NOT a discrepancy, so nobody "fixes" it: "извън ТР" is 4 in §10 and 3 in §3/§11 because the units differ — 3 ЕИК not in the register, carrying 4 links between them. §10's row counts links.
|
Re-checked the commits addressing my last two findings — both correctly fixed, and the ЕГН one is fixed better than I suggested. MAJOR (13-digit ЕИК crawl abort) — fixed ( Libel-critical bonus — The decision core + plumbing were already sound; with these two the crawl path is in good shape. The web render ( |
nikimilenkov
left a comment
nikimilenkov
left a comment
There was a problem hiding this comment.
Strict re-review at f4d73dc — verified every fix commit against @DiyanaDimitrova's and @lyubomir-bozhinov's findings, then covered the surfaces nobody had reviewed yet (the web render path, and the two workflows as a system). Method: local checkout of HEAD, all four test suites green (53 + 205 + 361 + 488), and every finding below was reproduced at HEAD or killed — the numbers come from real executions, not from reading the diff. My account has Triage rights, so this review is advisory; verdict at the end.
1. Fixes from the previous rounds — verified one by one
| Finding | Commit | Status at HEAD |
|---|---|---|
| 13-digit ЕИК aborts the crawl (MAJOR, @lyubomir-bozhinov) | 299400e |
✅ anchored (?<!\d)\d{10}(?!\d) + exclusion-based screening |
| Single-quoted-only parser attributes (M2/M3) | 7a424bd |
✅ ['"] at all three sites + fixture |
| ЕГН rail over all index fields (M4) | 299400e |
✅ |
| Monotonicity gate (HIGH) | 2327e62 |
|
| 35-day purge (M1) | 7f2e174 |
purgeExpired: rows+files+orphans), but the Actions cache sidesteps it — B3 |
| Back-pointers on the amended ADRs | 76ed3b2 |
✅ |
| §5/§10 control-number gap | f4d73dc |
✅ documented as a separate change |
Dead nameDistinctiveness import |
2327e62 |
✅ |
CI glob for scripts/tr/*.test.mjs |
b364031 |
✅ (without it, all 205 tests were decorative) |
H1 and H2 from @DiyanaDimitrova's third review are addressed by no commit and remain open at HEAD — reproduced in B4/B5 below.
2. Blockers
B1 · The two workflows cannot bootstrap each other — the cadence is dead on arrival. The hydrate step (related-persons-data.yml:166) does rm -f backfill.sqlite and exports only the 4 EOP tables from D1; nothing in this workflow restores scratch/tr → every daily run dies at REFUSE TO LOAD: no Trade Register cache (load.mjs:489). The monthly refresh in turn gates on scratch/cacbg/staging/candidate-eiks.txt (related-persons-tr-refresh.yml:81), which is written by load.mjs on the other workflow's runner and never persisted across workflows → a permanently green no-op ("Nothing to refresh", exit 0). No sequence of GitHub-side runs can ever bring the feature up: the daily job waits on a deed cache the monthly job will never produce, and vice versa. The failure direction is safe (nothing false ships — nothing ships), but the §9 cadence this PR claims to deliver does not exist. Fix: the candidate list and the TR index must travel between the workflows (cache key/artifact, or derive from D1); the raw deeds belong in R2 with a lifecycle rule, not in the Actions cache (see B3 — the two resolve together).
B2 · The monotonicity gate is inert exactly where the wipe happens. load.mjs exports the prior published set from the work DB — but CI recreates the work DB every run without interest_links, so the export hits "no such table" and writes []: the branch designed for "the legitimate first run" is every run. scratch/cacbg/staging/published-snapshot.json is not persisted anywhere either. The audit compares against an empty set every time and prints "0 published last run" — the gate only functions on a laptop with a persistent work DB, i.e. never over the surface it protects. Fix: compare against the shipped surface — the hydrate step already exports 4 tables from D1; add interest_links + interest_link_evidence and the prior becomes real with no new machinery.
B3 · Personal data outlives the purge in the Actions cache. related-persons-tr-refresh.yml:68 caches all of scratch/tr, including deeds/*.json — the names of third-party co-owners and managers, precisely the data ADR-0033 decision 5 promises a 35-day retention for. The artifact step (:111–118) correctly uploads only the index; the cache step makes no such distinction. purgeExpired deletes on the runner, while old cache entries live on — LRU/7-days-unused eviction is a capacity policy, not retention, and the restore-keys: prefix keeps the chain alive. Fix: cache only tr-cache.sqlite; if the raw deeds must survive across runs (B1!), their home is R2 with a lifecycle rule (the ADR-0006 model).
B4 · H1 stands: no response-body cap + a quadratic regex — reproduced at HEAD. httpsGet (client.mjs:73–90) buffers with no ceiling; the timeout bounds stall, not volume. The erasure-strip regex is quadratic on a chunk with no split points: 64K→8ms, 128K→26ms, 256K→98ms, 512K→387ms (×4 per doubling; extrapolated, a 5MB field ≈ 37s of blocked event loop). The two halves compose: the quadratic path is reachable only because the cap is missing. (Precision: the record-container fragment case is linear — each fragment is its own split point; the quadratic case is erasure-text-inline with no split points, consistent with Diyana's own correction on magnitude.) Fix: byte counter + req.destroy() in httpsGet — one chokepoint bounds both; make the split linear.
B5 · H2 stands: one malformed/hostile entity kills the whole crawl — reproduced at HEAD. � → RangeError: Invalid code point out of deed.mjs:36. The per-ЕИК try/catch covers only JSON.parse + assertUicEcho; registrySeat (:234), registryLegalForm ×2 (:242–243) and latestOwnershipEntryDate (:246) sit outside it → the exception escapes run() and kills the process (exit 1). Same exposure in load.mjs:535 (JSON.parse of the cached raw → evidenceVerdict with no per-link guard). Fix: bounds-check the codepoint (≤ 0x10FFFF, else drop the entity) + widen the refuse-and-continue block to cover the HTML parsing.
B6 · toLink fails toward the STRONGER claim. related-persons.ts:218: any evidence_kind ≠ 'confirmed' — including NULL, 'refuted', a future rung, a typo — is coerced to 'document' and renders as „лицето е вписано като съдружник/собственик": a fabricated registry claim about a named person. Unreachable today (PK + EXISTS), but the comment at :216 explicitly embraces "half-migrated environment rendering" — i.e. the failure direction was chosen wrong at the one place where a mistake = libel. The irony: the LEFT JOIN at :180 was deliberately kept non-inner to "surface a contradiction" — and the coercion then converts the contradiction into the strongest label. Fix: filter the row out or throw; never coerce upward.
B7 · The gate protecting all of this has zero red tests — proven by mutation. I deleted the entire EXISTS seal gate from SURFACED_OWNERSHIP and ran the full db suite: 361/361 green. Not one fixture in related-persons-sql.test.ts seeds a withholding rung (refuted/unknown/bar_joint_stock/outside_tr) or asserts absence from the four queries — the only SQL layer between a withheld link and a public named claim is entirely unexercised. Fix: one negative fixture per rung + absence assertions across LEADERBOARD/OFFICIAL/COMPANY/LINK_CONTRACTS. (Adjacent: rung-precedence coverage is partial — rung 1 has 2 discriminating tests, but swapping rungs 2↔3 leaves everything green, including the test at evidence.test.mjs:211, whose fixtures cannot reach rung 3 at all.)
3. Open items from the previous rounds (status at HEAD)
| Finding | Status |
|---|---|
| A1 "own stake" copy | Open — also on conflict.company.tsx:62, not just the official page (both files are outside this diff, but this PR made the per-card copy precise while the lede above it still overclaims) |
| Seed has no seals | Open — seed.sql writes no interest_link_evidence → /conflicts renders empty on a fresh dev DB: this PR's own fixture (ea00781) died under this PR's own gate (4226ae3); no test executes the seed |
--max-candidates / runtime projection |
Open — "~400" lives only in workflow prose |
matched_fact guard |
Partial — audit-time (CI), not write-time; and the regex admits a name: seat:ИВАН ПЕТРОВ ГЕОРГИЕВ passes the audit (reproduced) — bounding seat: to 1–2 tokens closes it |
| Duplicated sets | Partial — {document,confirmed} in 4 places with no keep-in-step test; JOINT_SUFFIX/JOINT_STOCK byte-duplicated, and the linking comment at deed.mjs:221 ("КДА … absent from classify.mjs today") is now factually wrong after 5f64f5c |
| Methodology page: 5 vs 6 rungs | Open (LOW) |
split(',')[0] name isolation (@lyubomir-bozhinov) |
Not adopted — remains a false-positive widener on the most sensitive path |
--help, double registryLegalForm, 5–6 HTML parses per link |
Open (LOW) |
4. New minor items (not blockers)
load.mjs:533—row.rawPathis joined verbatim, bypassing thesafeEikrail thatpurgeExpiredcorrectly uses; the index arrives via the Actions cache (an untrusted channel given B3). Re-derive viadeedPath(eik).api-contract:registryLookupDate: stringis non-null by type, buttoLinkfeeds it''— the type promises more than the code delivers.
5. Verified and holding (not repeating what colleagues already confirmed)
SSRF allow-list before the first packet, exact-host + https-only, redirects not followed; all 17 actions SHA-pinned, permissions: contents: read, no untrusted ${{ }} in any run:; zero new dependencies (node built-ins only); migration 0006 idempotent, no DROP/DELETE; SQL parameterized throughout; no relative's name can reach the DTO (matched_fact is never SELECTed; cut at the type level in api-contract); XSS clean (JSX escaping, no dangerouslySetInnerHTML); safeEik structurally unbypassable on the write path (the sole exception is :533 above); 429-stop/pacing/resume — confirmed and green locally.
Verdict
Request changes (advisory — final approval belongs to a maintainer): B1–B7 before merge. B1–B3 are operational/privacy (the cadence doesn't run; the flagship gate is inert; PII outlives retention), B4–B7 protect the libel-critical surface (the two still-open HIGHs from the previous review + the read path's failure direction + the unexercised SQL gate). None of this undermines the design: ADR-0033 and the decision core are sound, and the fixes from the previous rounds are real and verified. The problems live in the seams — the CI infrastructure as a system, and the failure direction at the one place where a mistake carries a name.
| # If the corpus cache has been evicted, extract yields nothing, the ship floor refuses, and the run | ||
| # fails loudly rather than wiping the surface. | ||
| schedule: | ||
| - cron: '0 3 * * *' |
There was a problem hiding this comment.
B1+B2: This cron can never go green: the hydrate step recreates the work DB with only the 4 EOP tables, and nothing in this workflow restores scratch/tr → load.mjs dies at REFUSE TO LOAD: no Trade Register cache on every daily run. At the same time the monotonicity pre-wipe export reads interest_links from that same recreated DB → "no such table" → snapshot [] every run, i.e. the gate from 2327e62 protects nothing in CI. Both resolve in one place: export interest_links + interest_link_evidence too in the hydrate step (it already runs wrangler d1 export for 4 tables), and restore the TR index from cache/artifact/R2.
There was a problem hiding this comment.
Потвърдено и по двете точки, и наистина се решават на едно място — макар и не съвсем на онова, което предлагате. Поправено в ae661ac (+ 024c26f).
B1. Работният поток за обхождане е премахнат; обхождането тече вътре в това задание. Възстановяване на scratch/tr от кеш/артефакт не приех — това е самото нарушение от B3, а суровите актове са и точно данните, които evidenceVerdict изисква (сравнява името на декларатора със самия текст на регистъра). Затова: без граница между двете. --emit-candidates (024c26f) зарежда списъка от нула на изпълнител без кеш, обхождането го използва, load.mjs чете scratch/tr от същия изпълнител, а суровите актове се трият изрично.
B2. Точно така — стъпката за захранване правеше rm -f backfill.sqlite и изнасяше само четирите EOP таблици, така че предварителният износ четеше „no such table", снимката беше [] при всеки ход, и гейтът от 2327e62 не пазеше нищо. Сега се изнасят и interest_links + interest_link_evidence — не за резолвера (load.mjs ги пресъздава), а именно като вход на гейта. Изнасят се отделно и липсата им се търпи: при първи ход 0003/0006 още не са приложени.
Едно нещо, което открих при поправката и което си струва да се отбележи, защото щеше да върне B2 през задния вход: зареждащият ход не бива да пипа истинската работна база. Стигането до списъка с кандидати минава през пресъздаване на таблиците, което изхвърля interest_links, а ходът никога не публикува — значи щеше да остави таблицата празна. Следващият истински ход щеше да прочете тази празна таблица като предишно публикувано състояние, да запише празна снимка, и гейтът, чиято единствена работа е да забележи изчезнало публикувано твърдение, щеше да минава безусловно завинаги. Затова --emit-candidates работи върху захвърляемо копие. Проверено с мутация: насочен към истинската база, проваля и двата теста.
Последица за графика, казана честно: решението не може да се преизчисли без актовете, актовете не преживяват изпълнителя, значи всеки ход, който решава, трябва и да обхожда — а ежедневно решение би значело ~400 заявки дневно към чужд регистър. Графикът стана месечен, не ежедневен, както го описват §9 и ADR-0033. Отбелязано е в коментар в самия работен поток. Връщането към ежедневно иска обхождането да издава присъди по (връзка, ЕИК), за да пресичат границата само булеви стойности — промяна в дизайна, нарочно невключена тук.
| id: cache | ||
| uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.0.2 | ||
| with: | ||
| path: scratch/tr |
There was a problem hiding this comment.
B3 (privacy): path: scratch/tr also caches deeds/*.json — third-party co-owner/manager names, exactly the data decision 5 promises a 35-day retention for. The artifact step below correctly uploads only the index; this step makes no such distinction. purgeExpired deletes on the runner, but old cache entries live on (LRU/7-days-unused eviction is a capacity policy, not retention, and the restore-keys: prefix keeps the chain alive). Cache only scratch/tr/tr-cache.sqlite; if the raw deeds must survive across runs — R2 with a lifecycle rule (the ADR-0006 model).
There was a problem hiding this comment.
Потвърдено, и то беше нарушение на собственото обещание на ADR-0033 решение 5. Поправено в ae661ac.
Права сте и за причината: purgeExpired тече на изпълнителя, но по-старите записи в кеша живеят в хранилището на GitHub по веригата на restore-keys, а изгонването там е политика за капацитет, не задържане.
Не приех обаче „кеширай само индекса" — това чупи решаващия ход: load.mjs чете суровия JSON, защото evidenceVerdict се нуждае от имената за подмножественото съвпадение, а индексът нарочно не пази нито едно. Тоест суровите актове наистина трябва да пресекат границата, което е и ограничението, в което опира B1.
Затова изборът е първата от вашите две алтернативи в друга форма: никаква граница. Обхождане и решение в едно задание, кешира се само scratch/cacbg/raw, scratch/tr не се запазва никога, а суровите актове се трият изрично в if: always() стъпка — за да не може по-късно добавено качване на артефакт да ги изнесе. R2 с правило за жизнен цикъл остава разумният вариант, ако някога се наложи да преживяват ход; днес не се налага.
Последица, която да е казана честно: решението не може да се преизчисли без актовете, а актовете не преживяват изпълнителя — значи всеки ход, който решава, трябва и да обхожда. Ежедневно решение би значело ~400 заявки дневно към чужд регистър. Затова графикът стана месечен, а не ежедневен, както го описват §9 и ADR-0033. Връщането към ежедневно иска обхождането да издава присъди по (връзка, ЕИК), за да пресичат границата само булеви стойности — това е промяна в дизайна и нарочно не е вкарана тук.
| - name: Check for a candidate list | ||
| id: candidates | ||
| run: | | ||
| if [ -s scratch/cacbg/staging/candidate-eiks.txt ]; then |
There was a problem hiding this comment.
B1 (the other half of the deadlock): candidate-eiks.txt is written by load.mjs on the other workflow's runner and is never persisted — this check is false on every run, forever, and the job is a permanently green no-op. The daily job waits on a deed cache this workflow will never produce; this one waits on a candidate file the daily job will never deliver. Persist the list (cache/artifact) or derive the candidates directly from D1.
There was a problem hiding this comment.
Потвърдено — и двете половини на блокировката бяха точно както ги описвате. Поправено в ae661ac (+ 024c26f).
Този работен поток е премахнат. Обхождането на регистъра вече тече вътре в заданието на решенията.
Не може да се поправи с пренасяне на файлове между ходовете, защото решението се нуждае от суровите актове (evidenceVerdict сравнява името на декларатора със самия текст на регистъра — точно данните, които индексът отказва да съхранява), а те не бива да се запазват между ходове. Затова: едно задание, --emit-candidates (024c26f) го зарежда от нула, и суровите актове се трият изрично накрая.
--emit-candidates работи върху захвърляемо копие на работната база, и това е коректност, а не спретнатост — вж. отговора за B2.
| { headers: { accept: 'application/json', 'user-agent': TR_USER_AGENT } }, | ||
| (res) => { | ||
| const chunks = []; | ||
| res.on('data', (c) => chunks.push(c)); |
There was a problem hiding this comment.
B4 (H1 from @DiyanaDimitrova's review, still open): unbounded buffering — the timeout at :87 bounds stall, not volume. It composes with the quadratic erasure regex in deed.mjs (reproduced: 64K→8ms, 128K→26ms, 256K→98ms, 512K→387ms — ×4 per doubling): the quadratic path is reachable only because the cap is missing. A byte counter + req.destroy() here bounds both with one change.
There was a problem hiding this comment.
Потвърдено, но поправката е недостатъчна както е формулирана — направено е и второто в 26333d8.
Границата по байтове я има: 8 MB, ~240× измерените 34 KB на реален акт, с req.destroy() вместо само отказ на промиса (иначе сокетът продължава да източва). Изнесено като collectBody, за да е тестваемо без TLS.
Само таванът обаче не ограничава квадратичния път. Възпроизведох го: 34K→3.3ms, 68K→13.6ms, 136K→53.8ms, 272K→240ms, 1M→4.0s — ×4 на удвояване. При 8 MB това е минути процесорно време на акт, управлявани отдалечено. Затова ограничих и мързеливия квантор до 2000 знака (~80× реалното известие): същите входове дават 5.3 / 10.3 / 24.8 / 41.6 / 187ms — линейно.
Оградата не може да съживи собственик: erased се решава отделно от ERASED_MARKER, така че прекалено дълго известие пак маркира блока като заличен и liveFields пак го изхвърля. Има тест и за това.
| function decodeEntities(s) { | ||
| return String(s) | ||
| .replace(/&(?:quot|apos|amp|lt|gt|nbsp);/g, (m) => ENTITIES[m]) | ||
| .replace(/&#(\d+);/g, (_, n) => String.fromCodePoint(Number(n))) |
There was a problem hiding this comment.
B5 (H2, still open, reproduced at HEAD): � → RangeError: Invalid code point — thrown out of both entityBlocks and registrySeat. Guard: n <= 0x10FFFF ? String.fromCodePoint(n) : '' (and the same for the hex line below).
There was a problem hiding this comment.
Потвърдено и поправено в 2b06b76.
Възпроизведено: entityBlocks("…�…") хвърля RangeError: Invalid code point. И двата реда — десетичният и шестнадесетичният — минават през обща проверка, а извън диапазона се декодира до нищо: останалата част от записа продължава да се разбира. Извън U+10FFFF не е знак и не може да е част от име, така че разликата е между загубен глиф и загубено обхождане.
Добавени 4 теста, включително контрола с валиден П — за да е ясно, че оградата ограничава, а не изключва декодера.
| -- one, so a fixture without seals renders an EMPTY surface and every assertion below passes vacuously. | ||
| -- Derived from interest_links itself, so a row added to the fixture later is sealed automatically and | ||
| -- cannot silently drop off the surface. | ||
| INSERT INTO interest_link_evidence (link_key, evidence_kind, registry_role, matched_fact, lookup_date, rules_version, live_status) |
There was a problem hiding this comment.
B7 (proven by mutation): I deleted the entire EXISTS seal gate from SURFACED_OWNERSHIP and ran the full db suite — 361/361 green. Every fixture here seeds only evidence_kind='document'; not one seeds a withholding rung or asserts absence. The only SQL layer between a withheld link and a public named claim is entirely unexercised. One negative fixture per rung (refuted/unknown/bar_joint_stock/outside_tr) + absence assertions across the four queries.
There was a problem hiding this comment.
Потвърдено — възпроизведох същата мутация независимо, преди да пиша каквото и да е: изтрих целия EXISTS от SURFACED_OWNERSHIP и db наборът остана 361/361 зелен. Това беше най-тежката находка от четирите прегледа.
Поправено в 95d8c51. Добавени са всички реални задържащи стъпала, които evidence.mjs може да издаде — refuted, bar_joint_stock, unknown, outside_tr — плюс връзка без никакъв печат (състоянието, което непълно зареждане реално оставя). Всички върху един и същ изпълнител и иначе еднакви, така че единствената разлика е печатът. Твърди се отсъствие и в четирите публични заявки: класация, страница на лицето, страница на дружеството и разгъването на договорите.
Носещата част е положителната контрола с confirmed. Без нея отсъстващ ред може да отсъства по несвързана причина и всяко твърдение минава напразно — което е точно режимът на отказ, който поправяме, а не форма за възпроизвеждане.
Проверено с мутация: без оградата падат 6 от новите тестове, а положителната контрола продължава да минава.
| -- fixture cannot teach a wrong ЕИК shape to anyone reading it. | ||
| -- | ||
| -- The rows are shaped by what the read gate actually requires (packages/db/src/queries/related-persons.ts): | ||
| -- • status='published' AND interest_class IN (private_ownership, family_ownership) — SURFACED_OWNERSHIP |
There was a problem hiding this comment.
The comment lists SURFACED_OWNERSHIP, but since 4226ae3 the gate also requires an EXISTS seal in interest_link_evidence — and the seed writes none. Result: /conflicts renders empty on a fresh dev DB — this PR's own fixture (ea00781, "without it /conflicts renders empty") died under this PR's own gate. Add document/confirmed seals to the published links + a test that a seeded DB yields a non-empty surface.
There was a problem hiding this comment.
Потвърдено и поправено в 2d3c064 — и да, това е нанесено от самия този PR: grep -c interest_link_evidence scripts/seed.sql даваше 0.
Всяка засята връзка вече носи печата на стъпалото, което представя — document за собствения дял, confirmed за връзката на близкото лице (доказана по седалище), bar_joint_stock за задържаното АД — така че локалната повърхност показва същите три изхода като производствената и запазва отрицателния си случай.
Добавен е и тестът, който искахте, но малко по-широк: пуска истинските миграции, истинския seed.sql и истинските изнесени заявки. Засятата база е обещание за прочитащата ограда и нищо не го проверяваше — оттук нататък промяна в правилото за публикуване, която seed не догони, пада тук, а не в нечий браузър след дни.
Тестът мина RED както трябва (празна класация) преди поправката.
| // The only two evidence rungs that publish (ADR-0033 decision 1). Everything else withholds. | ||
| const PUBLISHING_EVIDENCE = new Set(['document', 'confirmed']); | ||
| // The closed vocabulary for a sealed matched_fact. Anything else is a potential name leak. | ||
| const MATCHED_FACT = /^(?:seat:[\p{Lu} -]+|role:(?:owner|manager):CR_F_\d+[a-z]?_L|eik)$/u; |
There was a problem hiding this comment.
The rail admits exactly what it exists to catch: the seat: alternative accepts any number of tokens, so seat:ИВАН ПЕТРОВ ГЕОРГИЕВ passes the audit (reproduced). The three-name form is exactly 3 tokens; a settlement is 1–2. Bound it to 1–2 tokens (or validate against the settlement list the pipeline already has) — and consider the same check at write time (load.mjs:757), not only post-hoc.
There was a problem hiding this comment.
Потвърдено и поправено в df76ae7. Възпроизведох го — seat:ИВАН ПЕТРОВ ГЕОРГИЕВ минаваше оградата.
Оградата допускаше точно това, за което съществува: seat: е законен префикс, а образецът за населено място беше неограничен, така че пълно тричленно българско име (ЗГР чл. 9) минаваше, облечено в разрешен префикс — и това е стойността, която погрешно разделяне на полето за седалище реално би произвело.
Ограничено до 1–2 лексеми: населено място е една или две („СОФИЯ", „ВЕЛИКО ТЪРНОВО", „ГЕНЕРАЛ ТОШЕВО"), а тричленното име е точно три. Границата е нарочно стегната — по-рядко тричленно седалище спира хода за човек, вместо да публикува, което е правилната посока за ограда, чийто режим на отказ е име в сервирана колона.
И вашата скоба за писането: правилото се премести в evidence.mjs, до кода, който пише matched_fact, и се прилага и при запис. Одитът тече след като целият домейн е построен — дотогава името вече е в таблица. Едно определение, за да не се разминат гейтът и писателят.
| if (!row) entry = { deed: null, outsideTr: false, missing: true }; | ||
| else if (row.status === 'outside_tr') entry = { deed: null, outsideTr: true, missing: false }; | ||
| else { | ||
| const file = path.join(TR_RAW_DIR, row.rawPath ?? `${eik}.json`); |
There was a problem hiding this comment.
row.rawPath is joined verbatim — the only read that bypasses the safeEik rail (purgeExpired in cache.mjs:239 correctly re-derives). The index arrives via the Actions cache — an untrusted channel if B3 is resolved by caching the index. Use deedPath(eik) instead of the stored string, and/or shape-check rawPath in upsertDeed.
There was a problem hiding this comment.
Потвърдено и поправено в 436dbc2.
Пътят вече се извежда наново от ЕИК през deedPath/safeEik, никога от съхранения низ — точно както purgeExpired вече правеше. Това беше единственият прочит, който не минаваше през оградата. deedPath освен това хвърля при неправилен ЕИК, вместо тихо да прочете акта на друго дружество (R8).
Бележка по каналa: индексът вече изобщо не пътува между ходове — вж. отговора за ae661ac. Но обосновката не зависи от това; съхранен път, залепен за коренова директория, е обхождане на пътища независимо откъде идва индексът.
| // The фирма's legal form is its SUFFIX under ЗТРРЮЛНЦ, and the deed envelope's `fullName` carries it | ||
| // („ПИМК" ООД) while CR_F_2_L is bare („ПИМК"). So the bar has a second, independent signal at zero | ||
| // extra cost. КДА is included here and is absent from classify.mjs's JOINT_STOCK/FORM_TOKENS today. | ||
| const JOINT_SUFFIX = /(?:^|[\s"„“”«»])(АД|ЕАД|АДСИЦ|КДА)[\s"„“”«»]*$/u; |
There was a problem hiding this comment.
Byte-for-byte duplicate of JOINT_STOCK in classify.mjs:80 with no keep-in-step test — and the comment at :221 ("КДА … is absent from classify.mjs's JOINT_STOCK/FORM_TOKENS today") is now factually wrong: 5f64f5c added КДА there too. The exact drift this duplication risks has already happened — in the comment. Shared export, or a test asserting the two regexes are identical.
There was a problem hiding this comment.
Прието — и коментарът наистина беше просто невярен: 5f64f5c е добавил КДА в classify.mjs, а бележката тук продължаваше да твърди, че липсва. Разминаването, за което предупреждавате, вече се беше случило.
Поправено в 436dbc2, с втория вариант, който предлагате, а не с първия: общ износ не става тук. Посоката на зависимост е cacbg → tr (load.mjs внася този модул), така че внасяне обратно от scripts/cacbg/ затваря цикъл между двете директории — същата причина, поради която personTokens е близнак на holderTokens, а не внос.
Затова: тест, който закрепва двата образеца байт за байт (source + flags) и поведенчески върху формите, за които съществува преградата. Проверено с мутация — премахването на КДА от която и да е от двете страни проваля теста. Проза „дръжте ги в синхрон" не държи нищо в синхрон; това го държи.
…dt-bg#297) Правилото е на един ред в AGENTS.md и в шаблона за PR. Отделна проверка за него не се изплаща: искаше пълна история при всеки PR в задължителната работа `check`, добавяше начин тя да падне по причина, нямаща общо с прегледания код, и щеше да блокира вече отворен PR на сътрудник заради трейлър, който се маха с един ред в squash съобщението.
The R10 temporal guard read `firstDeclaredYear == null` as "covers the period" and short-circuited the whole disjunction. `load.mjs` passes null whenever no history row carried a parseable year, so every such link reached the seat rung with no temporal check at all — publishing „Потвърдено" on a settlement match alone, which is precisely the "company relocated INTO the declared settlement afterwards" case the guard exists to block. That made the weakest rung the only one without a temporal test, on exactly the links where we know least. Rung 4's refutation leg right below already refuses to run without a year. An unknown year now FAILS the guard instead of skipping it. The undated-seat leg stays: a seat with no entry date is the ordinary shape for a company that never moved, and it is still checkable because a known year sits on the other side. Three tests: both null-year shapes (dated and undated seat) held, plus a positive control proving a known year with an undated seat still confirms — the fix is a bound, not a blanket. The methodology page asserted this rule already; it now states the precondition it relied on.
…ctions
ADR-0033 decision 6 licenses removal by a rules bump "or a correction of wrong input", and
names the court-annulled entry as wired to the ADR-0031 suppression path. Neither could get
through the gate, and they failed in opposite directions:
- A suppression flips status published → suppressed with rules_version untouched, so the
link leaves the published set and the gate hard-fails the one takedown the ADR points at.
- Correcting the input UNBUILDS the link, so a suppression on it matches nothing and
load.mjs's B3 unused-entry rail fails the build instead.
So the only escape was bumping rules_version, which relabels every other link.
The audit now reads each vanished link's current status: `suppressed` is a declared removal.
For a corrected input there is a second fingerprinted list, link-corrections.jsonl, mirroring
ADR-0031 for the same reason — `pid|eik` in git records which named official was tied to which
company, permanently. load.mjs flags matching keys in the pre-wipe snapshot; the audit reads
the flag. Both grounds are still PRINTED, now with the ground that licensed them, since
"removed" alone flattens an annulment, a corrected parse and a rules bump into one line.
A correction that matches nothing fails the build, mirroring B3 and for a sharper reason: a
stale acknowledgement would pre-clear a future disappearance of that same link — the exact
regression the gate exists to catch — with nobody having decided it. Acknowledgements are
one-shot; the runbook says so and says not to use the list on a removal you cannot explain.
Six tests, including the mutation control: an unacknowledged, unsuppressed drop still
hard-fails, so a gate that accepted everything would not pass the two new cases.
Also lands the snapshot's atomic write (tmp + rename, as the raw deeds already do). The audit
deliberately does not swallow a parse failure there, so a torn write wedged every subsequent
audit until a human cleared the file by hand.
…e and card claims Four independent fixes, all from the deep-pass reviews still open at 9555fbf. related-persons-data.yml — the environment guard read `${{ inputs.environment }}` inline in two `run:` blocks. `type: choice` binds the UI only; a POST to .../dispatches can pass any string, which an inline expression pastes into the shell before `[` evaluates it. Worse, that same value decides whether the production guard fires, so a crafted one could inject AND walk past the guard meant to trip on it. Both now read $SIGMA_SHIP_ENV, already bound one block up, where the value is data rather than code. (deploy.yml:49 has the same shape and is untouched — it belongs to another change.) cache.mjs — the orphan sweep called unlinkSync unguarded, unlike the expired loop above it. It runs AFTER the DB DELETE commits, so a file that vanished between readdir and unlink aborts a half-finished purge: rows gone, files still on disk, run reported as failed, and no way to tell "already gone" from "an orphan still holding third-party names". ENOENT is now tolerated and uncounted; every other error still surfaces, because a purge that cannot delete has left PII behind and reporting success is the failure it exists to prevent. `unlink` is injectable so the race is tested rather than described. conflicts.render.test.tsx — the family fixture inherited evidenceKind 'document' + registryRole 'owner' from the self-link factory, a row the query cannot produce: findPerson searches for the OFFICIAL, and a relative's stake is registered to the relative, so only a seat/ЕИК confirmation is reachable. It now mirrors production, and a new test pins what the old shape hid — a family card must never render „вписано като съдружник/собственик", which would assert the named official is recorded as an owner, on the one card designed to keep the stakeholder anonymous. related-persons.ts — `lookup_date ?? ''` was an unreachable branch (NOT NULL in 0006, and the row only arrives through the seal filter) that would have shipped an empty string as a date if it ever ran. Narrowed like evidenceKind above it. Not taken: the orphan sweep's `known` set was flagged for using raw `r.eik` while the expired loop uses `safeEik(row.eik)`. safeEik VALIDATES rather than normalizes — it throws on a bad shape — so for any storable ЕИК the two are the same string, and a non-canonical one makes the deed unreadable through readDeed, which means the file really is unreachable PII and deleting it is what the privacy rail should do.
Merging the crawl into the decision job forced the schedule from daily to monthly, which
contradicts ADR-0033's own consequence line ("decisions daily, registry lookups monthly").
That line is now false, and the truth lived only in a workflow comment — the one place a
reader looking for the decision would not go, and nowhere anyone could argue with it.
ADR-0034 records it properly: the two constraints that collide (raw deeds must not outlive
the runner, because an Actions cache entry sits under a restore-keys chain the 35-day
retention cannot reach; and the decision cannot be made without them, because the strongest
rung compares the declarant's name against the deed text the index deliberately refuses to
store), and both rejected alternatives — persisting the deeds behind someone else's eviction
policy, and having the crawl emit per-(link, ЕИК) verdicts so only booleans cross the
boundary. The second is the right way back to a daily decision and is named as such rather
than left implicit.
ADR-0033 is not rewritten (repo convention: an accepted ADR is superseded, not edited) — the
stale line carries a pointer to its successor and is scoped to the cadence claim alone. The
workflow comment now says it is the operational summary, not the record.
Also notes the consequence a monthly rebuild does NOT have: a takedown does not wait for the
schedule. It goes through ADR-0031 and, on live exposure, a direct UPDATE on the served D1.
|
Благодаря за повторната проверка, @nikimilenkov — и че прекара отворените находки от другите два прегледа през този head, вместо да ги оставите между нишките. Четирите, които посочихте, се възпроизведоха. Поправени в четири подавания върху
Major 1 — това беше единствената находка по публикуващия път
Неизвестна година вече проваля проверката, вместо да я прескача. Кракът за седалище без дата на вписване остава: това е обичайната форма за дружество, което не се е местило, и е проверимо, защото от другата страна стои известна година. Три теста — двете форми с Гейтът за монотонност — по-лошо, отколкото е оцененоНе е липсващ авариен изход, а блокировка в две посоки, и то върху механизмите, към които самият ADR сочи:
Значи оставаше само вдигане на Одитът вече чете текущия статус на всяка изчезнала връзка ( Изтекла разписка проваля хода — по-остра причина от B3: тя би предварително оправдала бъдещо изчезване на същата връзка, тоест точно регресията, заради която гейтът съществува, без никой да го е решил. Разписките са еднократни; ръководството го казва, както и че списъкът не се използва за сваляне, което не разбираш. Едно предложение, което не приехМетенето на сираци беше посочено, че ползва суров Каденцията — записана като решениеПрава сте, че коментар в YAML не е записът. ADR-0034 записва двете ограничения, които се сблъскват, и двете отхвърлени алтернативи — пренасяне на актовете зад чужда политика за изгонване, и обхождане, което издава присъди по (връзка, ЕИК), за да пресичат границата само булеви стойности. Второто е верният път назад към ежедневно решение и е назовано като такова, вместо да остане подразбиращо се. ADR-0033 не е пренаписан (конвенцията на хранилището: приетият ADR се заменя, не се редактира) — остарелият ред носи препратка към наследника си и е ограничен само до твърдението за каденцията. Записано е и последствието, което месечният ход няма: свалянето не чака графика. То минава през ADR-0031, а при жива експозиция — през пряк Браузърната проверка минаПрава бяхте и за причината, и за заобикалянето:
Отделно, и не го поправям тук: несъответствие при хидратация заради CSP nonce се извежда на всяка страница, включително Проверка: |
|
Бележка за червената проверка Пада стъпката „Dependency audit", на новопубликуван съвет за Пакетът е чисто преходен и само за времето на изграждане: Поправката е лек повдиг: Останалите четири проверки ( |
The Dependency audit went red on a newly published advisory for nanoid 3.3.16 (GHSA-2v37-7h3g-55p8, CVSS 8.2), not on anything this branch changed — it touches neither package.json nor pnpm-lock.yaml, so main fails identically on its next run. Lockfile-only, and confined to nanoid: postcss@8.5.24 already declares `^3.3.16`, so 3.3.18 satisfies the existing range with no manifest edit and no override. `pnpm update nanoid` does not lift a transitive pin; `-r` does. Not suppressed in osv-scanner.toml on purpose — that file's own doctrine reserves entries for advisories whose only fix is unavailable or a major upgrade, and this one is a patch. Verified with the same checksum-pinned osv-scanner 2.4.0 the CI step runs: "No issues found". The package is build-time only (postcss → vite → vitest, all dev) and reaches no Worker bundle, but the chain is exercised anyway — full suite 6/6 packages, lint, typecheck 7/7, and a real `@sigma/web` production build through postcss.
Review — midt-bg#279 registry evidence replaces name-distinctivenessVerdict: Request changes. This is a libel-critical surface (every published link names a real official ↔ company), so I'm holding on one verified false-link path plus two hardening gaps. Security is clean and the crawler/EIK/КДА/parser work is solid. Scope reviewed against the PR head, prioritising the publish/read gates. Security — clean
BlockingCRITICAL — a false official↔company link can still be published via bidder-only resolution + a registry homonym (
The registry evidence proves that someone with that name owns B — not that this official does. So the ladder transfers the name-collision risk into the Register rather than closing it, which defeats the shared-name/homonym requirement midt-bg#279 is built on. The existing ambiguous-key quarantine only covers winner-vs-winner collisions; the winner-vs-non-winner case above isn't handled or documented as accepted residual risk. Please either add a disambiguation gate (e.g. require the declared/registry seat or ЕИК to corroborate the company identity before a Should-fix (hardening — together these also gate merge)HIGH — the company-search badge skips the evidence seal ( The main conflict query is belt-and-braces — HIGH — no DB invariant enforces "published ⇒ sealed" ( The seal table FKs evidence → interest_links, but nothing (reverse FK / trigger / CHECK) forces a Medium (tests)
What passedThe evidence ladder's fail-closed posture (missing deed throws; joint-stock/unknown withhold), the crawler safety, EIK checksum, КДА fix, host pinning, and bound-parameter SQL all check out. Relevant pure suites ran green (93/93: evidence, deed parser, EIK, client, КДА). Overall this is careful, well-documented work — the ADR-first framing and the belt-and-braces on the main query are exactly right. The blocker is that the company resolution step upstream of the evidence ladder can still point at the wrong same-named company, so the registry proof lands on a homonym; closing that (and mirroring the evidence gate onto the search badge + enforcing the seal invariant) would make the surface match its stated guarantee. |
Deep-dive review vs midt-bg#279 (governed by ADR-0033)ADR-0033 is the governing spec and it deliberately amends midt-bg#279 in four places, each justified by live probing of the register. This review is against ADR-0033-as-corrected, not midt-bg#279 literal:
Bottom line: the core registry-evidence pipeline is implemented correctly and to a high standard. The gaps concentrate in the inherited midt-bg#226 matcher cleanups that midt-bg#279 §5 (item 4) scheduled for this PR — two of which are genuine wrong-publish-direction defects that the maintainer's "these four don't fire on today's corpus" clearance did not cover. What is correct (the hard core)
Findings that fail toward publishing (highest priority)All three were on the midt-bg#279 §5 matcher checklist (item 4: "Матчър: 1.1, 1.3, 1.4, 1.5") for this PR. Unlike the four defects the maintainer checked and cleared, §1.3 and §1.4 were not among the cleared four and remain unverified against the corpus.
Schema hardening the ticket asked for (§2) and didn't get
Deviations from ADR-0033 / midt-bg#279
Inherited midt-bg#226 query defects — status (maintainer reclassified as non-blocking follow-ups)
Defensible as follow-ups per the maintainer's explicit call, but they were in the §5 plan for this PR — flagging as consciously-deferred, not silently done. Test-coverage gaps (no bug proven behind them)
Suggested pre-merge order
Items 9–11 are legitimately follow-ups per the maintainer and midt-bg#279 §4, but should be tracked. Review assisted by Claude Code — findings verified against the branch at file:line. |
`closelyHeldForm` asked whether the name ENDS in a joint-stock form. A declarant who types „ТРЕЙС ГРУП ХОЛД АД София" defeats that: the comma-peel needs a comma and SEAT_MARKER needs a literal dot, so neither strips the seat, the name stops ending in its form, and a listed АД reads as closely-held — the „11 акции на Trace → €88M" trap, reached from the plainest input shape there is. Decide on the LAST form TOKEN instead of an end anchor. Position-independent, so a trailing seat cannot flip the verdict, while „АД" leading („АД ГРУП ООД") or glued („АД-ХОК ЕООД") still isn't the form. JOINT_STOCK itself is untouched — deed.mjs's envelope test reads a registry-clean `fullName` that does end in its form, and deed.test.mjs pins the two patterns to each other. `stripSeatSuffix` gains the same cut for nameDistinctiveness, where the blind spot fails toward PUBLISHING: an uncounted seat token inflates the content-word count to 3 and reads as distinctive. Truncation is anchored on фирма-TERMINATING forms only — ЕТ/СД/КД precede the name, and cutting after them would eat it. Positive controls throughout: a predicate that always returned false would pass every new bar assertion, so each is paired with a name that must stay material.
…eadable `if (!Number.isFinite(fy)) continue` looked like the surrounding fail-safe — "never withdraw on missing evidence" — but it is the opposite. An absent filing legitimately yields no horizon; a PRESENT filing that is silently dropped never advances one, so `divested` stays false and a stake the official has since sold keeps naming them on the public surface. Stale claim, real person, no symptom in the output. filings.jsonl already carries `folder` (extract.mjs), so fall back to it. The folder is the PUBLICATION year and runs ahead of the declared year, so the horizon can advance up to a year early — that errs toward withdrawing a claim we are no longer sure of, which is the safe direction on this surface. Documented at the fallback rather than left for a reader to infer. A filing datable by NEITHER field is still ignored: this dates a filing, it does not invent one. Both counts are reported, because a horizon that failed to advance is invisible — without the counter, a corpus-wide date regression would show up only as a surface that quietly stopped withdrawing anything. The RED publishes via the SEAT rung, not rung 2: an owner-matched deed would let §7 reconciliation reverse the divestment and mask the bug. Mutation-checked — removing the fallback fails the test with published/withdrawn. Positive control: an undatable filing leaves its link published.
`colNum` always returns a fallback, so a table that renumbers the holder column resolves it to a column that is not in the row. `by[cHolder]` is then undefined, the caller reads '' and `classifyHolder` maps a blank cell to 'self' — its documented meaning. A RELATIVE's stake becomes the official's own, enters the OWN-only refutation path and publishes as their private_ownership. The distinction the parser needs is column RESOLVABLE-BUT-EMPTY (self, a blank cell does mean the declarant) versus column NOT RESOLVABLE (unknown — we never read the holder, so we make no claim). `colNumOrNull` supplies it for the holder column alone; the other five call sites keep `colNum` and its fallback verbatim. Scope: parse.mjs was byte-identical to main. It is touched here because midt-bg#279 §5 item 4 scheduled §1.4 for this PR, and this defect fails toward publishing a stake against a person who did not declare it as theirs.
…only the person ADR-0033 held that rung 2 needs no gate because "the register named this person in THIS company". That holds only if THIS company is the one the official declared, and it is not guaranteed. `resolveEntity` maps a declared фирма to the sole ЕИК among PROCUREMENT WINNERS, and `nameGloballyUnique` ranges over bidders — never the register at large. So an official who owns a same-named company that never bid resolves to an unrelated winner, and a three-token homonym in that winner's deed completes a link false in both halves. Two coincidences, neither rare in Bulgaria. The ladder moved the name-collision risk into the Register instead of closing it, which is the premise midt-bg#279 rests on. Rung 2 now asks for a reason to believe the company is the declared one: a declared ЕИК (the identifier resolves it outright), a declared seat matching the registered one, or a фирма distinctive enough that a national twin is improbable. Anything else returns a new withholding kind `document_uncorroborated` — never publishing, carrying neither role nor matched_fact, since asserting either would leak the claim the rung just refused to make. It is a distinct kind rather than a fall-through to `unknown` because "matched a person, could not establish the company" and "matched nothing" are different facts, and the seal exists on held links precisely to keep that queue reviewable. It is counted in the run summary: the third corroborator is a bound, not a proof, and that number is what F8 reads to decide whether this tightens to strict ЕИК/seat corroboration. Strict was the alternative; declared seats come only from the ООД/ЕООД table of asset declarations, so its recall cost cannot be known before that measurement. `companyNameDistinctive` defaults to FALSE — this gates the primary publishing rung, so a caller that forgets it must withhold. Rungs 2 and 3 now share one `matchDeclaredSeat`, R10 included, so they cannot drift about what a seat match means. Mutation-checked three ways: never-withhold fails 4 tests, always-withhold fails 7 (the positive controls hold it to a bound), and dropping R10 from the shared helper fails 2. Covered end to end in load.test.mjs, not only as a pure function. ADR-0035 records it and supersedes ADR-0033 decision 2 on this point alone; the methodology page discloses the rule verbatim per ADR-0021 E10.
… natural key midt-bg#279 §2 asked for CHECK constraints and a non-null control_hash. The enums are done as asked; the control_hash half is not, and the reason matters. Enums. `status` and `interest_class` (0003) and `evidence_kind`/`registry_role`/ `live_status` (0006) were plain TEXT while the surface reads them as enums. A value that only LOOKS like a gate value — 'published ' with a trailing space, the case the ticket names — passes every writer and then fails `status = 'published'` silently. A CHECK binds every writer at once, including a hand-run UPDATE during an incident, which is exactly when it gets typed. 0006 is unshipped in this PR, so its constraints go on directly. control_hash NOT NULL is REJECTED, and the underlying defect fixed differently. The field is genuinely optional at the source — the register omits <ControlHash> and parse.mjs carries that through as null — so NOT NULL converts a missing optional field into a run-stopping loader failure, and a fabricated placeholder would assert an integrity check nobody performed. The real defect is that `UNIQUE (xml_file, control_hash)` never constrained re-import: SQLite counts NULLs as DISTINCT, so every hashless declaration was mutually unique and re-imported as a fresh row each run, double-counting its stakes. Replaced by a unique index over (xml_file, folder_year, COALESCE(control_hash,'')) — the same natural key `id` already encodes, since the register reuses basenames across folders. 0007 retrofits both onto a deployed D1. `CREATE TABLE IF NOT EXISTS` never revisits an existing table and ship-related-persons wipes ROWS, not definitions, so a live database would otherwise keep the unconstrained 0003 shape forever however often the data reloads. interest_links needs a rebuild (SQLite cannot add a CHECK in place) under defer_foreign_keys, since interest_link_evidence references it and D1 enforces FKs; declarations needs only the corrected index. Both converge on re-application, which the bare `d1 execute --file` path requires, and both workflows apply it. Tests cover the trailing-space case by name, the ADR-0035 kind, the hashless duplicate that used to slip through, and a pre-0007 database that keeps its rows through the retrofit and survives a second application. Positive controls throughout: every real status and class still inserts, and the natural key still admits the same basename in a different folder and a corrected re-filing under a new hash.
… all four copies The detail-page gate is belt-and-braces — SURFACED_OWNERSHIP requires `status='published'` AND a publishing seal. The company-search badge checked status alone, so a published row whose seal was missing or withholding (a legacy row, a partial run, a loader bug) advertised a свързани-лица claim about a named official to every searcher, while the page behind the badge correctly showed nothing. Search is the wider surface of the two: it is what a reader sees before deciding to look. The table probe now requires interest_links AND interest_link_evidence. The join reads both, so an env with 0003 but not 0006 would 500 every search on every kind — the exact failure the probe was written to prevent. Requiring both also gives the right answer there: with no seal table nothing is provably sealed, so nothing should badge. The predicate now has four copies, and nothing but a test can bind SQL across three languages and two file types. related-persons-sql.test.ts asserts all four agree on both the EXISTS shape and the rung list, whitespace-insensitively. refresh-slice.sql is in that set for its own reason: it runs on the 6-hourly cron, so drift there silently keeps officials in the search index whose links no longer surface. Mutation-checked: adding a rung to refresh-slice.sql alone fails both binding tests. The badge fixture carries an unsealed published link and a withholding-sealed one, each of which must not badge, plus sealed links on both publishing rungs that must.
…b to staging §2.6 made the CARD labels tense-neutral and family-aware but left the surrounding page copy asserting „собствен дял" — an OWN stake — directly above cards that correctly read „деклариран дял на свързано лице". On a family-only page that is a false claim about the named official, and it is exactly the second source of truth the card-label fix set out to remove. `declaredStakeNoun` derives the wording from the links the page is actually rendering, so a page cannot describe a set it isn't showing. Family-AWARE rather than family-blind: an own-stake page still says „собствен дял" (a positive control covers this — deleting the claim everywhere would pass the family assertion while making the page vaguer than the data warrants), and a mixed set gets the only phrasing true of every card. The ledes also stopped claiming „точно съвпадение по фирмено име" — since midt-bg#279 identity rests on a Trade Register fact, not on the name. related-persons-data.yml offered `default: dev` while no `dev` environment exists, every fallback in the file is `|| 'staging'`, and the note at the top says the job targets staging. A manual dispatch therefore went to an environment with no secrets and failed at the credential guard rather than running where intended. Dropped from the options too, so it cannot be picked deliberately either. Methodology: „премахва незабавно" now distinguishes the D1 write (immediate) from what a reader sees, which the page's own publicCache(3600) can hold for up to an hour. The promise was not wrong about the action, only about when it becomes visible.
…nd the rule disclosure All three gaps were named by reviewers, and none had a bug behind it — but each guards a path where failure is silent, which is the case for testing them rather than trusting them. Audit axes A_key_missing, A_eik_mismatch, B_bidder_eik and B_eik_invalid fired in no test. The audit is the last gate before a named claim ships, so an axis that quietly stopped firing would be invisible: A_eik_mismatch is the libel case in its purest form (a name resolving to one winner, the link published against another), and B_eik_invalid is the only rail keeping a checksum-invalid ЕИК off the surface. Mutation-checked — disabling B_eik_invalid fails its test — and paired with a positive control, since four assertions that something fails prove nothing unless the clean shape passes. Every rung-2 owner test used CR_F_19_L, leaving CR_F_18_L (едноличен собственик) and CR_F_23_L (ФЛ-търговец) unexercised. A dropped entry there would withhold an entire ownership shape — a sole owner, the commonest ЕООД form — as „Неизвестна", a recall hole with no symptom. Controlled by a non-ownership field code that must NOT match. The methodology page's rule disclosure is a launch condition under ADR-0033 decision 7, not a nicety, and nothing kept it in step with the ladder. The rule could change in evidence.mjs and leave the page describing a system that no longer exists — worse than not disclosing it. Now pinned: every rung by name, the joint-stock bar, the three-names/ one-record rule, ADR-0035's company gate, both halves of R10, the no-ЕГН homonym limit, and the company-identity-not-ownership distinction the surface rests on.
…rebuild `wrangler d1 migrations apply` rejected the create-copy-drop-rename rebuild: interest_link_evidence references interest_links(link_key), D1 enforces foreign keys, and `PRAGMA defer_foreign_keys` does not survive the statement-by-statement execution wrangler performs — the migration aborted with SQLITE_CONSTRAINT_FOREIGNKEY and the Durable Object rolled back. The sqlite3-CLI test passed because that CLI has foreign keys OFF by default, so only a real local apply surfaced it. Dropping the child first would work mechanically and is the wrong trade: it strips every evidence seal, and since the read gate REQUIRES a seal, that empties the public surface until the next monthly data run. BEFORE INSERT/UPDATE triggers enforce the identical invariant for every writer — including the hand-run UPDATE during an incident that no application-level validation covers — with no rebuild, no FK exposure and no seal loss. Fresh databases still get true CHECK constraints from 0003/0006; this is only the retrofit path for databases that already exist. Verified against real local D1: applies clean, and a second application is a no-op. The test now also covers UPDATE, and asserts that a pre-existing bad row SURVIVES — a trigger constrains future writes and cannot retroactively reject stored data, and deleting a real link on a guess would be worse than leaving one the read gate already refuses to show.
|
Благодаря и на двамата — прегледите бяха точни и намериха един и същи клас дупка от двете страни. Възпроизведох всяка находка срещу HEAD преди да пипна код; три от предпоставките не издържаха тази проверка и ги отбелязвам отдолу, вместо да ги реализирам мълчаливо. Кръпките са КРИТИЧНАТА (@DiyanaDimitrova) — затворенаНаходката е вярна и по-точна, отколкото звучи: стъпало 2 доказва, че някой с тези три имена е вписан в дружеството, което сме извикали, а не че това е дружеството, което лицето е декларирало. Решение (ADR-0035): преди стъпало 2 да твърди, нещо извън фирменото наименование трябва да установи дружеството — деклариран ЕИК, декларирано седалище, съвпадащо с вписаното, или достатъчно отличителна фирма. Всичко останало връща нов, отделен въздържащ се вид Отделен вид, а не пропадане към Избрахме отличителността пред строгото потвърждаване (само ЕИК/седалище) съзнателно: декларирано седалище се извлича само от таблицата ООД/ЕООД на имуществена декларация, така че цената в покритие не е измерима преди F8 и можеше да свали повърхността под прага за публикуване. Строгият вариант остава на масата и измереният остатък е входът за това решение.
Стъпала 2 и 3 вече споделят една реализация на съвпадението по седалище ( Мутационна проверка (3 варианта): гейт, който никога не задържа → падат 4 теста; гейт, който винаги задържа → падат 7 (позитивните контроли го държат като граница, не одеяло); премахната R10 от споделения помощник → падат 2. Покрито и end-to-end в HIGH — значката в търсенето (@DiyanaDimitrova) — вярно, поправеноПотвърдено: гейтът има четири копия, не три. Проверката за наличие на таблица вече изисква и двете таблици — join-ът чете И трите копия вече са вързани с тест ( Находки на @cefothe§1.3 ( §1.4 ( §1.1 ( ADR-0033 решение 3, ред 182, казва обратното на цитираното: „ Бъгът вътре обаче е реален и го възпроизведох:
§2 (схема) — направено, с едно съзнателно отклонение. Enum-ите са CHECK-нати както е поискано:
§2 ретрофит — и находка, която само реален пуск даде. Първата версия на 0007 беше учебникарското create-copy-drop-rename. Мина през тестовете със Изхвърлянето на детето първо работи механично и е грешната сделка: маха всеки печат, а понеже гейтът за четене изисква печат, това изпразва публичната повърхност до следващия месечен пуск. Затова: тригери §8 ( §2.6 (остаряла проза) — вярно, поправено. §9 („незабавно") — уточнено. Записът в D1 е незабавен; страницата може да се вижда в кеш до час ( Находка, която вече съществуваше (@DiyanaDimitrova, HIGH #2)Одитната проверка „публикувана ⇒ подпечатана" вече е налице: Тестове (и двата прегледа)Четирите неупражнени одитни оси ( Проверкаturbo 6/6 — 492 web, 393 db, 45 shared, 48 ingest, 12 etl, 10 config · 53 root scripts · 248 pipeline · lint, typecheck 7/7, Две бележки по браузърния пуск: изисква локално зададен Съзнателно отложено
|
|
Поправка към по-ранния коментар: там писах, че вдигането на Не го подтиснах в |
midt-bg#302) OCDS анексите стейджваха OCID в `unp` вместо УНП, тъй че нито един не се връзваше с договор. УНП липсва в емисията; онова, което release-ът носи, е `tender.id` - същото пространство като `tenderId` на EOP. Поправката е в три части, неразделни: ingest хваща `tender_ext_id` и записва стойността от release-а като `value_before` с празно `value_after` (OCDS не може да знае стойността СЛЕД анекса и не бива да движи `current_value`); мост в derive-amendments.sql и refresh-slice.sql възстановява УНП през `tender_ext_id`, с отказ при двусмислие; и prefer-EOP dedup маха близнаците, за да не се удвои `annex_count`. По инкременталния път dedup-ът се сверява и с кумулативната сервирана таблица в двете посоки. Остатъчното подценяване е наблюдаемо през две диагностики, а инвариантът е закован от нова integrity проверка `amendment-twin-dedup`.
Проверката `amendment-twin-dedup` влезе в CHECKS с midt-bg#302, но нищо не доказваше, че може да падне: мутация `ok: n === 0` до `ok: true` оставяше целия суит зелен при напълно обезсилен гейт. Пет случая по идиома на файла: засято нарушение; формите, които работещият dedup наистина оставя и които не бива да палят гейта (два EOP анекса на договор, само-OCDS анекс на друг договор, отказан от моста ред под своя OCID, NULL номер на договор и NULL УНП); броене на двойки, не на редове, с три нарушения, които делят стойност по всяка от осите, тъй че се заковава и самият ключ на групиране; и самопропускането при липсваща таблица. Източниците са реалните дялове, а EOP страната стои в двата края на диапазона 2020-2026, тъй че закачане за една година вали. Плюс липсващата декларация на `checkAmendmentTwins` в `integrity-checks.d.mts`, която midt-bg#302 беше пропуснал. Четири кръга независим преглед; всяка находка е поправена и проверена с мутация.
checkout 7.0.0 → 7.0.1, pnpm/action-setup 6.0.9 → 6.0.10, setup-node 6.4.0 → 7.0.0, actions/cache restore и save 4 → 6.1.0. Всичките заковани по SHA с коментар за версията; само файлове в .github/workflows. Двете големи скачания са изпълнени от CI върху обновения клон, включително заданието cacbg, което иска Node 24.
Когато възложителят пропусне десетичния знак, стойността излиза точно 100 пъти по-голяма. `value_suspect` искаше над 200x от прогнозата, тъй че тези случаи минаваха отдолу и се сервираха на лице. Прагът не е свален - добавена е тясна лента около 100x. Мярката върху корпуса показва изолирано струпване: 13 договора между 95x и 105x, само 2 между 85x и 95x, и НУЛА между 105x и 200x. Границата идва от данните, не от преценка. Всичките 13 са проверени поотделно и са безспорни. Най-ясният: 05492-2023-0002 е доставка на 25 хил. литра дизелово гориво - при показаните 3 049 971 EUR излиза 122 EUR на литър, при прогнозата 31 189 EUR излиза 1,25 EUR на литър. Сред тях е и 00105-2025-0026 от доклада: 102 258 376 EUR при прогноза 1 022 584. Ефект: 13 договора, 318,3 млн. EUR показвани, 3,2 млн. след поправката към прогнозата. Логиката на флага стои в пет копия в двата derive пътя; сменени са всичките. Тестът кара истинските скриптове срещу истинска sqlite веднъж за всеки път, тъй че пропуснато копие вали. Мутации: махането на лентата от който и да е от двата файла вали по един тест, а разширяването ѝ до 90x-110x вали два - границата е закована, не козметична. Затваря midt-bg#247.
…idt-bg#299) annex_suspect искаше сборът текуща/подписана да стигне 100x, тъй че анекс със сбъркано число, издул договора до 30-50x подписаната стойност, се сервираше на лице. Новото правило е съюз: единична анексна стъпка >=10x И сбор >=5x. Двете половини са премерени поотделно на staging: - само стъпката не стига: 3 договора имат стъпка до 36 058x, но по-късен анекс сваля грешката и сборът свършва ПОД подписаната стойност - флаг там би ВДИГНАЛ показваната сума; - само сборът не стига: 40 договора на 5-100x се катерят бавно, без нито една подозрителна стъпка - остават както са (16,8 млн. EUR). Ефект: 43 договора, 184,6 млн. EUR показвани, 2,9 млн. след връщането към подписаната стойност. Най-големият: 105,2 млн. EUR при 49x сбор и 48x стъпка. Правилото влиза в петте копия на флаговата CASE и в охраната на reconciliation паса на refresh-slice; без охраната договор, чийто анекс пристига в по-късен прозорец, оставаше 'ok' завинаги - има тест точно за този сценарий. По въпроса от прегледа за приоритета между флаговете: когато договор е едновременно в лентата 95-105x И носи сбъркан анекс, value_suspect печели и repair-ва до процедурната прогноза, вместо annex_suspect да падне към подписаната. Премерено - точно един договор на целия корпус пали и двете правила (00747-2024-0003): подписаната му стойност е 1 093 863 EUR, а прогнозата 1 096 593 EUR, тоест двете котви се разминават с 0,25%. По принцип подписаната е по-добрата котва, когато виновникът е анексът, защото е записано число за този договор, а не оценка на ниво процедура; при N=1 и такава разлика пренареждането на CASE-а не се оправдава. Затваря midt-bg#248.
Re-review — midt-bg#279 registry evidence (@
|
Brings the branch current before opening the upstream PR (AGENTS.md: pull main in if a branch lingers). Merged rather than rebased — the branch is public and under review on the fork PR, so its history must not be rewritten. nanoid: upstream midt-bg#291 made the same 3.3.16 → 3.3.18 lockfile move as 36cdd9d and additionally pinned an override; both sides agree on the resolved version, so the lockfile merges to one consistent state.
The merge with upstream/main was textually clean and semantically broken — `git merge-tree` reported no conflict, and four suites then failed. Both sides had edited refresh-slice.sql and ship-related-persons.mjs, and upstream's new harnesses build their schemas from migrations that stop at 0003, while this branch made both files depend on interest_link_evidence (0006). - refresh-slice.test.ts: upstream's new OCDS-bridge site (midt-bg#286) was a third setup site this branch had not seen; without 0006 sqlite3 aborts on "no such table" at refresh-slice.sql:1957, before reaching the logic under test. - value-flag-stotinki-sql.test.ts / value-flag-annex-step-sql.test.ts: new files, same cause — they died on the свързани-лица block while testing the value_flag CASE. - ship-e2e.test.ts: the evidence seal is now a shipped table, so a target built from 0003 alone makes the generated wipe abort before a row moves. Its corpus now seeds one seal per link, which is the only shape the ship should ever carry — a published link without one is what the audit fails on and what the read gate refuses to surface. One behaviour change worth naming. `a request that never landed fails the run` injected a skipped interest_links chunk; with seals shipping, that now trips the FOREIGN KEY as the orphaned rows land, before the read-back runs. Stronger, but it leaves the read-back gate — the thing the test exists for — unexercised. The injection moves to a seal chunk, which has no dependents, so the shortfall stays invisible until the counts are compared. All suites green on the merged tree: 421 db, 492 web, 61 ingest, 45 shared, 20 etl, 10 config, 91 root scripts, 248 pipeline; lint, typecheck, check:docs and the coverage ratchet clean.
6 participants
Какво и защо
Реализация на midt-bg#279: връзка „длъжностно лице ↔ дружество" се публикува само срещу проверим факт от
Търговския регистър, а не срещу евристика по отличителност на името.
Днес самоличността на дружеството зад декларирано име се решава от
B_distinctive. От 101 публикуванивръзки 36 стоят на голо съвпадение на име. midt-bg#279 сменя основанието — и това е разширяване, не
затягане: повърхността расте до 329 връзки, тоест 264 връзки, които днес умишлено задържаме, стават
поименни публични твърдения. Затова прагът за точност се вдига, а не се запазва.
Решенията са записани в ADR-0033,
написан преди какъвто и да е код: ако прегледът отхвърли основанието, пада целият подход. Заменя
ADR-0009/0015/0017; изменя ADR-0007/0010/0021(E11)/0028.
Какво съдържа
ea00781/conflictsрендира празно и функцията е непроверима в браузърb364031scripts/tr/: клиент, кеш от актове, ЕИК контролна сума като Node функция + поправка на CI глобf8b198f98ac4f5800b2c15f64f5ccloselyHeldFormне изключваше КДА4226ae3load.mjs2a08744796903aКакво реализацията откри и поправи в дизайна
Работата по кода опроверга пет неща, три от които бяха записани грешно в самия ADR. Всички са
поправени в ADR-а с измерването зад тях.
Срещу issue-то:
Община София, празно два пъти, докато реално дружество връща пълен акт в същия прозорец. Правилото
стъпва на статуса, не на празнотата: празно при 200 е отговор и може да се кешира трайно; празно
при 5xx е отказ и остава преходно. Без това ~4 ЕИК никога не биха се разрешили.
fetchне работи срещу този хост. Същата заявка връща 500/празно през undici и 200 с пълния акт(34 398 байта) през
node:https. Ако беше минало така, всяко търсене щеше да се проваля като „5xx" ида се повтаря по 5 пъти — ~2000 заявки вместо ~400 срещу регистър, който блокира около 50-ата.
Срещу собствения ми ADR:
3. Маркерът за заличаване е
erasure-text-inline, неfield-text--erased— второто не се среща нитоведнъж в живия акт, а заличеният контейнер няма изобщо параграф
field-text. По ADR-а заличенитезаписи биха се четели като живи — точно капанът, за който самият ADR предупреждава.
4.
fieldOperationне е сигналът за заличаване (2 при заличените тук, 1 при заличените историческизаписи от сондажа) — съблазнителна корелация, негодно правило.
Два дизайнерски пропуска, хванати от фикстурите:
5. Бях приложил твърде широко гейта на ADR-0017. Той не бива да важи нито за декларирания ЕИК (ЕИК-ът е
самоличността — ADR-0028; иначе изхвърляме най-силния идентификатор точно там, където името е
безполезно), нито да изисква отличителност за седалището (седалището съществува именно за да
спасява родови имена). Стеснен до онова, което ADR-0017 всъщност постановява.
Едно стеснение на повърхността, което трябва да се знае
При семеен дял вписаният собственик е роднината, чието име умишлено не пазим — затова стъпало 2
(„Документ") не може да сработи по конструкция. Самоличността се потвърждава само от нещо, което
самото длъжностно лице е декларирало: седалището или ЕИК-а. Решението на ADR-0032 е непокътнато
(семейният дял се публикува поименно наравно със собствения), но вече иска същото доказателство като
всичко останало. Това е съвместимо с числата на issue-то (59 семейни от 329 повърхностни) и е покрито с
позитивна контрола — семейна връзка с декларирано седалище, която публикува, иначе „семейни: 0" е
неразличимо от структурно мъртъв път.
Какво е проверено и какво не
Последният ред е F8 и е блокиран от данни:
scratch/е празен, тоест иска пълно учтиво обхождане накорпуса (~256k декларации) преди ~400 регистърни справки. Планира се отделно.
Две неща вече изместиха базата спрямо измереното в issue-то и трябва да се имат предвид при сверката:
midt-bg#281 махна 87 фантомни реда, а стеснението по-горе намалява семейните връзки.
Как е тествано
node --import … --test scripts/cacbg/*.test.mjs scripts/tr/*.test.mjsscripts/tr/)node --test scripts/*.test.mjspnpm testpnpm typecheck·pnpm lint·pnpm check:docsФикстурите на парсера са реална разметка от жив акт (ЕИК 115536179). Ключовият тест доказва и трите
посоки: съставено име от двама различни хора не съвпада, правилният декларатор съвпада, а наивното
съпоставяне срещу цялото поле би сработило — без последното твърдение матчър, който винаги връща
false, минава целия файл.Свързан issue
Част от midt-bg#279. Предпоставката по §12 влезе нагоре по течението като midt-bg#281.
Вид промяна
feat— нова функционалностfix— поправка на бъг (КДА)docs— ADR-0033, методология, спец, LIA, runbookci— разписания + поправка на глоб, който не хващашеscripts/tr/Чеклист
Co-Authored-By:trailerpnpm typecheckминаваpnpm testминаваpnpm lintе чисто.env*или.dev.varsdocs/е обновена