feat(etl): weekly digest producer — @sigma/report extraction + Monday cron (#167A) - #80
Open
ydimitrof wants to merge 100 commits into
Open
feat(etl): weekly digest producer — @sigma/report extraction + Monday cron (#167A)#80ydimitrof wants to merge 100 commits into
ydimitrof wants to merge 100 commits into
Conversation
Read-side queries for the Weekly Digest producer: eight indicators (total
spend, volume, largest contract, single-bid rate with a 20-sample reporting
floor, week-over-week delta, top-10 contracts, sector breakdown, authority
breakdown) scoped by ISO 8601 week via strftime('%G-W%V', signed_at), plus a
local pure priorIsoWeek() helper and a log-only reconciliation check against
home_totals.value_eur. 0004_weekly_digests.sql adds the archive index table.
Consumer side of the weekly digest, built against the committed R2 artifact fixture (soft-dep on producer #167A): - stored-report: interim StoredReport contract + readStoredReport / listStoredWeeks over R2 (moves to @sigma/report when #167A lands) - ReportBlockRenderer: maps ResolvedBlock[] to reused components (totals/facts/table/timeseries) with entityHref deep-links; bar/flows render self-contained (generic blocks carry no entity ids); never dangerouslySetInnerHTML - ReportAiWatermark, DigestFooter, WeeklyGhostBars (net-new ghost chart) - routes: /weeks archive + /weeks/:iso detail, R2-only serve (no D1/LLM), 404 for absent weeks, immutable cache for settled weeks - weeks.css for digest-specific styling Tests: 45 new (per-block golden renders, loader 404 + no-D1 guard, contract drift guard, full-page golden, AI-free fallback). Full web suite green (404 tests), typecheck clean.
Move the pure report pipeline (report-schema, emit-report-schema, verifier, temporal, describe-schema, the StoredReport contract) out of apps/web into a new @sigma/report workspace package so apps/etl can build weekly digests without depending on @sigma/web. Barrel shims at every old apps/web path (`export * from '@sigma/report'`) keep the ~30 existing import sites resolving unchanged; the moved test suites are the drift proof and pass unmodified in their new home.
Extract a pure buildStoredReport/persistReport/readStoredReport API
from agent.ts's chat-coupled persistReport, so both the chat lane and
the ETL weekly-digest producer can build/persist the same StoredReport
shape without a ToolContext. agent.ts now wraps the shared builder,
keeping chat behavior (random id, report/{id}.json key, error
swallowing) unchanged.
Add packages/report/src/iso-week.ts (priorIsoWeek) for the Monday
cron's Mon-Sun ISO-week resolution, distinct from temporal.ts's
question-parsing half-open bounds. Covers the W52/W53/W01 year
boundaries.
Superseded by assistant-contract/fixtures/stored-report.sample.json (the one wired into fixtures.test.ts); had zero code references after the @sigma/report extraction.
Adds the wrangler bindings and dependencies the weekly digest producer
needs: the REPORTS R2 bucket (fixed bucket_name, matching apps/web's
committed binding — lower risk than teaching wrangler-render.mjs's TOML
path to rename R2 per-env, which risked the two workers drifting to
different bucket names), the AI Gateway vars (AI_GATEWAY_BASE_URL,
ASSISTANT_MODEL) and the DIGEST_ENABLED kill switch (committed "false",
fail-dark like ASSISTANT_ENABLED). Adds DIGEST_CRON ('0 7 * * 1') in
lockstep with wrangler.toml's crons array and the cron-guard test, and
the @ai-sdk/openai, ai, @sigma/report and @sigma/db workspace deps.
Adds generateWeeklyDigest, dispatched from scheduled() on DIGEST_CRON
behind the DIGEST_ENABLED kill switch. Anchors on home_totals.as_of to
resolve the prior full ISO week (@sigma/report's priorIsoWeek), gates
on settlement (ADR-0007 posture: as_of >= the week's Sunday) and on a
zero-contracts short-circuit (no LLM call, no R2 write) before running
the @sigma/db weekly queries a-h and the reconciliation tripwire.
Builds the report's data blocks (totals/table/bar) deterministically
from the query results and asks a single injected GenerateFn for a
number-free lead narrative (BgGPT via the Cloudflare AI Gateway, wired
the same way apps/web/app/lib/assistant/agent.ts's buildModel does —
fail-closed without AI_GATEWAY_BASE_URL). bindReport's material-number
gate rejects a narrative that leaks a figure; up to one retry, then an
AI-free fallback (data blocks only, no model prose) is bound and used
instead. verifyReport's role-4 pass strips any unsupported claim before
persisting. Sanity gates (total >= 0, largest <= total, plausible WoW
delta) block publish outright rather than persist an unvalidated
number. Persists an immutable StoredReport to weeks/{iso}.json and
UPSERTs weekly_digests, marking a re-run over an existing week
"коригирано".
Tests cover the full gate matrix with a fake D1 + R2 bucket and a mock
GenerateFn: unsettled week, missing as_of, zero contracts (asserting
neither generate nor bucket.put are called), sanity-gate failures, the
valid path, reissue-over-existing, narrative rejected/thrown on every
attempt (asserting the AI-free fallback carries no unbound prose
number), and the DIGEST_ENABLED kill switch's fail-dark truth table.
An LLM response that trims to empty silently continued to the next attempt, reading in logs as if the narrative step never ran. Emit a distinct etl_digest_narrative_empty event (mirroring the throw/reject branches) so the fallback is observable, with a test asserting it fires once per attempt and the AI-free fallback still persists.
…o feat/weekly-digest
|
🌐 Preview deployed → https://sigma-pr-80.midt-platforms.workers.dev Worker |
12 tasks
getWeeklyCounts returned only COUNT(*) — the raw activity volume, which includes rows the week's SUM(amount_eur) excludes. Callers pairing that count with the money total would present two different row sets as one KPI set, against precompute.sql's COUNT/SUM CONSISTENCY rule. Add contractsWithAmount (the count behind getWeeklyTotal). contracts is left as-is: it is the honest volume metric, and the digest's zero-row publish gate keys on it.
The totals strip rendered „Договори" (COUNT(*), every signed row) right next to „Обща стойност" (SUM over amount_eur IS NOT NULL). The two cover different row sets, so dividing one by the other yielded a wrong average contract value — on a seeded 30-clean + 1-NULL week, 44,758 instead of 46,250. Bind the clean-basis count in the strip and expose it as R1's contracts_with_amount. The raw volume stays in R1 for the zero-row gate.
status/model keyed on narrativeMd — whether the narrative BOUND — not on whether it survived the verifier. A verifier that strips every claim (it fails closed on malformed verdicts) leaves an artifact with no model prose at all, yet it was stored as status='ok' with provenance.model naming the model. The archive index reads status, so a numbers-only digest advertised itself as model-authored. Key both on the text block surviving verification. A partial strip still leaves prose and correctly stays 'ok'.
Integrates the landed digest/report foundation (@sigma/report package with
persist + iso-week utils, weekly-digest DB queries + migration 0004, ETL
cron/generator, canonical ReportBlockRenderer/ReportAiWatermark, /reports
route) and reconciles the interim consumer scaffolding onto the canonical APIs:
- drop interim apps/web/app/lib/assistant/stored-report.{ts,test.ts} —
StoredReport + readStoredReport now come from @sigma/report
- move digest-only archive helpers to apps/web/app/lib/weeks.ts
(isoWeekKey / isValidIsoWeek / listStoredWeeks)
- weeks.$iso: read via @sigma/report, strip provenance from the client payload,
render <ReportBlockRenderer blocks> + static <ReportAiWatermark>, guard the
optional REPORTS binding
- keep both /weeks and /reports routes; keep weeks.css alongside assistant.css
- remove the now-invalid interim component tests (theirs are canonical)
Verified: @sigma/web 1164 tests pass, typecheck clean.
…links (#167B) Closes the plan-audit MVP gaps on the digest: - §3.4 daily ghost-bars — new getWeeklyDailySpend (this week + prior, 7 Mon–Sun zero-filled slots) in @sigma/db; additive `weekbars` block in @sigma/report (schema + bindReport + validateEmitShape); the generator emits it (R6/R7) and ReportBlockRenderer renders it via WeeklyGhostBars — the net-new chart is now wired end-to-end (was built but orphaned). - §3.8 competition — a concentration bar (single-bid vs multi-bid counts, R8), gated on the reporting-sample floor. - §3.10 „Разгледай сам" — code-generated deep links (contracts/authorities/ companies/flows) on /weeks/:iso. Tests: @sigma/db 223, @sigma/report 222, @sigma/etl 46, @sigma/web 1166 — all pass; typecheck clean across all packages. Remaining (documented): §3.8 stacked-procedure lane (needs a weekly procedure-type query + a stacked block type); week-scoped list filtering for the explore links (needs a `?week=` loader param).
…-docs passes (#167B)
Non-blocking review follow-ups (PR #81 review by @ydimitrof): 1. weekly-digest.ts: guard the R8 (single-bid) snapshot push under the SAME rate!==null condition that gates the competition bar, so the persisted snapshot never carries a dead result no block references. 2. weekly.ts: document the date-alignment invariant in daySpendFor — substr(signed_at,1,10) and weekDates()'s UTC slots read the same stored UTC date, the basis WEEK_FILTER's strftime already relies on. 3. WeeklyGhostBars: comment the index-pairing invariant (both series are the fixed 7 Mon..Sun slots; do not reuse with unaligned series). 4. Record the week-scoped deep-link (?week=) follow-up in docs/tickets/167b and point DigestExplore's NOTE at it. No behaviour change beyond the R8 guard. Tests + typecheck green across @sigma/db, @sigma/etl, @sigma/web.
…ropagate (#81 M1) Strict-review M1: /weeks/:iso returned `Cache-Control: immutable` (s-maxage 1y), but the producer re-issues a corrected digest in place at the same `weeks/{ISO}.json` key (status „коригирано", spec §10.4). `immutable` tells the edge/browser never to revalidate, so a correction would not reach readers for up to a year. Switch to `s-maxage=1d, stale-while-revalidate=7d` — near-static edge performance while a late-data correction still propagates within a day. Added a headers() regression test asserting the bounded policy (not immutable). When StoredReport gains a settled/refreshedAt signal, a truly-settled week can return to immutable.
…o feat/weekly-digest
…to match /reports
…the narrow report-page
… workers-types dep Resolves two review findings on PR #80: - MINOR (review 5078506825): the base seed only paired value_flag='value_suspect' with a NULL amount_eur, so „summed in total, excluded from largest/top" held by construction. Add a suspect row carrying the week's HIGHEST non-NULL amount_eur and assert it lands in getWeeklyTotal (flag-independent basis) yet is filtered from getWeeklyLargestContract / getWeeklyTopContracts (value_flag='ok' guard). - MEDIUM (strict review 5094463521): @sigma/report used R2Bucket/ExecutionContext ambient types without declaring @cloudflare/workers-types; add it as a devDependency so the package type-resolves in isolation.
Owner
|
The new commit's |
Owner
|
Fix: stagger (The rollups themselves are clean — |
Review of #80 flagged that the two shim modules used `export * from '@sigma/report'`, which re-exported the ENTIRE barrel — making ./emit-report-schema and ./report-schema identical aliases of the whole package and silently hiding any name collision in the barrel. Replace the wildcards with explicit named re-exports that mirror exactly the surface each real module (packages/report/src/{emit-report-schema,report-schema}.ts) always exposed, keeping the module boundaries narrow. Verified: apps/web typecheck clean under verbatimModuleSyntax (proves every re-exported symbol resolves in the barrel — export parity — and all import sites still resolve); @sigma/report 230 tests and @sigma/web 1213 tests pass; @sigma/report already declared in apps/web dependencies; no dangling references to the removed r2-report-object fixture.
Collaborator
|
Благодаря за прегледа — адресирах и четирите точки в т. 2 (кодова промяна) — стеснен интерфейс. И двата shim-а вече правят експлицитен re-export вместо
Така т. 1 — паритет на експортите: потвърден. т. 3 — декларирана зависимост: потвърдена. т. 4 — премахната фикстура: чисто. Верификация: |
…udit The CI "check" job (osv-scanner) failed: undici 7.28.0 is affected by 5 advisories (GHSA-4cwx-7wf7-3272 HIGH + four Medium), all fixed in 7.29.0. Raise the undici floor. This branch carries the security overrides in BOTH pnpm-workspace.yaml and a legacy package.json `pnpm.overrides` block; the package.json block is the one pnpm actually applies here (it appears in the lockfile's overrides), so bump it too — as `^7.29.0` (bounded to the 7.x line, matching #87), not the open `>=7.29.0` selector which pulled undici 8.x. pnpm-workspace.yaml is set to the same `^7.29.0` + comment, mirroring #87 so the sibling branches stay in sync for the umbrella merge. Verified: osv-scanner v2.4.0 `scan source -L pnpm-lock.yaml` → "No issues found" (exit 0); lockfile diff is undici-only (7.28.0 → 7.29.0); frozen install clean; @sigma/etl 81, @sigma/web 1213 tests pass.
lyubomir-bozhinov
requested changes
Aug 18, 2026
lyubomir-bozhinov
left a comment
lyubomir-bozhinov
left a comment
Owner
There was a problem hiding this comment.
Verified against the branch. The producer/LLM path is genuinely careful — fail-dark gates, constant-time token auth in digest-trigger.ts, zero-row short-circuit with no LLM call or R2 write, and the AI-free fallback keyed on whether narrative text survived verification. Two blockers before this should land:
BLOCKER 1 — is_synthetic header-less contracts are excluded from no digest indicator (accuracy).
- All eight queries in
packages/db/src/queries/weekly.tsfilter onlyamount_eur IS NOT NULL/value_flag='ok', neveris_synthetic != 1(grep: zerois_syntheticin the file) — lines 30, 59, 110, 151, 283, 324, 360, 419. - The canonical rollups do filter it:
scripts/precompute.sql:64,69,88,92,132(home_totals/sector_totals/authority_totals/company_totals). - Synthetic rows (parent tender is a synthetic
неизвестнаheader, ~11k УНП per0006_contracts_is_synthetic.sql) can carry a non-NULLamount_eurandtitle='(без предмет)'. So the digest can (a) over-counttotal_eur/sector/authority sums vs the rest of the site, (b) surface a'(без предмет)'synthetic as the week's „Най-голяма поръчка", and (c) makegetWeeklyCounts.contractsnon-zero, bypassing the zero-row publish gate for a week that should not publish. value_flag='ok'is orthogonal — a synthetic row can bevalue_flag='ok'. Andweekly.ts:7/ the reconciliation comment atweekly.ts:505both claim the week total andhome_totals"sum the sameamount_eur IS NOT NULLbasis" — they don't;home_totalsadditionally filtersis_synthetic, so the "week ≤ home total" tripwire is weaker than documented.weekly.test.tsnever inserts anis_synthetic=1row, so this isn't covered. Fix: addAND c.is_synthetic != 1to all eight queries, correct the two comments, and add a discriminating test (a synthetic row that would otherwise become the week's largest / inflate a sum).
BLOCKER 2 — migration 0004 prefix collision.
- The branch's
packages/db/migrations/has both0004_cpv_division_stats.sql(from base) and0004_weekly_digests.sql. The base already carries0000–0006, so0004is taken and the PR description's "assumes #17 lands 0002/0003 first" no longer holds after the base advanced. Renumber to0007_weekly_digests.sql. (migrations.test.tsreferences files by name, so CI won't flag the duplicate prefix — this is the cross-PR NNNN-collision trap this repo has hit before.)
Non-blocking: the PR description's pipeline shows persistReport(…, {immutable:true}), but the code intentionally omits immutable (correct — the object is overwritten in place on a re-issue, so immutable cache-control would be a stale-serve trap). Just update the description to match.
Strong work on the auth + producer side — these two are the gate.
Resolve conflicts: - packages/db/src/queries/index.ts: keep both weekly + related-persons exports - apps/web/workers/app.integration.test.ts: keep both noindex describe blocks (weeks + conflicts) - apps/web/app/lib/assistant/describe-schema.ts: keep the @sigma/report shim (#167A); carry base's annex_total_suspect value_flag additions into packages/report/src/describe-schema.ts - osv-scanner.toml / pnpm-workspace.yaml: keep the fuller HEAD suppression docs - package.json: drop the stale pnpm.overrides block so pnpm 10 reads overrides from pnpm-workspace.yaml (base's migration); regenerate pnpm-lock.yaml with pnpm 10.33.0
…o 0007 Resolves both blockers from the review of #80. BLOCKER 1 (accuracy) — synthetic orphan contracts (parent tender procedure_type= 'неизвестна', title='(без предмет)', is_synthetic=1 per 0006) can carry a non-NULL amount_eur with value_flag='ok', so the eight weekly.ts indicators were over-counting sums, could surface a '(без предмет)' row as the week's largest, and could make the volume count non-zero — bypassing the zero-row publish gate. Add `is_synthetic != 1` to the shared WEEK_FILTER so all eight queries (and any future one) inherit the guard, matching precompute.sql's sector/authority/company rollups. Correct the top-of-file and getWeeklyCounts comments; rewrite the reconciliation comment — home_totals.value_eur is SUM(amount_eur) over ALL contracts and does NOT itself filter is_synthetic, so the week is a strict subset of it and 'week ≤ home' is a valid loose bound, not an equal basis (the review's 'home_totals additionally filters is_synthetic' was inaccurate for that rollup). Add a discriminating test: a synthetic value_flag='ok' row with the week's highest amount is excluded from total/largest/top, and a synthetic-only week counts 0. BLOCKER 2 (migration collision) — the base advanced to 0006, so 0004_weekly_digests.sql collided with 0004_cpv_division_stats.sql. Renumber to 0007_weekly_digests.sql (git mv; table name unchanged) and fix the ticket's stale reference. Also updates the ticket's pipeline note to drop persistReport({immutable:true}), matching the code (the object is overwritten in place on re-issue, so immutable would stale-serve). Verified: @sigma/db 350 tests, @sigma/etl 81 tests pass; db+etl typecheck clean; prettier clean.
…ase merge Merging the advanced base (feat/ai-assistant-contracts) into feat/weekly-digest renumbered the migration chain — is_synthetic moved to 0012 and the base now reaches 0012 (0007 is taken by 0007_amendment_value_suspect). The weekly_digests migration (previously renumbered 0004→0007 for the review of #80) collided again, so move it to 0013_weekly_digests.sql. Refresh the two now-stale references: weekly.ts's is_synthetic provenance comment (0006 → 0012) and the ticket's migration path (0007 → 0013). No duplicate migration prefixes remain. Verified post-merge: @sigma/db 521 tests, @sigma/etl 89 tests pass; db typecheck clean; osv-scanner exit 0; check:docs ok.
Collaborator
|
Благодаря за прегледа — и двата blocker-а са адресирани (плюс non-blocking-а), на BLOCKER 1 — synthetic контракти в индикаторите (точност). Добавен е Уточнение по reconciliation-а: Добавен е разграничаващ тест: synthetic ред с BLOCKER 2 — колизия на префикса. След merge на напредналата база ( Non-blocking: описанието на PR-а и ticket-ът вече показват Проверено: |
CI 'Coverage ratchet' failed: @sigma/report (new in #167A T1) has a "test" script but no entry in coverage-baseline.json, and no vitest.config, so it emitted no coverage-summary.json and check-coverage.mjs fails closed on the missing baseline key. Add packages/report/vitest.config.ts using the shared sharedCoverage(['src/**']) preset (mirrors packages/shared) so it emits coverage/coverage-summary.json like every other workspace, and add its baseline entry (lines 96.3, branches 86.9 — floored from the measured 96.33/86.93). Verified: full 'pnpm test -- --coverage' + check-coverage.mjs exits 0, report row 96.33%/86.93%, no workspace below baseline.
Test coverage
✅ No workspace dropped below its baseline (tolerance 0.5pp). 📈 Coverage rose by more than 1pp — run |
Drop docs/tickets/167a-weekly-digest-producer.md and 167b-weekly-digest-consumer.md (the folder held only these two, so docs/tickets/ goes with them) and their two index entries in docs/README.md. check:docs stays green — no dangling references remain.
Follow-up to 82ad5c3, which deleted the 167a/167b ticket files but (due to a staging slip) landed without the docs/README.md edit that removes their two index links. Remove them now so check:docs has no dangling references.
Drop the midt-bg#167 numeric prefix from the implementation-plan filename; update the single in-repo reference (docs/README.md). git mv preserves history; check:docs stays green.
Remove .github/workflows/preview.yml and preview-reap.yml — the per-PR `sigma-pr-<n>` preview-env deploy + reaper. They are fork-only infrastructure (gated to same-repo branches, keyed to a `preview` GitHub Environment), absent from midt-bg:main and from the #79 branch that targets it. Dropping them so the upstream PR (midt-bg#324) carries the same shape as #79 instead of proposing the fork's preview setup to the main repo. check:docs stays green (the dev-environment docs reference them as inline code, not validated links).
Review of midt-bg#324 caught a dev-testing config leaked into the committed wrangler.toml (introduced by 5e760b0 'enable workers.dev route for dev digest-trigger testing'): - workers_dev = true → false (main is false) - DIGEST_TRIGGER_ENABLED = "true" → "false" Neither scripts/wrangler-render.mjs nor deploy.yml rewrites these, so the committed literals went verbatim to prod: sigma-etl would get a public *.workers.dev URL AND the on-demand HTTP trigger would be ENABLED, collapsing the three-layer defence (unreachable + flag off + bearer token) to just the DIGEST_TRIGGER_TOKEN secret. Both values also contradicted four in-code comments that state the surface is committed-false / unreachable (wrangler.toml:7-9, src/index.ts:34 & 297, src/digest-trigger.ts:3). The trigger opt-in is per-env by design (index.ts:297) — a preview env raises these via var/render, they are not a committed default. @sigma/etl 89 tests pass; toml valid.
…e refresh boundary Two findings from the review of midt-bg#324. MAJOR — deploy.yml never forwarded SIGMA_TURNSTILE_SITE_KEY to the render step. wrangler- render.mjs reads it (:98) and swaps the per-account key over the committed one (:246) only when non-empty, but the job env set just SIGMA_BUILD_ID/ASSISTANT_ENABLED/ENVIRONMENT/ AI_GATEWAY_ACCOUNT — so the value was always '', the swap a no-op, and the committed prod (domain-bound) Turnstile SITE key shipped to every environment. The moment a non-prod env turns on the assistant (SIGMA_ASSISTANT_ENABLED=true), the prod key can't validate on that domain and the bot-gate (useTurnstileGate) silently breaks. Add SIGMA_TURNSTILE_SITE_KEY: ${{ vars.SIGMA_TURNSTILE_SITE_KEY }} alongside the other four (same missing-var class as SIGMA_AI_GATEWAY_ACCOUNT). Verified via render dry-run: set → dev key renders, unset → committed prod key stays. MINOR — PROMPTS_CRON '0 6 * * 1' fired at exactly the Monday 06:00 REFRESH_CRON slot, so scheduled() ran twice and generateSuggestedPrompts raced itself on the same D1 (idempotent per-slot upserts, so wasted compute + last-write-wins refreshed_at, not corruption). Since the 6-hourly refresh already regenerates prompts and PROMPTS_CRON is only the coarse weekly fallback, move it to '5 6 * * 1' — runs just AFTER the Monday refresh (what a fallback should do) and clears every refresh slot (:00). wrangler.toml [triggers].crons updated in lockstep (cron-guard test enforces crons.ts == wrangler.toml). @sigma/etl 89 tests pass (incl. cron-guard); deploy.yml valid; render swap verified.
lyubomir-bozhinov
pushed a commit
that referenced
this pull request
Aug 26, 2026
) * test(ci): gate the fake-D1 doubles, and fail on an unmatched query midt-bg#325: 24 test files hold 36 `as D1Database` casts, one hand-rolled double each. Every one dispatches on `sql.includes('…')` and falls through to `{ results: [] }` when no marker matches, so renaming a CTE or reordering a JOIN leaves the test green against emptiness — asserting nothing. Only details.test.ts throws today. This is the acceptance test for that work, written before the work: outside an explicit allowlist, no file under apps/ or packages/ may type a value as a D1Database. It is red now (24 files, 36 casts) and goes green when the last double moves to the shared helper. The allowlist is by name, never a directory glob — the argument the midt-bg#254 review already made about the coverage exclusion list. A glob lets a new double leave the gate by where it sits; a named entry means someone had to add it, which is reviewable. A stale entry is an error rather than a no-op, so a renamed double cannot leave the gate widened by a line nobody reads again. That fail-closed branch is what fires right now, since the helper does not exist yet. Matching is over blanked source — comments, strings and regex literals removed, byte positions kept — so a comment describing the old design is not a finding. `as unknown as D1Database` is matched before `as D1Database` because the short spelling is a suffix of the long one and a naive pattern counts one cast twice. `satisfies` is covered too: it is the only other operator that types a literal. Self-test is mutation-checked — dropping the `as unknown as` alternative, the `satisfies` alternative, the comment blanking, the trailing word boundary, or the stale-entry check each kills exactly one named test, and no others. scripts-test.yml needs no edit: its lane already globs scripts/*.test.mjs. * test(test-support): a shared D1 double that throws on an unmatched query The helper midt-bg#325 asks for, as its own private workspace. `@sigma/db` exports only `.`, and neither apps/etl nor packages/ingest depends on it, so putting the double under db/src/test/ would have meant a subpath export plus two new workspace deps. A separate package also sits outside all six measured workspaces, so it cannot enter their coverage denominators by construction — stronger than the by-name vitest.shared.ts exclusion the issue proposes, and it leaves that file (which midt-bg#254 rewrites) untouched. A route is a marker set and a response; every marker must appear in the SQL, and the first matching route wins so a specific route can precede a general one. Unmatched throws, naming the offending statement and every registered marker. `{ onUnmatched: 'empty' }` buys emptiness back, at the call site, in writing. Three entry points, one core: fakeD1 for query tests, recordingD1 for the tests of a *wrapper* over D1 (readonlyD1) that must accept arbitrary SQL and assert on a call log, throwingD1 for the error paths. Two design notes worth keeping: - `first` is typed `object | null | (call) => object | null`, not `unknown`. A top type absorbs the union and the callback form silently loses its parameter type — tsc caught it. A D1 row is an object or nothing anyway. - No pagination feature. Keyset slicing is already `all: (call) => rows.filter(r => r.id > call.binds.at(-2))`, which is what the doubles in companies.test.ts do by hand today. Tests written before the code, behaviour by behaviour, and mutation-checked: never throwing on an unmatched all() or first(), matching a route that answers a different method, `some` for `every` over the markers, last-match instead of first, no truncation, dropping the marker list from the message, discarding bind() arguments, not recording prepare() or batch(), and ignoring throwingD1's supplied error — eleven mutations, each killing a specific named test. The `?? []` fallback in all() went away rather than getting a test: the route lookup already guarantees the response is defined, so it was unreachable. Returning the response instead of the route also keeps `first: null` — a route meaning "no such row" — distinct from no route at all. Seventh key in coverage-baseline.json: check-coverage's findTestWorkspaces fails closed on a workspace that has a test script without a baseline entry, and this one should be measured. 100% lines, 100% branches. The six existing workspaces are untouched — they were already reading above their baselines before this branch, which is pre-existing drift and not for a test refactor to ratchet. * test(test-support): keep `sql` live, and fail the throwing double at execution Two defects the first migration batch walked straight into. `sql` was a getter over `calls`, so `const { db, sql } = fake()` — the natural way to use it, and what flows.test.ts and authorities.test.ts already wrote against their hand-rolled spies — captured an empty snapshot that never filled in. Every later assertion then read nothing and passed for the wrong reason, which is the exact failure this helper exists to remove. It is a live array kept in step with `calls` now, and a test pins the destructured form. throwingD1 threw from prepare(). D1's prepare() is lazy and never touches the database: a missing table surfaces on all()/first()/run(). A double that failed earlier would let a test claim it covers an error path it never reaches — and related-persons.test.ts, whose whole point is that an un-migrated environment degrades instead of 500ing, hand-rolled a double that threw at execution for exactly that reason. It now rejects from the three execution methods and records the statement that failed, so the offending SQL stays inspectable. * test(db): route the query doubles through the shared fake Sixteen files in packages/db/src/queries, each of which built its own D1 double that dispatched on `sql.includes('…')` and fell through to no rows. Fixtures and assertions are unchanged — only the double moves. Measured before touching anything, by breaking each marker in the production SQL and running the test: ELEVEN marker paths across nine files stayed green against an emptied result. authorities (FROM authority_totals), companies (ORDER BY bidder_id), competition and trend and flows (FROM sector_totals), contracts (facet_counts), home (bids_received = 1, JOIN), network (FROM company_totals, FROM authority_totals WHERE authority_id), search (sqlite_master). Every one of them now rejects with the marker set it was looking for. Three things the migration turned up that were not in the issue: - regions.test.ts served *region* rows to sectorOptions, which asks a completely different table. It reached the same answer only because sectorOptions reads r.division, the region fixture has no such field, and the filter dropped every row. The route says `all: []` now, and says why. - companies.test.ts registered two facet routes for queries no test in it ever issues — getCompanyFacets is not exercised there. Dropped rather than kept as decoration. - companies' CSV stream and list query both read company_totals, so breaking the stream's ORDER BY quietly fell through to the list route and returned an unpaginated page. They are separated by their own markers now (ORDER BY bidder_id vs AS sort_value), and breaking either one throws. Two markers still survive being broken — authorities' and companies' `FROM <rollup>`. That is the harness, not the tests: `FROM ${src.from}` is composed at runtime, so the literal never appears in the source to be mutated. Mutating the `from:` value itself is caught by both. Route matching is still substring-based, so a query can fall from a specific route to a more general one in the same set. What is gone is the *default* fall-through to emptiness — an unrouted query throws. packages/db: 487 tests pass; coverage unmoved. * test(test-support): one real-SQLite D1 facade instead of four copies d1FromSqlite lived in packages/ingest/src/test/, and packages/db had two byte-identical re-implementations of it (contracts-filter-sql, value-base-sql, differing only in a local variable name) while apps/etl reached the original through ../../../packages/ingest/src/test/d1-sqlite — a relative path across a workspace boundary, which is what a missing shared home looks like. It moves to @sigma/test-support beside the fake. The two are different tools and stay different: this one runs the real SQL against a real node:sqlite database where SQL semantics are what is under test; fakeD1 is for the TypeScript logic around a query. Now they at least live in the same place, and the gate's allowlist names one package instead of two. Slightly wider than midt-bg#325 asked — the issue scopes itself to the fake doubles and puts real-SQLite tests out of scope. It is here because "one cast everywhere" is one of its own done-when boxes, and two of the four remaining casts were these copies. Reviewer's call; it lifts out cleanly. Side effect worth noting: d1-sqlite.ts leaves packages/ingest's coverage denominator by leaving the workspace, which is the outcome midt-bg#254 wanted from a by-name exclusion, reached by construction instead. db 487, ingest 84, etl 20 — all pass. * test(db): recording doubles for the two readonly wrapper suites readonly-d1 and readonly-corpus test a *wrapper* over D1, not a query: what matters is which statements reach the handle underneath, not what comes back. Marker dispatch is the wrong shape for that, so both use recordingD1 — answers anything, records everything — with `when: []`, a route that constrains nothing. Two things came out of it, both in the helper: - `when: []` matching every query was already true (every() over no markers), but undocumented and unpinned. Now both. - readonly-d1's hand-rolled log tagged its entries `prepare:` / `exec:`, and flattening that into plain SQL would have cost the test its point: a wrapper that sent an exec down the prepare path emits identical text, and the assertion could no longer tell. FakeD1Call carries `via` now, and the corpus's zero-proxy row survives as the response to a constraint-free route. readonly-corpus also dropped a `raw()` no production path calls. packages/db: 487 tests pass. * test(etl): route the ETL doubles through the shared fake Three doubles. The integrity gate's fake dispatched on eleven markers and fell through to no rows; it now names all eleven as routes and rejects anything else. Its local builder was called `fakeD1`, which is the shared helper's name, so it becomes `servedD1` — which is what it models anyway: a served D1 after precompute, not any old one. eop.test.ts also passed `{} as D1Database` twice, for paths that fail before they reach the database. `fakeD1([])` states that instead of implying it: a route-less double rejects any query, so if one of those paths ever did reach D1 the test would say so rather than throwing an incidental TypeError on an empty object. The freshness double's guard survives as a route that throws its own message — "raw staging should not be read for planning" is a claim worth keeping in the test, rather than degrading to the generic no-route error. apps/etl: 20 tests pass. * test(test-support): per-route meta, and cover the SQLite facade in its new home apps/web's assistant tests need meta.rows_read and meta.total_attempts: they drive the rows-read budget that keeps a retried full scan from under-billing the Denial-of-Wallet limit (midt-bg#122, review #80). Flattening that to a fixed empty meta would have quietly removed what those two tests assert, so a route can declare its own meta. Default stays `{}`. Moving d1-sqlite.ts here left it with no tests of its own — its callers live in db, ingest and etl, and none of them count toward this workspace. The ratchet caught it at 81% and it is covered directly now, including the case nothing tested anywhere before: batch() rolls back when one statement fails. A half-applied batch would leave a fixture in a state no production path can reach, and whoever met it would be debugging a ghost. While covering it, throwingD1's bind() read `calls.at(-1)` — so binding statement A after preparing B recorded the arguments against B. Same statement-independence bug fakeD1 already had a test against; it captures its own record now, and so does the test. 100% lines, 100% branches, 44 tests. * test(web): route the last two doubles through the shared fake assistant/tools built a double whose only real job was carrying meta; it now declares that meta on a route. csv-export asserted the expected SQL *inside* its fake — that assertion becomes the route's own marker, so a query that no longer matches rejects and names both the statement and what was expected, instead of failing an inline expect from inside a stub. With these two the gate from the first commit goes green: 279 files scanned, no D1Database cast outside @sigma/test-support. It opened at 36 casts in 24 files. apps/web: 493 tests pass. * fix(test-support): route exec() and batch() through the same contract batch() recorded each statement and returned a synthetic success without ever consulting the routes, so a batch of unregistered SQL passed against nothing — the silent green this helper exists to kill, on the one entry point the write paths use exclusively (staging, refresh, fx never call prepare().run()). exec() had the identical hole one method up. Both now look the statement up. They ask only whether it is registered at all, not for a particular response shape the way all()/first()/run() do, and throw naming the SQL and every marker when it is not. Also: run() and batch() carry the `results` key a real D1Result always has — the cast to D1Database was hiding its absence; the header no longer points at the facade's pre-move path; and the second batch() record is documented as a log of entry points rather than a double count. * fix(test-support): carry the full D1Result shape in the SQLite facade all() returned no `meta`, run() neither `meta` nor `results`, batch() no `results`. The cast to D1Database hid every one of them: the first caller to read one would get `undefined` from the facade where real D1 hands back `[]` or `{}`. One D1Shape type spells out all three keys. * test(web): keep the exact-SQL check csv-export had before the migration The hand-rolled double asserted `expect(sql).toBe(...)` on the whole statement. Migrating turned that string into a `when` marker, and markers match by substring — so the one place the refactor loosened a check rather than tightening it. Measured: wrapping the production statement leaves all 34 tests green. The equality moves inside the route, where the callback sees `call.sql`. * test(ci): pin the gate's scan roots and catch an aliased D1Database Two ways past the gate, both reproduced. `type DBAlias = D1Database` and then `as unknown as DBAlias` leaves no D1Database token for the pattern to find; a renamed type import does the same. A second pass treats giving the type another name outside the allowlist as the offence, while leaving ordinary annotations (`db: D1Database`, a field on an Env type) alone. And SCAN_ROOTS was module-private, so deleting 'apps' from it left the self-test 12/12 green while web and etl dropped out of enforcement. Exported and pinned: a pattern applied to half the repo is a gate that passes while enforcing nothing. * fix(test-support): route a batched statement by what it does, not its markers Markers alone are blind to the method, and it is measurable: a route declaring only `all:` answered a batched `DELETE FROM staging` with its rows, because `FROM staging` is a substring of the write. The same SQL through prepare().run() threw. Narrower than an unrouted batch, but the same silent pass. A batched write now needs a `run:` route and a batched SELECT an `all:` one; neither settles for the other, and a SELECT no longer fires a write effect it happens to match. A write still serves rows when it has them, for RETURNING. exec() asks for `run:` too — it hands back no rows, so nothing else means anything to it. That retires `registered()`: every entry point is method-aware. Reading is decided by the leading keyword, so a `WITH … INSERT` reads as a SELECT here. That costs a false rejection, never a false pass. Also: run() carries the route's meta, which batch() already resolved for the same statement, and batch() documents that it is not transactional — real D1 and the d1-sqlite.ts facade roll back, this does not. * test(ci): widen the alias rule to where an alias actually goes The pattern closed three spellings while the comment promised the class. Four more walked past it: `D1Database & {}`, `Pick<D1Database, …>`, a namespaced `import('…').D1Database`, and a heritage list naming it off the first position. The rule is now positional — the mention must sit right of `=`, or inside an intersection, union, type argument or namespace, never where a parameter or a field goes. `type Env = { DB: D1Database }` and the conditional type in readonly-corpus.test.ts stay clean, both pinned. `implements` is deliberately out: ReadonlyD1 implements D1Database in production, and TypeScript forces a complete implementation there, so it is no shortcut to a stub. The comment now says best-effort and means it. --------- Co-authored-by: Todor Kolev <tkolev@obecto.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #167A — Weekly Digest: Producer (pipeline · data · generation) from
docs/implementation-plans/167-weekly-digest.md. This is the generation spine for „Седмицата в пари": every Monday the ETL worker writes a digest artifact to R2. Dev B consumes theStoredReport; this PR only produces it.Base is
feat/ai-assistant-contracts, notmain. This is a hard dependency, not a convenience: the digest reusesverifier.ts, the ETL cron scaffold, and ADR-0007, none of which exist onmain. Merge #17 first, then retarget this tomain.Migration is numbered
0004on the assumption that #17 lands0002/0003first.What's here
T1 — extract
@sigma/report(~2d)packages/report— pure, worker-agnostic (no React, nocloudflare:*), so both the web worker and the ETL cron run the same emit→bind→validate→verify pipeline. It was previously trapped inside@sigma/web, which is why a cron couldn't reuse it.git mvto preserve history: block schema,bindReport,validateEmitShape,sanitizeProse/Cell,findProseNumbers,asNumber,formatCell,verifier.@sigma/webre-exports from the new package, so the chat suites pass unchanged — that's the drift proof.persist.ts:StoredReport/Provenance,persistReport(bucket, key, stored, {immutable}),readStoredReport(bucket, key). R2 arrives as an injectedR2Bucket— no binding names baked in.isoWeekLabel(date),weekBounds(iso). Tested on the W52/W53/W01 boundaries.T2 — DB layer
0004_weekly_digests.sql:weekly_digests(iso_week PK, as_of, refreshed_at, status, total_eur)— an archive index only. The report payload stays in R2 as the single source of truth rather than being copied into D1.packages/db/src/queries/weekly.ts: one function per indicator a–h. Money guarded withWHERE amount_eur IS NOT NULL, sector viasubstr(cpv_code,1,2), single-bid atbids_received=1behind a ≥20 sample floor, top-10 carrying ids for links.SUM(amount_eur)againsthome_totals.value_eurand logs on drift. It deliberately compares value againstvalue_eurand never equates counts —home_totals.contractsis aCOUNT(*)over all rows, so it isn't the clean-amount count.T3 — ETL cron
DIGEST_CRON = '0 7 * * 1'(Monday 07:00 UTC, after the 06:00 refresh), appended in the order cron-guard expects.weekly-digest.tspipeline: freshness anchor → GATE 1 settled-week (ADR-0007) → GATE 2 zero-row short-circuit → queries a–h → reconciliation → emit blocks → LLM narrative (BgGPT via AI Gateway) →bindReport→findProseNumbersgate → bounded regeneration → verifier strip → still invalid ⇒ AI-free fallback template →persistReport(…)(noimmutable— the object is overwritten in place on a re-issue, soimmutablecache-control would be a stale-serve trap) → UPSERT → structured log.wrangler.tomlgains theAIandREPORTSbindings plus the kill-switch var;wrangler-render.mjsupdated to match.Both gates run before any query or LLM spend. Numbers are 100% SQL-bound — the model only writes connective narrative, and unsupported claims get stripped rather than rewritten. An unvalidated number is never persisted.
Tests
Written alongside, per the plan's TDD requirement:
persist.tsvalidated against the golden fixture; ISO-week util on W52/W53/W01.puthappens; invalid after N regens ⇒ fallback persisted; kill-switch off ⇒ no publish. cron-guard extended forDIGEST_CRON.Notes for review
r2-report-object.fixture.jsonmoved out ofapps/web/.../fixtures/— it's the frozen golden reference for both this ticket and Dev B's renderer./companies/{ЕИК}; that's wrong for name-keyed bidders (name:<name>), so links route throughentityHref/hrefForEntityrather than formatting ЕИК by hand./weeks/{ISO}SSR route,ReportBlockRenderer,ReportAiWatermark.