fix(tr,cacbg): 429 е изстиване, не спиране — и присъдите преживяват хода - #3
fix(tr,cacbg): 429 е изстиване, не спиране — и присъдите преживяват хода#3ydimitrof wants to merge 10 commits into
Conversation
ADR-0033 context 4 recorded the limiter as ~50 cumulative requests followed by
a sustained block, and decision 7's crawl hygiene ("a 429 is an instruction to
STOP") rests on that. Re-measured on 2026-08-19 through client.mjs's own
httpsGet — the production transport, same UA, same headers, so the result
transfers:
budget 5 requests; the 6th returns 429
scope IP, not session — a fresh EPZEUSessionID is refused too
shape 0 bytes, no Retry-After, no X-RateLimit-*, 48-67ms vs 250-475ms
for a real deed, so refused at an edge before the application
second face while blocked the connection may STALL (20s timeout), not 429
recovery still blocked at +60s, clear at +161s
CI run 32154394308 fails identically — at request 6, `TR cache: 5/537 covered`
— so the threshold is not a property of GitHub's runners.
Both halves of the old premise are false: the trip point is 5, not ~50, and the
block is not sustained. That changes what the design can achieve: 537 candidates
at 5 per ~176s cycle is ~5.3h against timeout-minutes: 300, and since the deed
cache deliberately does not survive the runner, every run restarts at `cached 0`.
No pacing value completes 537 lookups in one job.
ADR-0036 records the measurements and rules that a 429 is a cooldown, not a stop.
ADR-0037 takes the way out that ADR-0034 named and left on the table: the crawl
emits per-(link, ЕИК) verdicts — a role, an entry reference and booleans, no
third-party name — so only those cross a run boundary and progress becomes
durable.
client.mjs keeps its behaviour exactly: it still reports the 429 and never folds
it into the retry budget. Only the justification changes, and the wait-and-resume
policy lands in fetch-deeds.mjs, where "how long to wait" belongs.
No code paths change in this commit.
…n one 429 The block clears in ~161s (ADR-0036), so ending a whole crawl on the first 429 threw away the run for a wall that was about to come down on its own. In CI that meant five deeds out of 537, every month. `fetchOne` now waits RATE_LIMIT_COOLDOWN_MS (180s) and re-requests the SAME ЕИК, recording nothing about it in between — the block is a fact about us, not about that company. Three cooldowns without a success still end the run with exit 2, so a genuinely sustained block stays distinguishable from the ordinary 5-per-window rhythm, and the counter resets on any success so a slow healthy crawl never approaches it. The block has a second face: while it holds, the connection stalls rather than answering 429. Before the first cooldown we cannot know we are blocked, so a network fault keeps the documented TRIES_PER_EIK budget; once we do know, further attempts get exactly one try and any throw reads as "still blocked". Otherwise each stall costs five 20s attempts fed into a tarpit — the new test pins the difference at 4 requests rather than 16. `--max-runtime-min` lets a run stop cleanly between two ЕИК when its wall-clock budget is spent. Candidates never attempted are not counted as unresolved, but a candidate we asked about and lost still exits 1 — a deadline must not launder a real failure into a green run. This is only safe because progress is cached; the verdict cache that makes it safe across runs lands next (ADR-0037). politeTrGet is untouched. It still reports the 429 and never folds it into the retry budget; only who decides how long to wait has moved. scripts/tr: 141 tests pass, 255 across the whole scripts lane.
…roduced it The deed cannot cross a run boundary — it carries the names of co-owners and managers who hold no public office (ADR-0033 decision 5) — so every run has been restarting at `cached 0` and re-spending the register's budget on the same first few ЕИК. The decision that deed produces carries none of that: a kind, a role, an entry reference and booleans. ADR-0037; the `verdicts` table is where it lives. `verdictInputsHash` is the invalidation, and it refuses an evidenceVerdict argument it does not recognise rather than hashing a subset. The failure mode of a missed input is silent — a stale decision about a real person, published — so adding an input now fails the run until someone decides where it belongs. Arrays are sorted before hashing because declaredSeats arrives from a Set spread, and insertion order must not read as a change. A verdict is stale three ways, each of which must re-decide: the rules moved (RULES_VERSION), the declaration moved (inputs_hash), or the lookup simply aged. Coverage and pending are computed over LINKS rather than ЕИК, since one company carries several and a rules bump invalidates them independently of when its deed was last seen. The ЕГН rail screens verdict rows too — this is the row that crosses a boundary, which makes it the surface most worth screening, not least. purgeExpired ages verdicts on the same clock: not as a privacy rail here, but because a published claim must not outlive the lookup it rests on. ADR-0037 said the family-scope link key might carry the relative's name and would be hashed if so. Verified during implementation: personId is built from the DECLARANT, who is the official even for a family-scope stake, and the relative is never named. Replaced the conditional with the checked fact. scripts/tr: 150 tests pass.
…mmediately `--links-file` carries the declaration side of each decision, so the crawler can compute evidenceVerdict at the moment it holds the deed rather than handing the deed onward to a decision step that must not receive it. The raw body is deleted as soon as it has served its purpose; the end-of-job rm -rf stays as the backstop but no longer has to be the only thing standing between a failure and a deed left on disk. `--eiks-file` still works and still only fetches. The two are mutually exclusive: one names companies, the other names decisions, and quietly treating a links file as a candidate list would fetch deeds nothing ever decides. The pending set is computed over LINKS in links mode. Deed freshness alone would skip a company whose deed is fine but whose declaration moved, or whose rules version was bumped, or that gained a link since the last run — three silent ways to publish yesterday's answer. A company carrying several links is still one request. An outside-ТР answer is a decision, not an absence of one, so 404 and the empty 200 both write verdicts. Otherwise every run re-asks the register a question it has already answered — which at 5 requests per window is the expensive kind of mistake. A link whose evidence cannot be read is left WITHOUT a verdict rather than given a false one: an absent verdict re-pends the ЕИК next run, while a fabricated "unknown" would cache a hold nothing ever revisits. The test that keeps ADR-0037 honest asserts a co-owner who holds no public office appears in no column of the verdict row and nowhere in the cache file — against a verdict pinned to `document`/publishable/role=owner, so it cannot pass by degrading to a null "unknown". scripts lane: 271 tests pass.
…surface The decision pass no longer touches a deed — it cannot, since ADR-0037 has the crawler delete it the moment it has decided. evidenceVerdict and reconcileTermination are no longer imported here at all; load.mjs reads what they concluded, keyed on link_key and validated against rules_version and inputs_hash. reconcileTermination turned out to be a SECOND deed-dependent decision, and a dangerous one to lose: with no deed it falls to `terminated`, which WITHDRAWS a live link rather than holding it — a silent recall regression. It is now computed and cached beside the verdict. The coverage gate was all-or-nothing because a crawl was. Under the measured limiter (ADR-0036) that gate never opens at all, so it is scoped down to the case where refusing is the only protection there is: a first published run, where §8's monotonicity gate has no prior surface to compare against. Everywhere else the monotonicity gate takes the duty it already had — it hard-fails on a published claim that vanished, with the rules_version escape for a deliberate bump. Two gates for one duty was the redundancy. The sealed lookup date is now per link, from the verdict's own decided_at. A global MAX(fetched_at) was honest while a crawl was atomic; once it is incremental that would stamp this run's date onto a decision reached weeks ago and overstate the freshness the methodology page promises. One splitLinkRecord defines what a link record is, because the crawler and the loader hashing even slightly different objects would miss every cache entry and silently re-crawl the whole register — a five-hour bill, not an error. The unknown-input guard caught exactly that drift while wiring this up. It also caught a bug worth naming: inputsHash is a sha256 digest, and the ЕГН rail refused any digest carrying a standalone ten-digit run — ~7% of them, the same measurement that already exempts bodySha256. Unexempted it would have dropped one verdict in fourteen at random. Not a loud failure; a recall hole spread evenly across the surface. The load fixtures now run the REAL decideLinks over their deeds (tr-fixture.mjs) rather than hand-written verdict rows, so these tests still exercise the evidence ladder end to end. scripts lane: 273 tests pass. check:docs clean.
…register The verdict cache is restored and saved across runs, which is the change that makes the monthly job finishable at all: at 5 requests per window (ADR-0036) a crawl starting from `cached 0` cannot cover 537 links in one job, and until now every run started exactly there. Only scratch/tr/tr-cache.sqlite is cached — the raw deeds are never uploaded and are deleted as soon as the crawler has decided, so ADR-0033 decision 5's retention promise still covers everything it covered. Saved on always(), because a run cut short by the limiter or the runtime budget is precisely the one whose progress must survive, and also the one most likely to fail a later step. The crawl now takes --links-file and a --max-runtime-min (default 180), leaving room for resolve/audit/ship inside the 300-minute ceiling. Spending the budget is not a failure — the next run continues. The concurrency group drops the environment key. Two writes to different D1 slots cannot collide, but both crawl the SAME public register, so a staging and a production run in parallel doubled the effective request rate against it. The shared resource is the register, not the database. Also corrected three rationale comments that described the design as it was: the crawl does emit verdicts now, "decide without crawling" is a normal state rather than an impossible one, and a daily cadence is no longer blocked by the design — only by the cost to the register, which is a separate argument nobody has made. scripts lane: 273 tests pass. check:docs and prettier clean.
Test coverage
✅ No workspace dropped below its baseline (tolerance 0.5pp). 📈 Coverage rose by more than 1pp — run |
nikimilenkov
left a comment
nikimilenkov
left a comment
There was a problem hiding this comment.
Прегледах този PR на HEAD 25870a9 срещу main по строгия протокол: четири независими измерения (сигурност, дълбока логика, архитектура/ADR, тестове с мутации), всяка ключова находка проверена от мен лично в клонинг на клона, плюс 10 мутации, приложени и върнати една по една. Базовият клон е проверен отделно: main е пълен и свеж (носи целия ТР конвейер през squash acfcdce, midt-bg#309), merge-base е върхът на main — този PR не стъпва върху клона на PR #2 и няма скрити зависимости.
Първо честното: механиката на промяната е силна. Изстиването вместо спиране е вярната интерпретация на измерения лимит (180 s > ~176 s прозорец), дискриминацията 429/празен-200/5xx е структурна (RateLimitError се хвърля преди тялото изобщо да се гледа), актът се изтрива в мига, в който е решен, а rm -rf остава само подсигуряване. verdictInputsHash отказва непознат вход, recon_terminated в кеша е най-финият улов в целия PR (без него продаден дял щеше тихо да се чете като „прекратен" и да тегли жива връзка), а тестът, който сканира байтовете на sqlite файла за името на съдружника, е точният вид доказателство. Тестовият масив е бърз и детерминистичен — 273 теста за ~4 s, без нито едно реално чакане.
Проблемът не е в механиката, а в това, че новата инкрементална машина е пусната с параметри, при които не може да се сближи, и с една премахната защита, чиято обосновка не е оборена. Присъдата ми е връщане за промени (съветодателно; решението е на поддържащите): шест находки с висока тежест, всяка проверена механично.
Високи
1. Кешът не може да достигне пълно покритие при зададената каденца — централното твърдение на ADR-0037 се опровергава от собствените константи.
related-persons-data.yml:54 (cron: '0 3 1 * *'), :297 (MAX_AGE по подразбиране 30) и cache.mjs:341 (присъда по-стара от maxAgeDays не е актуална). Присъда от 1-ви число е на 30–31 дни при следващия ход — целият кеш се преизчаква всеки месец. По собствената аритметика на ADR-0036 (5 заявки / ~176 s) бюджет от 180 минути купува ~270 справки от 537 кандидата. Скоростта на опресняване е структурно под скоростта на остаряване: покритието осцилира около ~50% и никога не стига 1.0. „Студеният старт стига до пълно покритие за няколко хода" (0037:78-79) изисква каденца < max-age или кандидати ≤ бюджет на ход — нито едното не е вярно. Трите константи (каденца, --max-age-days, RETENTION_DAYS) вече образуват едно условие за сближаване и трябва да се решат като едно решение.
2. Опашката се подрежда лексикографски и гладува фиксирана опашка от ЕИК → пургата ги изтрива → монотонният гейт спира конвейера окончателно.
cache.mjs:374 връща [...out].sort() — по ЕИК, не по давност; fetch-deeds.mjs я консумира отпред. Всеки ход, ограничен от бюджета, обслужва същия префикс и никога не стига същия суфикс. Комбинирано с №1 и RETENTION_DAYS = 35: гладуваната опашка губи присъдите си на ~35-ия ден → тези връзки падат до „без доказателство" → изчезват от публикуваната повърхност при непроменен rules_version → audit.mjs ги брои за регресии и ходът се проваля твърдо. Режимът на отказ не е „временно по-малка повърхност" (0037:83-84), а „конвейерът спира да публикува след ~2–3 хода" — и гейтът, който стреля, е точно този, който ADR-0037 посочва като заместващата защита. Отделно: първият ход след сливането започва с 0% покритие срещу непразно priorPublished, така че CI се проваля ~2 месечни цикъла без никакъв път за наваксване. Подредба по давност (най-старата присъда първа) е минималната поправка.
3. Една нерешима връзка проваля целия месечен ход — завинаги; премахнатата per-link защита пазеше точно от това.
decideLinks умишлено не записва присъда при отказ (fetch-deeds.mjs:186-192 — правилно само по себе си), но fetch-deeds.mjs:470 брои отказа като unresolved → изход 1 → workflow-ът го разпространява (:319) → load.mjs изобщо не тръгва. Връзката се преизчаква всеки ход, изходът е 1 всеки ход, а от страната на обхождащия няма еквивалент на --allow-partial-tr. Един детерминистично неразбираем акт е перманентен пълен отказ. Изтритата обосновка в стария load.mjs го казваше дословно: „един малформиран акт от ~400 би решил съдбата на всяка друга връзка… превръщайки един лош payload в пълен отказ. Шумно, по връзка, fail-closed." Тя не е оборена — кодът, който пазеше, просто е преместен на място, където вече не пази.
4. RULES_VERSION стана носеща инварианта без никакъв механизъм зад себе си — и проверката ѝ откъм публикуващата страна е нетествана (оцеляла мутация).
evidence.mjs:38 е ръчно поддържан низ; единственият му тест е „непразен низ"; нула споменавания в workflow-ите. Преди този PR всеки ход преизвеждаше всяко решение от акта; сега това е единственият пазач срещу публикуване на решение, взето от по-стара версия на стълбата — забравен bump значи, че поправка в правилата не сваля фалшиво твърдение до ~35 дни. По-лошо: load.mjs преизписва проверката за актуалност на ръка на две места (:690 и :825) вместо да вика verdictIsCurrent, и изтрих двете сравнения rulesVersion === RULES_VERSION — всичките 273 теста минават (проверено лично, върнато). Това е последната fail-closed проверка преди публикуване на твърдения за реални хора. Кешовата страна е убита от тест (cache.test.mjs:253); публикуващата — не, и дублирането е причината убийството да не се пренася. Нужни са: тест, който сее присъда със стар rules_version и гледа отказа; една параметризирана функция вместо три копия; и обвързване на низа със съдържанието на стълбата (или поне CI проверка).
5. Затвореният речник не се налага на границата, която редът реално пресича.
upsertVerdict (cache.mjs) проверява само ЕИК и ЕГН-форма — няма isSealedFact, няма allowlist за kind/registry_role. Собствената фикстура на тестовете (cache.test.mjs:188) записва matchedFact: 'name' и той се приема. Гейтът при запечатване (load.mjs:975) е след факта: редът вече е прекосил в кеша, който сега пътува през actions/cache между ходове. Точно този ред е артефактът, за който схемата обещава „a ROLE, never the person" — обещанието трябва да се налага при записа, не само при четенето месец по-късно.
6. Каноничните документи твърдят премахнатото поведение като текущо — дословно.
docs/spec/related-persons-foundation.md:74: „a 429 is an instruction to stop, never a transient to retry through: the crawler ends the run, marks nothing" — всяка клауза вече е невярна, а това е документът, който ADR индексът сочи като носещ. docs/adr/0034:60-62 още твърди „ако обхождането спре при 429, гейтът за покритие отказва частичния кеш и ходът не публикува нищо" — точно поведението, което 0037 маха — без никакъв back-pointer (конвенцията Amended by: живее в пет други ADR-а, но не е в шаблона, и това е трето поредно пропускане). В самия workflow :100-102 още обещава „~20 минути отгоре", докато :301 задава 180; :24-34 твърди „every run that decides must also crawl" точно преди :36-39 да каже обратното. Читател, стигнал до 0034 или спецификацията, няма никакъв сигнал, че са стеснени.
Средни
- Студеният под се изключва от един-единствен остатъчен публикуван ред. Тригерът е
priorPublished.length === 0(load.mjs:709), не съотношение — един редstatus='published'от частично изпращане (или от директнияUPDATE, който runbook-ът за сваляне санкционира) изключва 95%-ния под изцяло, и монотонността пази само този един ред. Свързано: при bump на правилата подът не важи (не е студен старт), аdeclaredRemovalобезоръжава монотонността — прозорец, в който повърхността може да падне до--min-links 250от ~329 без нито един гейт да гръмне. - Възстановеният кеш се отваря без никаква проверка за цялост. Повреден restore →
openCacheхвърля → ходът се проваля →if: always()save записва същия повреден файл под по-нов ключ, който следващият ход предпочита. Самоувековечаваща се повреда без изход; липсваPRAGMA integrity_check+ „неизползваем restore → започни празен". Отделно WAL/-shm файловете не са в кеширания път, а:174-175и:321-322правят противоположни твърдения за това какво спасява save стъпката при твърд таван на заданието (300 мин срещу 180 мин само за ТР + пълното обхождане на реалните данни). - „Извън ТР" се маркира трайно на първия празен 200, а измерването документира два поредни. Коментарът на fetch-deeds.mjs:390-393 сам цитира „empty on two consecutive requests" като основанието; кодът маркира на първия. Общинско ЕИК, ударило единичен аномален празен отговор, става 30-дневен негатив. Или второ потвърждение, или корекция на коментара с обосновка защо статусът стига.
- Прозорец на срив оставя суров акт с имена на трети лица 35 дни на диска. Срив между
upsertVerdictиfs.rmSync(fetch-deeds.mjs:441→471) оставя акта под свежа присъда: следващият ход прескача ЕИК-то (присъдата е актуална), файлът не е сирак и не е изтекъл — седи пълния период. Нито тест, нито метла го покриват; метлата на пургата освен това филтрира по.jsonи прескача.tmpсираците наatomicWrite. - Бюджетът на изстиванията е непинат — тавтология в тестовете (оцеляла мутация).
MAX_COOLDOWNS3→1 и 3→100 минават всичките 273 теста, защото всяко твърдение следи внесената константа; същото заRATE_LIMIT_COOLDOWN_MS(нищо не твърди, че покрива измерените ~161 s). За контрастMIN_INTERVAL_MSе пинат правилно (>= 3000). Един директен пин на стойностите решава и двете. - Двата теста на гейта зависят от реда на изпълнение през споделената база (
load.test.mjs:1191-1231— единият триеinterest_linksи възстановява след себе си, другият разчита на предходенrunLoad()). Разместване или паралелизъм обръща коя клонка се упражнява — тихо. Най-високорисковото поведение в PR-а е пинато с най-крехкия наличен механизъм.
Ниски
short_name/latin_in_name се записват и никога не се четат (мъртви колони с тест, който твърди „the refusal is counted" — нищо не брои); verdictCoverage е мъртъв импорт във fetch-deeds.mjs:44; seedVerdicts в новата фикстура презаписва с NULL колоните на deeds, които четирите тестови файла ръчно попълват (безвредно днес, капан утре); отказан upsertVerdict оставя суровия акт на диска; ADR-0036 забранява бъдещо опипване на прага, без да записва процедура за следващото премерване, което сам предвижда (:85-86); проверката на бюджета е преди ЕИК-то, така че едно ЕИК надхвърля тавана с до ~10 мин (ограничено и тествано с контролно рамо — само за документиране).
Какво изпълних и проверих лично
- Пълния масив: cacbg+tr 273/273 (~4 s), plain 91/91, ingest 84/84, db 486/487 (единственият провал е познатият env артефакт — тестът извиква
pnpmбез пълен път; не е от този PR). - 10 мутации, 8 убити: брояч на изстиванията глобален вместо per-ЕИК — хванат (4 теста, вкл. „нулира се при успех"); застой по време на изстиване получава пълните 5 опита — хванат (пинът 4-срещу-16 заявки); бюджетът проверяван след ЕИК вместо преди — хванат (двойката с контролното рамо); 429 → провал вместо изстиване — хванат (5 теста); hash сляп за
declaredSeats— хванат (вкл. end-to-end „променена декларация преизчаква своето ЕИК");verdictIsCurrentбезinputsHash/ безrulesVersion— хванати; изтрит студен под — хванат. Оцелели: №4 (loader-siderules_version, повторено от мен) и №5 (тавтологията наMAX_COOLDOWNS). - Веригите на високите: константите на каденцата/възрастта/пургата,
[...out].sort(), пътя отказ→изход 1→workflow, липсата на allowlist вupsertVerdict(фикстурата с'name'се приема), недокоснатите 0033/0034/спецификация в диффа. - Чистото: 429 никога не става
outside_trред (тест на :202); PII байт-сканът не може да мине на празно (броят/kind/role са пинати първо);load.mjsлогва ЕИК, никогаlink_key(ключът носи име); глобалната конкурентност е правилно обоснована (споделеният ресурс е регистърът); изтриването на актовете преживява по всички пътища.
Приоритет на поправките
(1) Каденца/max-age/retention като едно решение — или направете твърдението на 0037 вярно, или го оттеглете; (2) per-link fail-closed за нерешима връзка вместо изход 1 за целия ход; (3) опашка по давност, не по ЕИК; (4) тест + една функция + механизъм за RULES_VERSION; (5) isSealedFact/allowlist в upsertVerdict; (6) спецификация §3.3, ADR-0034 back-pointer, коментарите в workflow-а — и полето Amended by: в шаблона, за да спре да се пропуска; после средните.
Механиката е готова; параметрите и документите не са ѝ настигнали. С №1–3 решени този конвейер наистина ще прави това, което ADR-0037 обещава.
…e the review's gaps Review on #3 found that the machine shipped with parameters it could never converge under. Verified independently rather than taken on trust: at 5 requests per ~195s cooldown cycle a 180-minute budget buys ~277 lookups against 537 candidates, and with --max-age-days 30 on a monthly cron the whole cache expires between runs. Coverage oscillates near 50% and never reaches the 95% floor, so the pipeline would never have published at all. Worse, the queue sorted lexicographically by ЕИК while the crawl consumes from the front, so it was always the SAME ~277 — the tail never decided, then purged, then counted as a monotonicity regression. Convergence is now one condition rather than three unrelated constants: queue never-decided first, then oldest decided_at — a bounded run drains a rotating set instead of re-serving one prefix retention split from the deeds' 35 days into its own 45. The deed number is a privacy obligation (ADR-0033 decision 5); the verdict number is a freshness policy, and sharing one constant deleted verdicts faster than a budget-bounded crawl could refresh them. The cadence half lands in the workflow commit. A refused LINK no longer counts as an unreachable ЕИК. It did, which made one deterministically unparseable deed exit 1 → the workflow stop before load.mjs → the same failure every run, for ever. The old load.mjs said exactly this before the code moved; the protection did not move with it. The loader hand-rolled the verdict-currency check in two places instead of calling verdictIsCurrent. Both copies could be deleted with all 273 tests green — on the last fail-closed check before publishing a claim about a named person. One definition now, and a test that seeds an older rules_version and watches the run refuse. upsertVerdict enforces the closed vocabulary at the boundary the row actually crosses. The seal gate in load.mjs runs a month later, on a row that has already travelled between runs; the schema's promise of "a ROLE, never the person" was checked only by the eventual reader. The fixture, meanwhile, stored matchedFact: 'name' — a value production cannot emit. The verdict floor applies to every run, not only a first one. Keyed on "has anything ever been published", a single leftover row switched a 95% floor off entirely. Affordable because the loader's currency test ignores AGE deliberately: a verdict past its refresh window is still publishable, so steady state is ~100% and the two ways to fall below are the two where refusing is right. Also: a corrupt restored cache is quarantined instead of being re-saved under a newer key and preferred for ever; an empty-200 negative needs the second observation its own measurement cites, and stays pending until it gets one; the cooldown constants are pinned rather than tautologically followed; short_name and latin_in_name are reported instead of written and never read; .tmp orphans are swept; the dead verdictCoverage import is gone; and the one test that mutated shared state to make its point is replaced by a stronger one that does not. Mutation-checked: reverting the queue order, and deleting the rules_version comparison, each now fail tests that previously passed. scripts lane: 285 tests pass.
…code again The cadence is the other half of the convergence fix. Monthly, every verdict expired between runs, so one run faced all ~537 candidates and its budget bought ~277 — coverage oscillated near half and never crossed the floor. Weekly does not ask the register for more: a link comes due once its verdict passes --max-age-days either way, so the register still sees each company about monthly. Only the bunching changes — ~135 due per run against ~277 capacity, with headroom for a cold start to finish in two. That reasoning is now in ADR-0037 as an explicit convergence condition with the arithmetic, replacing the claim it made without checking: "cold start reaches full coverage in a few runs" was false under the constants it shipped with. The condition is pinned by a test rather than asserted in prose — its absence is exactly how a non-converging design got this far. Three documents asserted behaviour the code no longer has: - docs/spec/related-persons-foundation.md still recorded Phase-0's "the block is sustained… the crawler ends the run, marks nothing". Every clause of that is measured false; it now states the re-measured behaviour and points at ADR-0036, and the retention bullet reflects that deeds are no longer written to disk at all on the decision path. - ADR-0034 still said the coverage gate refuses a partial cache. It carries a status-line back-pointer and an in-place note now, and its index row says it is amended — the convention the reviewer noted was being skipped for the third time. - The workflow promised "~20 minutes" for a pass now bounded by a 180-minute wall-clock budget, and asserted "every run that decides must also crawl" a few lines above the note saying the opposite. Not adopted, and argued rather than skipped: an `Amended by:` field in the ADR template (a repo-wide convention change, so its own change), a CI mechanism binding RULES_VERSION to the ladder's content (machinery guarding a one-line discipline, and it would fire on comment edits), and a re-measurement procedure in ADR-0036 (process documentation for a hypothetical, in a record whose job is to state what was measured). scripts lane: 285 pass. pnpm test: 6/6. check:docs and prettier clean.
|
@nikimilenkov — прегледът е верен, включително по най-важното. Проверих сам всяка находка с висока тежест, вместо да ги приема на доверие, и преизведох аритметиката на №1 от собствения си жив ход, а не от твоите числа: 5 заявки на цикъл от ~195 s → ~92 справки на час → ~277 за 180-минутен бюджет срещу 537 кандидата. При месечна каденца и Ето какво влезе ( Високи — всичките шест1 + 2 (сходимост и подредба). Решени заедно, като едно условие, както настоя:
Ключовото за ADR-0034: седмично не е повече трафик. Отхвърлянето на ежедневната каденца там беше за обем (~400 дневно); при Първата ми версия на теста за подредбата не хващаше мутацията — при студен кеш всички са еднакво нерешени, тъй че tie-break-ът по ЕИК съвпада с лексикографския. Преместих твърдението на ниво 3. Отказана връзка вече не се брои за недостижим ЕИК. Изтритият коментар на стария 4. 5. 6. Спецификацията §Phase-0, ADR-0034 (back-pointer в статуса, в тялото и в индекса) и трите противоречиви коментара в workflow-а. Средни — всичките шестСтуденият под важи вече за всеки ход, не само за първия (тригерът по М4 реших по-строго от предложеното: в links режим суровият акт изобщо не стига до диска. Разборът е от тялото на отговора и никой друг не чете файла, тъй че прозорецът се затваря, вместо да се скъсява. НискиМъртвият Три неща, които не приемам — и защо
Ако не си съгласен по някое от трите, кажи — аргументът ми е за пропорционалност, не за обхват. 285 теста в |
nikimilenkov
left a comment
nikimilenkov
left a comment
There was a problem hiding this comment.
Прегледах двата нови commit-а до 728b39d със същата дисциплина: изпълних повторно цялата мутационна батерия срещу новия HEAD, пробвах граничните случаи директно през реалните функции и проследих всяка от дванадесетте находки до кода ѝ. Всичко от прегледа е адресирано; вдигам „връщане за промени" — от моя страна това е одобрение (съветодателно; решението е на поддържащите).
Повторени мутации — оцелелите вече падат
| Мутация | При прегледа | На 728b39d |
|---|---|---|
load.mjs губи сравнението по rules_version |
оцеляваше (273/273 зелени) | пада на две нива — „a verdict from an OLDER rules version is held, never published" + кешовия unit тест; двете места викат verdictIsCurrent, преписите са изтрити |
MAX_COOLDOWNS 3→1 |
оцеляваше (тавтология) | пада — „the cooldown constants are pinned to the measurement, not merely imported"; и RATE_LIMIT_COOLDOWN_MS >= 161_000 е пинат |
опашката обратно на [...out].sort() (лексикографска) |
n/a — новият код | пада — „pendingVerdictEiks rotates by staleness — oldest first, never-decided ahead of all" |
Високите — всичките шест, проверени механично
- 1+2 (пълно покритие): честотата на ходовете е седмична (
0 3 * * 1), задържането на присъдите е отделено на 45 дни от 35-те на актовете, опашката се върти по давност с детерминистичен tie-break по ЕИК. Условието е записано като два теста, не като абзац: end-to-end (9 връзки при бюджет 2 стигат до пълно покритие за ≤6 хода, всичко решено, а улегнал кеш прави нула заявки към регистъра) и тест върху самите константи (задържането > max-age + един интервал между ходовете,max-age > интервала). Изменението на ADR-0037 записва грешката честно, със собствената преизведена аритметика. Точката за ADR-0034 е вярна: приmax-age 30общият трафик не се променя, мени се само разпределението му. - 3: отказана връзка отива в
undecided— шумно, по връзка, изход 0, останалото на хода стои; върнатата обосновка стои дословно в коментара на кода, където ѝ е мястото. - 4: проверено с изтриване на сравнението — два теста почервеняват (виж таблицата).
- 5: пробвах
upsertVerdictдиректно с четири входа:matchedFact: 'name'→ отказ, буквално име → отказ, име вregistry_role→ отказ, валиденrole:owner:CR_F_19_L→ приет. Плюс allowlist заkind(седемте стъпала на стълбата) и тест, който пина отказа. Фикстурата вече издава само форми, коитоevidenceVerdictреално произвежда. - 6: спецификацията §Phase-0 е пренаписана с новото измерване и сочи ADR-0036 като заместващ запис (вкл. „§3.3 still governs — the threshold is not to be probed further"); ADR-0034 носи back-pointer в статуса си; трите противоречиви коментара в workflow-а вече разказват една история (300-те минути срещу 180-те са обяснени, не в конфликт).
Средните — всичките шест
Подът от 95% важи за всеки ход и е пинат с точно теста, който липсваше („refuses EVEN WITH a prior published surface" — един остатъчен публикуван ред вече не го изключва); повреден кеш се проверява с PRAGMA integrity_check + проба по схемата, карантинира се настрани и ходът тръгва празен — шумно, с губене на прогрес вместо на конвейера; празният 200 иска второ наблюдение (outside_tr_pending, което не пише присъда, тъй че повторното питане е структурно гарантирано, а unambiguous е документираният изход за статуси, които не искат потвърждение) — и благодаря за честността, че поправката извади втория пропуск с pendingEiks наяве; М4 наистина е решена по-строго от предложеното ми — в links режим актът се разбира от тялото на отговора и никога не докосва диска (rawPath: null), тъй че прозорецът не е скъсен, а премахнат; двете константи на изстиването са пинати; двойката тестове на гейта е заменена и споделеното състояние (DELETE FROM interest_links + възстановяващият runLoad()) е изчезнало.
Ниските също: .tmp-<pid> сираците са в метлата, мъртвият внос го няма, short_name/latin_in_name се отчитат в телеметрията на обхода, фикстурата не занулява колони.
Трите отказа — приемам и трите
RULES_VERSIONбез машинария: съгласен. Исканото по същество беше тестът и края на дублирането — и двете влязоха, и мутацията вече пада на две нива. Извеждане на „промени ли се стълбата" от съдържание на файл наистина би гърмяло при редакция на коментар; дисциплината около еднолинейния bump вече има тест, който я прави видима, а това е реалната защита.Amended by:в шаблона като отделна промяна: правилно скопиране — този PR дължеше конкретните back-pointer-и и ги достави.- Процедурата за премерване в ADR-0036: пропорционалността печели; записът вече казва кое число се преглежда и това стига.
Остатъчни бележки (нищо блокиращо)
- Първите 1–2 седмични хода след сливането ще са червени на пода от 95%, докато покритието се навакса — fail-closed, самоизлекуващо се, сервираната повърхност не се пипа. Очаквано поведение, казвам го само за да не изненада никого в Actions.
- WAL/-shm файловете още не са в кеширания path — но с карантината това вече е само „загубен прогрес от последния checkpoint" при ненормална смърт на процеса, което resumability-то поема. Ниска, по преценка.
- Тестът на константите пина
CADENCE_DAYS = 7като огледало на cron-а (тестът не може да чете YAML) — ако някой върне честотата на ходовете назад, огледалото трябва да се смени на ръка. Коментарът в теста го казва; само отбелязвам връзката.
Изпълнено тук: cacbg+tr 285/285 (~4.5 s), plain лентата 91/91, дървото чисто след всяка мутация.
Образцов цикъл на поправка: аритметиката преизведена независимо вместо приета на доверие, условието за пълно покритие превърнато в тест, една от поправките по-строга от исканото, и три отказа, аргументирани с пропорционалност вместо замълчани. Веригата №1+№2 беше уловът на този преглед, а признанието „конвейерът, който изпратих, нямаше да публикува никога" е точно видът честност, върху който се гради доверие в този слой. Благодаря!
…ng the old design Second review (cefothe, midt-bg#323) — APPROVE, security clean, three low nits. All three verified real, plus a fourth found next to one of them. purgeExpired has returned a verdict count since verdicts got their own retention window, but the log condition never checked it. A purged verdict DROPS coverage, and coverage is what the 95% floor refuses on — so the one event that best explains a surprising refusal next run was the one event that never printed. The two coverage lines in load.mjs sit next to each other and count different things: `TR cache:` was ЕИК-denominated, `TR verdicts:` link-denominated, and the refusal that follows is about links. Reading "537/537 covered" immediately above "REFUSE TO LOAD" invites exactly the wrong conclusion. Now `TR deeds:` and `TR verdicts:`. Two comments still described the design this PR replaced: cache.mjs's header said a 429 ends the run — the model ADR-0036 overturns, in a file this PR rewrites — and the purge comment still credited a monthly refresh for keeping rows inside their window. No behaviour changes beyond the added log line. scripts lane: 285 pass. check:docs and prettier clean.
Third review (cefothe, midt-bg#323) — APPROVE, security clean, two low nits plus three branches listed as informational coverage gaps. Both nits verified against the code before being taken. One of the three gaps cannot honestly be tested, and now says so instead. load.mjs opened the verdict cache and closed it on one exit of three. The verdict-floor refusal closed the pair; the suppression-gate throw and the success path closed only `db`. Nothing observable followed — the workflow saves the cache BEFORE load.mjs runs, so an uncheckpointed WAL cannot reach the saved artifact — but this file closes `db` explicitly before every exit it has, and an intent kept on two paths out of three is worse than either alternative. Not the reviewer's try/finally: it would reindent ~520 untouched lines for a low nit. purgeExpired counted the verdicts it was about to delete with a SELECT COUNT(*), then deleted them. The reviewer's stated reason does not apply here — DatabaseSync is single-process and synchronous, and the workflow serialises runs against the register — but the count was already available from the DELETE. Two statements asking one question can only agree or be wrong; one statement cannot disagree with itself. Three tests for branches nothing exercised, each mutation-checked rather than merely green: openCache's quarantine of a corrupt restore, and of a file that is valid sqlite but not this cache — only the table probe catches the second, since integrity_check passes it — and the idempotent ALTER TABLE migration, whose load-bearing claim is that a restored older cache keeps its rows. That older shape is synthesised by dropping the two columns from the current schema, so the fixture cannot drift from SCHEMA when a column is next added. The third gap gets a comment, not a test. `decided === 0 && refused === 0` is unreachable: readLinksFile normalises every link.eik through safeEik, and the queue is derived from those same links, so every ЕИК reaching it has a link that matches exactly. A test would have to fabricate a state the wiring cannot produce. It is not dead code either — the queue compares safeEik(link.eik) where decideLinks compares link.eik raw — so it stays, with the reason written down. No behaviour changes. scripts lane: 288 pass; plain lane 91. check:docs and prettier clean.
…ода (#323) * docs(adr): re-measure the ТР rate limiter and record what it rules out ADR-0033 context 4 recorded the limiter as ~50 cumulative requests followed by a sustained block, and decision 7's crawl hygiene ("a 429 is an instruction to STOP") rests on that. Re-measured on 2026-08-19 through client.mjs's own httpsGet — the production transport, same UA, same headers, so the result transfers: budget 5 requests; the 6th returns 429 scope IP, not session — a fresh EPZEUSessionID is refused too shape 0 bytes, no Retry-After, no X-RateLimit-*, 48-67ms vs 250-475ms for a real deed, so refused at an edge before the application second face while blocked the connection may STALL (20s timeout), not 429 recovery still blocked at +60s, clear at +161s CI run 32154394308 fails identically — at request 6, `TR cache: 5/537 covered` — so the threshold is not a property of GitHub's runners. Both halves of the old premise are false: the trip point is 5, not ~50, and the block is not sustained. That changes what the design can achieve: 537 candidates at 5 per ~176s cycle is ~5.3h against timeout-minutes: 300, and since the deed cache deliberately does not survive the runner, every run restarts at `cached 0`. No pacing value completes 537 lookups in one job. ADR-0036 records the measurements and rules that a 429 is a cooldown, not a stop. ADR-0037 takes the way out that ADR-0034 named and left on the table: the crawl emits per-(link, ЕИК) verdicts — a role, an entry reference and booleans, no third-party name — so only those cross a run boundary and progress becomes durable. client.mjs keeps its behaviour exactly: it still reports the 429 and never folds it into the retry budget. Only the justification changes, and the wait-and-resume policy lands in fetch-deeds.mjs, where "how long to wait" belongs. No code paths change in this commit. * fix(tr): wait out the register's cooldown instead of ending the run on one 429 The block clears in ~161s (ADR-0036), so ending a whole crawl on the first 429 threw away the run for a wall that was about to come down on its own. In CI that meant five deeds out of 537, every month. `fetchOne` now waits RATE_LIMIT_COOLDOWN_MS (180s) and re-requests the SAME ЕИК, recording nothing about it in between — the block is a fact about us, not about that company. Three cooldowns without a success still end the run with exit 2, so a genuinely sustained block stays distinguishable from the ordinary 5-per-window rhythm, and the counter resets on any success so a slow healthy crawl never approaches it. The block has a second face: while it holds, the connection stalls rather than answering 429. Before the first cooldown we cannot know we are blocked, so a network fault keeps the documented TRIES_PER_EIK budget; once we do know, further attempts get exactly one try and any throw reads as "still blocked". Otherwise each stall costs five 20s attempts fed into a tarpit — the new test pins the difference at 4 requests rather than 16. `--max-runtime-min` lets a run stop cleanly between two ЕИК when its wall-clock budget is spent. Candidates never attempted are not counted as unresolved, but a candidate we asked about and lost still exits 1 — a deadline must not launder a real failure into a green run. This is only safe because progress is cached; the verdict cache that makes it safe across runs lands next (ADR-0037). politeTrGet is untouched. It still reports the 429 and never folds it into the retry budget; only who decides how long to wait has moved. scripts/tr: 141 tests pass, 255 across the whole scripts lane. * feat(tr): store the decision, so progress can outlive the deed that produced it The deed cannot cross a run boundary — it carries the names of co-owners and managers who hold no public office (ADR-0033 decision 5) — so every run has been restarting at `cached 0` and re-spending the register's budget on the same first few ЕИК. The decision that deed produces carries none of that: a kind, a role, an entry reference and booleans. ADR-0037; the `verdicts` table is where it lives. `verdictInputsHash` is the invalidation, and it refuses an evidenceVerdict argument it does not recognise rather than hashing a subset. The failure mode of a missed input is silent — a stale decision about a real person, published — so adding an input now fails the run until someone decides where it belongs. Arrays are sorted before hashing because declaredSeats arrives from a Set spread, and insertion order must not read as a change. A verdict is stale three ways, each of which must re-decide: the rules moved (RULES_VERSION), the declaration moved (inputs_hash), or the lookup simply aged. Coverage and pending are computed over LINKS rather than ЕИК, since one company carries several and a rules bump invalidates them independently of when its deed was last seen. The ЕГН rail screens verdict rows too — this is the row that crosses a boundary, which makes it the surface most worth screening, not least. purgeExpired ages verdicts on the same clock: not as a privacy rail here, but because a published claim must not outlive the lookup it rests on. ADR-0037 said the family-scope link key might carry the relative's name and would be hashed if so. Verified during implementation: personId is built from the DECLARANT, who is the official even for a family-scope stake, and the relative is never named. Replaced the conditional with the checked fact. scripts/tr: 150 tests pass. * feat(tr): the crawl decides while the deed is in hand, and drops it immediately `--links-file` carries the declaration side of each decision, so the crawler can compute evidenceVerdict at the moment it holds the deed rather than handing the deed onward to a decision step that must not receive it. The raw body is deleted as soon as it has served its purpose; the end-of-job rm -rf stays as the backstop but no longer has to be the only thing standing between a failure and a deed left on disk. `--eiks-file` still works and still only fetches. The two are mutually exclusive: one names companies, the other names decisions, and quietly treating a links file as a candidate list would fetch deeds nothing ever decides. The pending set is computed over LINKS in links mode. Deed freshness alone would skip a company whose deed is fine but whose declaration moved, or whose rules version was bumped, or that gained a link since the last run — three silent ways to publish yesterday's answer. A company carrying several links is still one request. An outside-ТР answer is a decision, not an absence of one, so 404 and the empty 200 both write verdicts. Otherwise every run re-asks the register a question it has already answered — which at 5 requests per window is the expensive kind of mistake. A link whose evidence cannot be read is left WITHOUT a verdict rather than given a false one: an absent verdict re-pends the ЕИК next run, while a fabricated "unknown" would cache a hold nothing ever revisits. The test that keeps ADR-0037 honest asserts a co-owner who holds no public office appears in no column of the verdict row and nowhere in the cache file — against a verdict pinned to `document`/publishable/role=owner, so it cannot pass by degrading to a null "unknown". scripts lane: 271 tests pass. * feat(cacbg): read the cached verdict, and let monotonicity guard the surface The decision pass no longer touches a deed — it cannot, since ADR-0037 has the crawler delete it the moment it has decided. evidenceVerdict and reconcileTermination are no longer imported here at all; load.mjs reads what they concluded, keyed on link_key and validated against rules_version and inputs_hash. reconcileTermination turned out to be a SECOND deed-dependent decision, and a dangerous one to lose: with no deed it falls to `terminated`, which WITHDRAWS a live link rather than holding it — a silent recall regression. It is now computed and cached beside the verdict. The coverage gate was all-or-nothing because a crawl was. Under the measured limiter (ADR-0036) that gate never opens at all, so it is scoped down to the case where refusing is the only protection there is: a first published run, where §8's monotonicity gate has no prior surface to compare against. Everywhere else the monotonicity gate takes the duty it already had — it hard-fails on a published claim that vanished, with the rules_version escape for a deliberate bump. Two gates for one duty was the redundancy. The sealed lookup date is now per link, from the verdict's own decided_at. A global MAX(fetched_at) was honest while a crawl was atomic; once it is incremental that would stamp this run's date onto a decision reached weeks ago and overstate the freshness the methodology page promises. One splitLinkRecord defines what a link record is, because the crawler and the loader hashing even slightly different objects would miss every cache entry and silently re-crawl the whole register — a five-hour bill, not an error. The unknown-input guard caught exactly that drift while wiring this up. It also caught a bug worth naming: inputsHash is a sha256 digest, and the ЕГН rail refused any digest carrying a standalone ten-digit run — ~7% of them, the same measurement that already exempts bodySha256. Unexempted it would have dropped one verdict in fourteen at random. Not a loud failure; a recall hole spread evenly across the surface. The load fixtures now run the REAL decideLinks over their deeds (tr-fixture.mjs) rather than hand-written verdict rows, so these tests still exercise the evidence ladder end to end. scripts lane: 273 tests pass. check:docs clean. * ci(related-persons): cache the verdicts, budget the crawl, share the register The verdict cache is restored and saved across runs, which is the change that makes the monthly job finishable at all: at 5 requests per window (ADR-0036) a crawl starting from `cached 0` cannot cover 537 links in one job, and until now every run started exactly there. Only scratch/tr/tr-cache.sqlite is cached — the raw deeds are never uploaded and are deleted as soon as the crawler has decided, so ADR-0033 decision 5's retention promise still covers everything it covered. Saved on always(), because a run cut short by the limiter or the runtime budget is precisely the one whose progress must survive, and also the one most likely to fail a later step. The crawl now takes --links-file and a --max-runtime-min (default 180), leaving room for resolve/audit/ship inside the 300-minute ceiling. Spending the budget is not a failure — the next run continues. The concurrency group drops the environment key. Two writes to different D1 slots cannot collide, but both crawl the SAME public register, so a staging and a production run in parallel doubled the effective request rate against it. The shared resource is the register, not the database. Also corrected three rationale comments that described the design as it was: the crawl does emit verdicts now, "decide without crawling" is a normal state rather than an impossible one, and a daily cadence is no longer blocked by the design — only by the cost to the register, which is a separate argument nobody has made. scripts lane: 273 tests pass. check:docs and prettier clean. * fix(tr,cacbg): make the incremental crawl actually converge, and close the review's gaps Review on ydimitrof#3 found that the machine shipped with parameters it could never converge under. Verified independently rather than taken on trust: at 5 requests per ~195s cooldown cycle a 180-minute budget buys ~277 lookups against 537 candidates, and with --max-age-days 30 on a monthly cron the whole cache expires between runs. Coverage oscillates near 50% and never reaches the 95% floor, so the pipeline would never have published at all. Worse, the queue sorted lexicographically by ЕИК while the crawl consumes from the front, so it was always the SAME ~277 — the tail never decided, then purged, then counted as a monotonicity regression. Convergence is now one condition rather than three unrelated constants: queue never-decided first, then oldest decided_at — a bounded run drains a rotating set instead of re-serving one prefix retention split from the deeds' 35 days into its own 45. The deed number is a privacy obligation (ADR-0033 decision 5); the verdict number is a freshness policy, and sharing one constant deleted verdicts faster than a budget-bounded crawl could refresh them. The cadence half lands in the workflow commit. A refused LINK no longer counts as an unreachable ЕИК. It did, which made one deterministically unparseable deed exit 1 → the workflow stop before load.mjs → the same failure every run, for ever. The old load.mjs said exactly this before the code moved; the protection did not move with it. The loader hand-rolled the verdict-currency check in two places instead of calling verdictIsCurrent. Both copies could be deleted with all 273 tests green — on the last fail-closed check before publishing a claim about a named person. One definition now, and a test that seeds an older rules_version and watches the run refuse. upsertVerdict enforces the closed vocabulary at the boundary the row actually crosses. The seal gate in load.mjs runs a month later, on a row that has already travelled between runs; the schema's promise of "a ROLE, never the person" was checked only by the eventual reader. The fixture, meanwhile, stored matchedFact: 'name' — a value production cannot emit. The verdict floor applies to every run, not only a first one. Keyed on "has anything ever been published", a single leftover row switched a 95% floor off entirely. Affordable because the loader's currency test ignores AGE deliberately: a verdict past its refresh window is still publishable, so steady state is ~100% and the two ways to fall below are the two where refusing is right. Also: a corrupt restored cache is quarantined instead of being re-saved under a newer key and preferred for ever; an empty-200 negative needs the second observation its own measurement cites, and stays pending until it gets one; the cooldown constants are pinned rather than tautologically followed; short_name and latin_in_name are reported instead of written and never read; .tmp orphans are swept; the dead verdictCoverage import is gone; and the one test that mutated shared state to make its point is replaced by a stronger one that does not. Mutation-checked: reverting the queue order, and deleting the rules_version comparison, each now fail tests that previously passed. scripts lane: 285 tests pass. * ci,docs(related-persons): weekly cadence, and records that match the code again The cadence is the other half of the convergence fix. Monthly, every verdict expired between runs, so one run faced all ~537 candidates and its budget bought ~277 — coverage oscillated near half and never crossed the floor. Weekly does not ask the register for more: a link comes due once its verdict passes --max-age-days either way, so the register still sees each company about monthly. Only the bunching changes — ~135 due per run against ~277 capacity, with headroom for a cold start to finish in two. That reasoning is now in ADR-0037 as an explicit convergence condition with the arithmetic, replacing the claim it made without checking: "cold start reaches full coverage in a few runs" was false under the constants it shipped with. The condition is pinned by a test rather than asserted in prose — its absence is exactly how a non-converging design got this far. Three documents asserted behaviour the code no longer has: - docs/spec/related-persons-foundation.md still recorded Phase-0's "the block is sustained… the crawler ends the run, marks nothing". Every clause of that is measured false; it now states the re-measured behaviour and points at ADR-0036, and the retention bullet reflects that deeds are no longer written to disk at all on the decision path. - ADR-0034 still said the coverage gate refuses a partial cache. It carries a status-line back-pointer and an in-place note now, and its index row says it is amended — the convention the reviewer noted was being skipped for the third time. - The workflow promised "~20 minutes" for a pass now bounded by a 180-minute wall-clock budget, and asserted "every run that decides must also crawl" a few lines above the note saying the opposite. Not adopted, and argued rather than skipped: an `Amended by:` field in the ADR template (a repo-wide convention change, so its own change), a CI mechanism binding RULES_VERSION to the ladder's content (machinery guarding a one-line discipline, and it would fire on comment edits), and a re-measurement procedure in ADR-0036 (process documentation for a hypothetical, in a record whose job is to state what was measured). scripts lane: 285 pass. pnpm test: 6/6. check:docs and prettier clean. * fix(tr,cacbg): report a verdict purge, and stop two comments describing the old design Second review (cefothe, #323) — APPROVE, security clean, three low nits. All three verified real, plus a fourth found next to one of them. purgeExpired has returned a verdict count since verdicts got their own retention window, but the log condition never checked it. A purged verdict DROPS coverage, and coverage is what the 95% floor refuses on — so the one event that best explains a surprising refusal next run was the one event that never printed. The two coverage lines in load.mjs sit next to each other and count different things: `TR cache:` was ЕИК-denominated, `TR verdicts:` link-denominated, and the refusal that follows is about links. Reading "537/537 covered" immediately above "REFUSE TO LOAD" invites exactly the wrong conclusion. Now `TR deeds:` and `TR verdicts:`. Two comments still described the design this PR replaced: cache.mjs's header said a 429 ends the run — the model ADR-0036 overturns, in a file this PR rewrites — and the purge comment still credited a monthly refresh for keeping rows inside their window. No behaviour changes beyond the added log line. scripts lane: 285 pass. check:docs and prettier clean. * fix(tr,cacbg): close both handles, and count a purge off its own delete Third review (cefothe, #323) — APPROVE, security clean, two low nits plus three branches listed as informational coverage gaps. Both nits verified against the code before being taken. One of the three gaps cannot honestly be tested, and now says so instead. load.mjs opened the verdict cache and closed it on one exit of three. The verdict-floor refusal closed the pair; the suppression-gate throw and the success path closed only `db`. Nothing observable followed — the workflow saves the cache BEFORE load.mjs runs, so an uncheckpointed WAL cannot reach the saved artifact — but this file closes `db` explicitly before every exit it has, and an intent kept on two paths out of three is worse than either alternative. Not the reviewer's try/finally: it would reindent ~520 untouched lines for a low nit. purgeExpired counted the verdicts it was about to delete with a SELECT COUNT(*), then deleted them. The reviewer's stated reason does not apply here — DatabaseSync is single-process and synchronous, and the workflow serialises runs against the register — but the count was already available from the DELETE. Two statements asking one question can only agree or be wrong; one statement cannot disagree with itself. Three tests for branches nothing exercised, each mutation-checked rather than merely green: openCache's quarantine of a corrupt restore, and of a file that is valid sqlite but not this cache — only the table probe catches the second, since integrity_check passes it — and the idempotent ALTER TABLE migration, whose load-bearing claim is that a restored older cache keeps its rows. That older shape is synthesised by dropping the two columns from the current schema, so the fixture cannot drift from SCHEMA when a column is next added. The third gap gets a comment, not a test. `decided === 0 && refused === 0` is unreachable: readLinksFile normalises every link.eik through safeEik, and the queue is derived from those same links, so every ЕИК reaching it has a link that matches exactly. A test would have to fabricate a state the wiring cannot produce. It is not dead code either — the queue compares safeEik(link.eik) where decideLinks compares link.eik raw — so it stays, with the reason written down. No behaviour changes. scripts lane: 288 pass; plain lane 91. check:docs and prettier clean.
2 participants
Какво
Конвейерът за свързани лица не е завършвал ход в CI нито веднъж, откакто ADR-0033 влезе. Причината е премерена, не предположена — и е двойна: ограничителят на ТР се е затегнал далеч под това, което ADR-0033 записа, а дизайнът не може да напредва на части.
Closes midt-bg#279 (регистърната част). Заменя контекст 4 и хигиената на решение 7 на ADR-0033, и твърдението „всеки ход, който решава, трябва и да обхожда" на ADR-0034.
Находката
Премерено на 2026-08-19 през
httpsGetнаscripts/tr/client.mjs— тоест през самия производствен транспорт:EPZEUSessionIDсъщо получава 429Retry-After, безX-RateLimit-*; 48–67 ms срещу 250–475 ms за истински акт → отказан на ръбаИзпълнение
32154394308пада идентично — на шестата заявка,TR cache: 5/537 covered. Прагът не е особеност на изпълнителя на GitHub.ADR-0033 записа „429 при ~50 натрупани заявки" и траен блок. И двете вече не са верни: прагът е 5, а блокът се чисти сам за под три минути. Оттам: 537 кандидата ÷ 5 на прозорец × ~176 s ≈ 5,3 часа при
timeout-minutes: 300— а понеже кешът не преживява изпълнителя, всеки ход тръгва отcached 0. Незавършимо при какъвто и да е темп.Поправката
fetch-deeds.mjs)TRIES_PER_EIK— иначе всяко увисване харчи пет × 20 s в увиснала връзка (тестът заковава 4 заявки вместо 16).--max-runtime-minkind, роля, отпратка към вписване и булеви — нито едно име на трето лице.concurrencyвече не е по среда — регистърът е общият ресурс, не базата.politeTrGetне се променя: докладва 429 катоRateLimitErrorи не го слага в бюджета за повторения. Мести се само кой решава колко да се чака.Проверка на живо срещу регистъра
Не опипване на прага (спец. §3.3 го забранява и ADR-0036 не разрешава повече) — а проверка, че нашият клиент се държи правилно срещу него. Цена: ~10 заявки, един умишлен блок.
scratch/tr/deeds/е празна след хода, при 8 присъди в кеша.управител(оказа се коментар в собствената ми SQL схема, вsqlite_master) иСОФИЯ(seat_normalized, разрешено по решение 3 на ADR-0010). Нула имена на лица от пет истински акта.declaredSeatsна една връзка →cached 7 · to fetch 1, поискан точно121396123.Проверка на здравия разум, че стълбата наистина е вървяла: Български пощи ЕАД →
bar_joint_stock, което е вярно.Какво това НЕ доказва: пътят
load.mjs→interest_linksот край до край (иска корпуса на ЦАИС; покрит е от тестовете) и CI обвързването (искаworkflow_dispatchсрещу staging). Това са двете дупки, които един staging ход ще затвори.Тестове
scriptsлента: 273 теста зелениpnpm test: 6/6 задачи, 487 теста в@sigma/dbpnpm check:docsи prettier чистиЗаслужава да се отбележат три неща, които работата извади:
reconcileTerminationе втора зависима от акта присъда. Без акт тя пада къмterminated— тоест сваля жива връзка, вместо да я задържи. Вече се смята и кешира до присъдата.inputs_hashе sha256 дайджест, а самостоятелен десетцифрен низ се среща в ~7% от дайджестите — същото измерване, което вече освобождаваbodySha256. Без освобождаване това би изпускало една връзка на четиринайсет, тихо. Оградата го хвана; има регресионен тест.link_keyноси само името на декларатора — служител, когото повърхността публикува по замисъл — понежеpersonIdсе строи от декларатора дори при обхватfamily. Хеширане не е нужно; ADR-0037 казва проверения факт вместо предположението.Ревю
Единствената част с експозиция е постепенният гейт в
load.mjs— той решава какво се публикува. ADR-0037 и зелениятaudit.mjsса контролите; струва си отделен поглед точно върху този хънк.