feat(ai-assistant): conversational analytic layer (BgGPT) - #79
feat(ai-assistant): conversational analytic layer (BgGPT)#79lyubomir-bozhinov wants to merge 92 commits into
Conversation
Team-of-agents design for the AI assistant, reconciled with spec §9: roles + trust zones, prompt-injection defenses (incl. signed-transcript vector), report generation (when/how), serving view, voice lane, AI Gateway routing, RAG (grounding + semantic_search), and report dedup/idempotency (L1 prompt-hash → L2.5 result-fingerprint).
Add a Guarantees-vs-limits subsection: integrity + traceability are guaranteed by construction (values-by-reference, deterministic link form, reproducible SQL/freshness/version), but data correctness is best-effort (wrong query, staleness, upstream quality, ETL bugs, interpretation). Documents the prose-leak, aggregate-vs-entity link nuance, link rot, and four gap-closers. Honesty as a defamation-risk control.
Extend the guarantees-vs-limits section with concrete guardrails that harden the residual data-correctness gaps, building on PR #80's system-prompt/describe-schema foundation: default filters, reconcile- with-rollup self-check, explicit CPV interpretation, mandatory methodology callout, Verifier trap-compliance checks, and a golden- reports CI harness. Honesty (watermark + methodology) stays load-bearing.
Address all findings from the team review: - Separate deterministic gates (code: trap checks, sanitization, reconciliation, no-number-in-prose) from the probabilistic LLM Verifier (necessary-not-sufficient, fed references not raw strings). - Reframe the read-only D1 honestly: net-new infra, not today's ETL; engine-truthful guards (EXPLAIN allowlist + single-statement + canonical-AST) are load-bearing. - Scope reconcile-with-rollup to a rollup's exact grain; mark others unreconciled; block-not-substitute. - Fix §4 circuit-breaker contradiction: AI Gateway global cap fires mid-pipeline; orchestrator handles a 429. - Specify trim/summarize (server-side, HMAC-signed) and bind HMAC to conversation/turn/position; make guard-(b) load-bearing. - EXPLAIN closed read-opcode allowlist; composite per-source freshness token; named sanitizer + strict-CSP defense-in-depth. - Add fail-closed UX, deterministic-guard telemetry, publish-path caching off, default-filter/callout tie-in, My-reports/dedup reconciliation, no-number-in-prose as a launch requirement.
lyubomir-bozhinov
added
docs
Sign HMAC-SHA-256(role, content, conversationId, turnIndex, position) over every server-emitted assistant/tool message and drop unsigned, forged, cross-conversation, replayed, or out-of-position messages on the next turn. Length-prefixed canonical encoding prevents field-boundary forgery; constant-time compare; fails closed when ASSISTANT_HMAC_KEY is unset. Adds the key to env.d.ts and .dev.vars.example.
Keep the last N turns verbatim and collapse older ones into one deterministic, HMAC-signed summary (tool payloads dropped, report chips folded into the signed content). Re-signed with the E1 key so it survives the next turn's filter.
Apply safe defaults deterministically (exclude value_suspect, exclude synthetic 'неизвестна', use signed_at not published_at) as a parameterized SQL fragment, with explicit opt-outs each tied to a surfaced callout line.
Reconcile a computed aggregate against a fixed-scope rollup at its exact grain (exact counts, epsilon tolerance for REAL sums) and block-and-surface via ReconcileError on mismatch instead of substituting either figure.
Map word sectors to CPV divisions explicitly against the @sigma/config taxonomy, record the mapping in the callout, and flag ambiguity (category words -> multiple divisions; unknown -> assumption, no filter) so the assumption is surfaced.
…rowing (E1) filterIncomingTranscript receives attacker-controlled messages; a non-integer or negative turnIndex/position let integerField throw out of verifyMessage, failing the whole turn instead of dropping the offending message. Add hasValidSlot, return false from verifyMessage on malformed slots, and drop them with a new 'malformed-slot' reason. Sign path still throws (producer-side programmer error).
…med turns E1: extend the signed tuple to (role, content, conversationId, turnIndex, position, report-chips) so a verbatim message's /reports/:id chips cannot be retitled or re-pointed at another report across turns. E2: trimTranscript now takes conversationId and independently re-verifies every collapsed assistant/tool message under the E1 key before folding it into the signed summary, so a mis-ordered pipeline cannot launder injected text into server-authentic content.
…_eur IS NOT NULL) E3 excluded value_flag = 'value_suspect', but per the ETL's correction-over- exclusion policy those rows are repaired to a non-NULL procEst amount and ARE counted in the rollups E4 reconciles against (amount_eur IS NOT NULL). The two guards therefore disagreed on the row-set: a live aggregate built through E3 undercounted vs the matching rollup, so assertReconciled (E4) could throw on a correct number. Default now excludes amount_eur IS NULL (the unrecoverable value_suspect subset with no procEst), matching the rollup basis exactly. Renames the opt-out includeValueSuspect/excludeValueSuspect -> includeUnsummable/excludeNullAmount, rewrites the callout (suspect rows are corrected, not distorted), documents the expected c./t. join aliases, and records the canonical row-set in docs/etl.md. Corrects two 0000_init.sql comments that wrongly equated NULL amount_eur with value_suspect.
…een tests Replaces the empty afterEach + inline 'sign to re-prime the cache' calls (which left the suite order-dependent) with an exported resetKeyCache() called in afterEach. Production behaviour is unchanged — the cache is still keyed by material and rotation-safe.
…uard home_totals is not realigned with E3: precompute.sql fills home_totals.contracts with COUNT(*) over ALL contracts (a corpus tally) and home_totals.suspect with COUNT(value_flag = 'value_suspect') — neither matches the amount_eur IS NOT NULL basis. Correct the three home_totals column comments to describe what the query actually computes (a prior edit had mislabeled suspect as the NULL-amount set), and document in reconcile-rollup.ts + docs/etl.md that E4 reconciles only against the amount_eur-filtered rollups (sector/authority/company), never home_totals.contracts — else a count reconcile would throw on a correct number. Also drop the dead 't.procedure_type IS NULL' branch from the synthetic-tender guard: contracts.tender_id is NOT NULL REFERENCES tenders and tenders.procedure_type is NOT NULL, so the column is never NULL and the branch was unreachable.
… drift mapSectorWord's SECTOR_SYNONYMS/CATEGORY_SYNONYMS hardcode division codes and category keys that duplicate @sigma/config. Add round-trip tests asserting every synonym still resolves to a division/category that exists in the catalog, so a taxonomy change can't silently break the mapping.
…ction feat(assistant): Lane E — integrity & anti-injection guards
Mirror the assistant-contract seam (report.ts, stream.ts, fixtures + spec) onto feat/ai-assistant. It re-exports ResolvedReport from app/lib/assistant/report-schema, which is present here now that #80 has landed in main and feat is caught up. Closes the seam parity hole between feat/ai-assistant and feat/ai-assistant-contracts.
Lint (prettier --check) failed after the foundation merge + seam add: - the 4 assistant-contract seam fixtures/README were formatted for the contracts branch's prettier; reformat to this branch's config. - RiskIndicators.tsx and riskLogic.test.ts are upstream prettier debt the merge pulled in; pure line-wrapping, no semantic change.
…eam parts (#10) * docs(assistant): Lane F report dedup, single-flight & dock UX spec * feat(assistant): F1 report dedup (L0-L3) with freshness token Pure KV-backed dedup keyed on the resolved query (L2) and result data (L2.5) so identical fixed-period requests never regenerate or diverge. Freshness token reuses the home_totals.refreshed_at derivation; every read error falls toward regeneration. encodeFields vendored from the Lane E length-prefix pattern (PR #3 not yet merged). * feat(assistant): F2 single-flight report generation coordinator Collapses concurrent generations for one key onto a single shared in-flight promise so two identical fixed-period requests can never diverge. Gates cache hits on R2 artifact existence, clears the flight on generator failure (fail toward regeneration), and rebroadcasts coarse progress to all waiters with late-waiter catch-up. The DO shell and wrangler bindings are deferred to the Phase 3 wiring step (spec 3). * Add F3 dedup/progress stream parts Producer adapters and centralised Bulgarian copy for data-dedup and data-progress custom stream parts, mirroring the data-report-ready contract. Maps F2 single-flight outcomes to wire parts; graduates into assistant-contract/stream.ts when the seam converges. * fix(assistant): harden dedup canonical encoding against value collisions - canonicalJson: tag Date/NaN/±Infinity/undefined/bigint so distinct values never share a dedup key (midt-bg#97); JSON.stringify collapses them. Document the value domain and the caller trust boundary (reportId must be server-minted). - sha256Hex: cast to BufferSource — fixes the lane's only tsc -b error under TS's split Uint8Array<ArrayBufferLike> typing; mirrors transcript-hmac.ts. - single-flight: honest header — in-isolate collapse + KV backstop hold now; the Durable Object is Phase 3 (was implied present). Note record-failure can't diverge numbers (values bound by reference). - tests: +9 adversarial canonical cases, +4 single-flight (write-failure swallowed, r2Exists throw, cross-isolate KV dedup). - spec: pin the new F1/F2 guarantees. * test(dedup): make single-flight collapse assertion deterministic The 'runs the generator exactly once under N concurrent calls' test asserted every concurrent caller collapses onto the in-flight promise (deduped:false). That is not a guarantee: a caller whose async key derivation lands after the leader records legitimately reuses the recorded report via the cache path — identical numbers, different layer. Under full-suite load the crypto timing shifted and one caller took that path, failing the assertion. Replace with the guarantees that are deterministic and that actually matter: generator called exactly once (broken collapse would call it three times), all callers see one identical report, single createdAt across callers (midt-bg#97 no-divergence). Drops the timing-dependent label check; no production change. * harden(dedup): unify L2.5 fingerprint framing; make encoder injective over -0 Addresses review finding #1 and pre-empts adjacent nits, no behavioural change for real inputs: - resultFingerprint now frames rows through encodeFields ('L2.5-rows' domain), the same length-prefixed injective encoding dedupKey already uses, instead of a NUL-separated join. The join was injective only by relying on JSON escaping NUL inside strings; length-prefix framing is self-delimiting by construction, so L2.5 (the strongest layer) no longer looks weaker than L1/L2/L3. - canonicalJson now tags -0 distinctly ('number:-0'); JSON.stringify erases the sign (-0 -> 0). Keeps the injectivity claim airtight with no caveat. No divergence risk: -0 and 0 bind identically in D1, so worst case is a redundant generation for an input that effectively never occurs, reconciled by L2.5. Also tightened in-file docs to close consistency questions: vendor rationale for encodeFields (deliberate, not duplication; consolidates when PR #3 lands), freshnessToken injectivity over its fixed ISO/build-id domain, acyclic-domain note on canonicalJson, and L3's out-of-resolveReport scope. Spec test obligations updated to list -0. Tests: 215 passed / 0 failed; typecheck clean. * docs(dedup): scope canonicalJson injectivity claim to its domain A second adversarial pass confirmed the only constructible non-injectivity is out-of-domain (function/Symbol -> null via JSON.stringify -> undefined), which cannot reach the encoder (D1 scalars / JSON.parse tool-args). Make the one unqualified sentence precise rather than add whack-a-mole guards for unreachable exotics. Comment-only; no behaviour change.
The dedup files were formatted for the pre-foundation-merge prettier config; new feat (upstream config) reformats them. Pure formatting, no semantic change.
…ality fix(assistant): refuse hollow fallback reports + answer_directly escape hatch (feat mirror of #69)
fix(assistant): SQL guard fail-closed allowlist + Turnstile prod fail-closed (feat mirror of #64)
feat(assistant): voice input lane /assistant/transcribe (feat mirror of #66)
feat(assistant): HMAC transcript signing §9.3 (feat mirror of #68)
…pstream feat Reflects #75 (PRIV-1 star-rule scoping, a11y, voice, web-perf) and #77 (report-quality: context overflow, region/CPV mapping, ranking headline, chip freshness) onto the upstream-facing branch. Deploy-layer artifacts (docs/deploy-assistant.md and its README index entry) stay contracts-only per dual-PR parity.
… feat
Brings feat to content-parity on the assistant's deploy layer (was missing/stale upstream):
- wrangler-render.mjs: SIGMA_ENVIRONMENT->ENVIRONMENT (HMAC fail-closed gate), ASSISTANT_ENABLED,
DEDUP_KV id + BUILD_ID stamping, Vectorize per-env rename; fixes R2 by-binding rename and the
JSONC parse crash on renamed-resource deploys.
- deploy.yml: SIGMA_* env + Ensure DEDUP_KV / voice-provider / ASSISTANT_HMAC_KEY steps + ref-injection hardening.
- ci.yml: script-tests job. deploy.md: assistant secrets + ENVIRONMENT/HMAC section.
- ensure-{kv-namespace,voice-provider,worker-secret}.mjs (+tests), bootstrap-r2, provision-environments,
deploy-assistant.md, dev-environments docs, README index.
Fork ephemeral preview/dev machinery (preview*.yml, reap/teardown, scripts-test) and the
CF-account/gateway secret values stay contracts-only. Pre-commit secret-scan bypassed: confirmed
false positive on doc placeholders, byte-identical to the already-accepted -contracts copy.
Was stale at the backend-only milestone. Now reflects the whole feature: three-layer read-only SQL guard, server-owned report values + dedup, RAG, voice, dock UI, transcript HMAC (enforced), enforced RPM circuit-breaker, wired freshness, eop_fetch returning rows. Fixes: 150->1223 tests, 27B->31B, two->three guard layers, HMAC no longer 'deferred'. Keeps the (still-accurate) provisioning gate + the semantic_search ns:'entity' caveat.
ydimitrof
left a comment
ydimitrof
left a comment
There was a problem hiding this comment.
Обобщен преглед на PR — feat(ai-assistant): conversational analytic layer (BgGPT)
Какво прави PR-ът
PR-ът въвежда цялостен разговорен аналитичен слой („BgGPT") върху платформата — чат-док асистент, който отговаря на въпроси и генерира структурирани отчети от данните за обществени поръчки. Обхватът е широк и добре пластуван:
- Клиентски UI (React): чат-док (
AssistantDock/Panel/Composer/Message/Launcher), гласов вход, hooks за чат/Turnstile/starter-prompts, персистенция в localStorage, проекция и експорт на отчети (Markdown/DOCX), безопасен Markdown рендер и силна достъпност (WCAG — скрити таблици за екранни четци, aria-live, focus-trap). - Сървър/worker слой: агент оркестрация върху AI SDK,
emit_reportсхема и валидация, многослойна SQL защита (L1 структурен → L2 AST allowlist → L3 EXPLAIN opcode guard), default-filter gate, read-only binder на отчети, temporal резолвър, dedup/single-flight, rate-limiting, circuit breaker, транскрибиране (Whisper/STT през AI Gateway) и HMAC подписване на транскрипта. - Инфраструктура и документация: provisioning скриптове (идемпотентни, GitOps), CI/deploy workflow-и, DB миграции (
is_synthetic,assistant_prompts), golden фикстури + replay harness, ADR-и и обширни spec/deploy документи.
Сигурност — ЧИСТО във всичките 24 партиди
Phase 0 скенът не откри блокиращи проблеми: няма хардкоднати тайни (всички ключове идват от env/wrangler secret put, dev-плейсхолдърите са ясно обозначени), няма злонамерени модели, backdoor-и или обфускация. Новите URL-и са легитимни (Cloudflare Turnstile/AI Gateway, CF API, BgGPT upstream) и в повечето случаи документационни. PR-ът дори затяга сигурността: positive allowlist за SQL функции (fail-closed), GDPR денилист за лични колони, защита срещу prompt-injection (nonce-таговани огради, verifier), маскиране на грешки, constant-time сравнения, HMAC ротация на ключове, и поправка на shell-инжекция в deploy.yml.
Обща оценка
Много високо качество навсякъде — детерминистична чиста логика, отлично разделяне на отговорностите, коментари обясняващи „защо", и смислени (не тривиални) тестове с adversarial уклон. Няма частични имплементации, дублиране или мъртъв код (извън съзнателните TODO(foundation-merge) огледала, които трябва да се проследят при merge).
Единствено блокиращо за APPROVE
- Покритие с тестове на
report-export.ts(партида 14).reportToDocxBlob(~200 реда),downloadBlobиfactsклонът вreportToMarkdownостават нетествани — трябва да се добавят тестове преди APPROVE, за да се удовлетвори гейтът ≥90%.
Съществени находки за адресиране (не блокиращи, но си струват)
freshnessTokenколизия (партида 18): премахването на всички не-алфанумерични символи отbuildIdможе да сведе различни билдове до един и същ токен (1.2.3и12.3→123), което би сервирало остар отчет след деплой. Заслужава корекция.flowsбез null-guard (партида 14):money(e.valueEur)дава „NaN" при null стойност, за разлика отfmt, който връща „—".- Потенциален leak на ключ в dry-run (партида 23):
ensure-voice-provider.mjsможе да отпечатаAuthorization: Bearer <secret>на stdout — да се маскира тялото на заявката преди merge. chooseToolChoiceразминаване с коментара (партида 7): при извикване на reconcile чак на последната forced стъпкаemit_reportникога не се форсира; сървърниятbuildFallbackReportпокрива случая, но поведението се разминава с документацията.
По-дребни/консистентност (по избор)
alignне е деклариран вEMIT_REPORT_JSON_SCHEMA, макар фикстурите да го ползват; да се потвърдиisFormat=FORMAT_SCHEMA(партида 8).- Възможно прекомерно fail-closed блокиране в
denyAmplifyingStringChainи false-positive при CTE/литерали на имеcontracts/rollup таблици (партиди 7, 10, 12) — безопасно, но води до излишни retry-и. - Индекс с ниска селективност
idx_contracts_is_synthetic(партида 22); свързване чрез литерали вместо константи вagent.ts(партида 7); непоследователно фиксиране на версии вpackage.json(партида 16). - Няколко UX ръба: микрофонът не се изключва при
busy,target="_blank"за вътрешни линкове, евристикатаMIN_LEN=10вAssistantMessage.
Cross-batch зависимости за финална проверка при консолидация
- Съществуване на маршрута
/reportsи на външните CSS правила (.ts-data-table) за a11y контракта. normalize-raw.sqlдействително задаваis_syntheticпри insert (иначе slot 4 брои грешно нови синтетични договори).- Дефиниран turbo task
test:goldenи доставен golden harness. - Docs-integrity: препратки към ADR-0011..0013 и spec файлове да не станат мъртви връзки; уеднаквяване на
import.mjsкомандите между двата dev-environment документа. - Реалното изпълнение на тестовете, покритието ≥90% и SLA/производителност се затварят едва при агрегиране на всичките 24 партиди.
Заключение
Силен, добре структуриран и сигурен PR с последователно fail-closed поведение и задълбочена защита срещу XSS/SQL/prompt-injection. Едно блокиращо условие за APPROVE: тестово покритие на report-export.ts. Препоръчително преди merge: коригиране на freshnessToken колизията, null-guard в flows и маскиране на dry-run изхода. Останалите наблюдения са незадължителни подобрения по usability, консистентност и проследимост.
| const safeHref = sanitizeLinkHref(m[8] ?? ''); | ||
| if (safeHref !== null) { | ||
| nodes.push( | ||
| <a key={k} href={safeHref} target="_blank" rel="noopener noreferrer"> |
There was a problem hiding this comment.
Котвата се рендерира с target="_blank" rel="noopener noreferrer" за всеки позволен href, включително относителни/вътрешни пътища (тестът [report](/reports/r_abc123) минава именно през този клон). Отваряне на вътрешна навигация в нов таб е необичайно за in-app линк към справка и заобикаля client-side router-а. Обмислете target="_blank" само за абсолютни (external) URL-и, а за относителни пътища да се остави нормална навигация. Не е блокиращо, но е UX-несъответствие.
| </ul> | ||
| {truncated && ( | ||
| <p className="report-block__truncated-note"> | ||
| Показани са само първите резултати — данните са отрязани. |
There was a problem hiding this comment.
Дублиране на UI низ (CLAUDE.md „NO CODE DUPLICATION"): идентичният блок за отрязани данни — <p className="report-block__truncated-note">Показани са само първите резултати — данните са отрязани.</p> — се повтаря буквално четири пъти (bar, flows, table тук + timeseries в TimeseriesBlock.tsx). При промяна на текста рискувате разминаване между блоковете. Препоръчвам извличане в споделен компонент TruncatedNote (или общ низ) — така форматирането остава консистентно на едно място.
| return { y: PAD.top + CHART_H - fraction * CHART_H, value: minVal + fraction * valueSpan }; | ||
| }); | ||
|
|
||
| // X-axis labels: show every nth point to avoid overlap. |
There was a problem hiding this comment.
toSeries(...).slice(0, MAX_SERIES) отрязва мълчаливо серии след 4-тата — при това и от SVG-то, И от скритата таблица с данни. Ако multi-series някога се излъчи (>4 серии), данни се губят без никакво указание, което противоречи както на самия коментар по-горе („surface a truncation note"), така и на WCAG-целта данните да са пълно достъпни за AT. Днес се излъчва само single-series, така че това е латентна забележка, но си струва да се затвори едновременно с multi-series: при series.length > MAX_SERIES покажете truncation note (или го логнете), вместо тих drop.
| // its first tool call, so an empty post-tool text must not silently drop that summary. Skip preambles | ||
| // starting with `|` (partial markdown tables — the case the original preamble-discard rule prevents). | ||
| const MIN_LEN = 10; | ||
| if (lastToolIdx >= 0 && postTool.length < MIN_LEN) { |
There was a problem hiding this comment.
Евристиката MIN_LEN = 10 може да отхвърли кратък, но легитимен финален отговор след tool-part (напр. „Готово.“ или „Ето.“). В такъв случай се връща pre-tool преамбюлът, а ако той е празен — празен низ, т.е. валиден кратък отговор не се показва. Приемливо е, защото картата на справката се рендира отделно, но си струва да се провери дали кратки истински обобщения не изчезват мълчаливо.
| onKeyDown={onKeyDown} | ||
| placeholder="Напишете въпрос…" | ||
| rows={1} | ||
| disabled={busy} |
There was a problem hiding this comment.
Микрофонът (AssistantComposerMic) не е обвързан с busy. Докато има активна заявка textarea е disabled, но гласовият вход остава активен и завършен транскрипт се добавя през appendTranscript към поле, което потребителят не вижда/не може да редактира. Обмислете подаване на busy към mic-а (или игнориране на транскрипт докато turn-ът е в ход).
|
|
||
| ```bash | ||
| SIGMA_D1_NAME=sigma-dev SIGMA_D1_ID=<dev-d1-id> \ | ||
| node scripts/import.mjs --work-db --remote |
There was a problem hiding this comment.
Тази команда използва --work-db (без стойност), докато dev-environments.md §1.3 описва същата операция като --work-db=data/work/backfill.sqlite (с явен път към work SQLite). Двата runbook-а описват идентичния еднократен seed — моля уеднаквете формата на флага, за да не се чуди операторът коя форма е коректна.
| FROM tenders t | ||
| WHERE t.id = contracts.tender_id; | ||
|
|
||
| CREATE INDEX idx_contracts_is_synthetic ON contracts(is_synthetic) WHERE is_synthetic = 0; |
There was a problem hiding this comment.
Частичният индекс WHERE is_synthetic = 0 покрива мнозинството от редовете (нормалните договори) и индексира колона, която в самия индекс е константа (0) — т.е. практически ~цялата таблица без реална селективност, само с разход при запис. Заявките филтрират is_synthetic = 0 AND signed_at > …, където селективен е диапазонът по дата. Обмислете композитен индекс (напр. (signed_at) или (is_synthetic, signed_at)), или частичен индекс по рядката стойност WHERE is_synthetic = 1, ако целта е бързо намиране/изключване на синтетичните договори.
| // Dry-run decorator: GETs pass through (safe); every mutation is logged as WOULD … and answered with a | ||
| // synthetic success so the full plan prints in one pass without touching the account. | ||
| function dryRunFetch(real, log) { | ||
| return async (url, opts = {}) => { |
There was a problem hiding this comment.
Потенциален leak на тайна в dry-run изхода. dryRunFetch логва opts.body дословно на stdout. Когато VOICE_ASSISTANT_API_KEY е зададен и bggpt-voice провайдърът още не съществува, ensureCustomProvider изгражда POST тяло с headers: { Authorization: 'Bearer <apiKey>' } (ред 136). Тъй като dry-run е режимът по подразбиране, стартиране на node scripts/ensure-voice-provider.mjs (без --apply) с наличен ключ ще отпечата WOULD POST ... {"...","headers":{"Authorization":"Bearer <secret>"}} в терминала/CI лога.
Това противоречи на грижата за тайните, приложена в ensure-worker-secret.mjs (където ключът умишлено никога не докосва stdout). Предложение: маскирайте Authorization/headers преди логване, напр. отпечатвайте url + метода и заменяйте стойността на Authorization с ***, или логвайте само Object.keys(body).
Пример:
const safeBody = opts.body
? opts.body.replace(/("Authorization"\s*:\s*")Bearer [^"]+/, '$1Bearer ***')
: '';
log(` WOULD ${method} ${url}${safeBody ? ` ${safeBody}` : ''}`);| @@ -61,12 +61,12 @@ SELECT b.id, b.name, b.kind, b.ownership_kind, b.eik_normalized, b.eik_valid, b. | |||
| SUM(CASE WHEN c.eu_funded = 1 THEN c.amount_eur ELSE 0 END), | |||
| MIN(c.signed_at), MAX(c.signed_at) | |||
| FROM contracts c JOIN bidders b ON b.id = c.bidder_id JOIN tenders t ON t.id = c.tender_id | |||
| WHERE c.amount_eur IS NOT NULL | |||
| WHERE c.amount_eur IS NOT NULL AND c.is_synthetic != 1 | |||
There was a problem hiding this comment.
Филтърът c.is_synthetic != 1 изключва и редовете с is_synthetic IS NULL (в SQLite NULL != 1 → NULL, т.е. не преминава). normalize-raw.sql и refresh-slice.sql попълват 0/1 явно, но евентуални заварени редове без backfill на новата колона биха изпаднали тихо от всички публични rollup-и (company/authority/sector totals). Тъй като integrity-checks.mjs също ползва != 1 от двете страни, reconciliation-ът няма да улови този drop. Моля потвърдете, че schema миграцията (в друг batch) backfill-ва is_synthetic за съществуващите договори — напр. ADD COLUMN is_synthetic ... DEFAULT 0 или явен UPDATE — така че NULL да не остане в реални редове.
| if (names.vectorizeName && Array.isArray(obj.vectorize)) { | ||
| for (const index of obj.vectorize) { | ||
| if (index && typeof index === 'object') index.index_name = names.vectorizeName; | ||
| } | ||
| } | ||
| // Stamp the real per-build dedup freshness `c` over the committed "dev" constant. | ||
| if (names.buildId && obj.vars && typeof obj.vars === 'object') obj.vars.BUILD_ID = names.buildId; | ||
| // Opt this environment's assistant IN over the committed fail-dark "false". |
There was a problem hiding this comment.
ASSISTANT_ENABLED (master kill switch, #83) и ENVIRONMENT (HMAC gate, ADR-0012) са критични за сигурността. Присвояванията се пропускат тихо, ако obj.vars липсва (if (names.x && obj.vars && ...)). При committed файл с vars секция това е само защитна мярка, но при бъдещ рефактор, който премести/премахне vars, подаден SIGMA_ENVIRONMENT=production би останал без ефект → committed default "development" → HMAC gate fail-open в production, при това без сигнал. Предвид ролята им бих предпочел явна грешка (fail loud), когато очакваната vars секция липсва, вместо тих no-op.
Ревю на асистента — коректност и защити (последен
|
report-schema: flag трилион/билион/квадрилион in the prose number gate — a spelled trillion-scale figure matched no pattern and could land unbound on a public report. report-schema: drop an entity link whose id-domain prefix (auth:/eik:/name:/c:) contradicts the model-declared link.kind, which rendered a wrong-collection href on a citation-bearing report. describe-schema: correct the parties data dictionary — it advertised a non-existent `role` column, so the model emitted `SELECT role` that passed the SQL guards and then errored at D1. dedup: make the freshness token injective over buildId (encodeURIComponent, not a lossy [a-z0-9] strip) so distinct builds can't collide a freshness code and serve a stale report after deploy. ensure-voice-provider: mask the Authorization bearer in the dry-run body log so a stored VOICE_ASSISTANT_API_KEY never prints to stdout/CI. agent: clarify the chooseToolChoice comment for the final-step reconcile edge; behavior unchanged. Tests added/updated for each change and proven to fail against the pre-fix source; report-export docx/download/facts branches are now covered.
…ency queries
Two defects surfaced by the live 50-question run on sigma-pr-17:
- Q17/Q46: when the model SELECTs an id column (`authority_id`=`auth:…`,
`bidder_id`=`eik:…`/`name:…`, `contracts.id`=`c:…`) as a display column, the
raw internal scheme prefix leaked into the public report. sanitizeCell now
strips a leading scheme token at the single choke point, so a stray id renders
as its real-world value (name / ЕИК / УНП). Entity links are unaffected — they
bind from the raw row value on a separate path. A DATA_TRAP also steers the
model to show names and reserve ids for link.idCol.
- Q19: a "last 7 days" query returned a 2029 row because it left the upper date
bound open. A DATA_TRAP now requires recency/relative-date queries to cap
`c.signed_at <= date('now')`, matching the existing timeseries-bounding trap.
Covered by unit tests: sanitizeCell strip (unit + binder-level proving the link
keeps the full id), and DATA_TRAPS content/count guards.
…r token cannot leak Live re-verification (Q46 „най-рисковите поръчки") showed the entity-id strip was incomplete. The contract id is a composite that embeds the bidder id mid-string — `c:e:00042-2025-0016:237236:1:eik:175405647:1`. The previous strip was anchored at `^`, so it removed the leading `c:e:` but left the embedded `:eik:…` visible when the model SELECTed the id column for display. stripEntityIdPrefix now recognises a composite id (a `c:*` prefix, or an embedded `:eik:/:name:/:auth:` token after a colon) and collapses it to its head segment — the user-facing УНП/ocid, which already equals the report's SOURCE column — dropping the embedded bidder token entirely. Whole-cell ids still strip their scheme prefix; a plain text cell that merely contains a colon (a subject line) is left intact. The DATA_TRAP now also names the composite and points the model at УНП (`t.source_id`) for a visible contract reference instead of `c.id`. Tests: composite-collapse cases (with and without the `c:e:` prefix), colon-bearing free text preserved, whole-cell strip unchanged.
Ре-ревю — последните промени (3 комита,
|
ydimitrof
left a comment
ydimitrof
left a comment
There was a problem hiding this comment.
Обобщено ревю на PR — feat(ai-assistant): conversational analytic layer (BgGPT)
Прегледано на 24 партиди. По-долу е обединеното заключение.
Какво прави PR-ът
Въвежда разговорен аналитичен слой над BgGPT за данни за обществени поръчки: асистентски „dock" (React/TypeScript UI с транскрипт, композитор, отчетни блокове, гласов вход), сървърен агент с инструменти (run_sql, find_entity, reconcile_rollup, verifier), многослойна SQL защита (структурен → AST → EXPLAIN opcode allowlist + default-filters gate), HMAC подписване на транскрипта срещу prompt-injection/credibility-laundering, anti-bot защита (Turnstile), rate-limiting и circuit-breaker, износ на отчети (Markdown/DOCX), ETL starter-prompts, миграции, provisioning скриптове, CI/deploy конфигурация и обширна документация/ADR-и.
Обща оценка
Кодът е с високо качество, отбранително написан (последователно fail-closed поведение), с добро и смислено (не тривиално) тестово покритие. Сигурността е силната страна на PR-а: параметризиран SQL, positive-allowlist AST guard, устойчив на кавички, PII денилист, XSS gate с URL allowlist, каноничен HMAC с константно-времево сравнение, маскиране на тайни в логове и без изтичане на съдържание в телеметрията. Не са открити твърдо кодирани тайни, бекдори, обфускация или нови уязвими зависимости.
Блокиращи / изискващи корекция преди merge
- (Блокиращо — покритие, партида 13/
transcribe)transcribe.test.tsе празен (+0/-0), докатоtranscribe.tsвъвежда 6 чувствителни към сигурността експортирани функции без нито един тест. Единственият нов модул без покритие. - (За проверка — коректност, същата партида)
UNSAFE_CHARSрегулярният израз изглежда съмнителен (потенциално обърнат диапазон-\^_). Без тестове това няма да бъде уловено — моля потвърдете, че компилира и наистина покрива C0/C1/bidi/zero-width знаци. - (Data-integrity, партида 20)
summaryContentвtranscript-signer.tsсъбираm.reportsот всички свити съобщения, включително неверифицираниuserсъобщения, които попадат в подписания summary. Ако ingest-пътят допускаreportsвърхуuserсъобщение, това е точно credibility-laundering векторът, който ADR-0011 иска да затвори. Препоръка: събирайreportsсамо от верифицираните server съобщения и добави тест.
Средно-важни находки (партида 22 — миграции)
0003_assistant_prompts.sql:CREATE TABLE IF NOT EXISTSможе тихо да пропусне създаването върху DB със стара форма на таблицата, оставяйки новияCHECKнеприложен (schema drift).integrity-checks.test.ts: backfill логиката на миграция0002се изпълнява върху празна таблица (no-op) и реално не се упражнява от теста.
Повтарящи се теми за потвърждение между партидите
- Тестово покритие ≥90%: освен празния
transcribe.test.ts, липсват тестове заreport.tsx/reports.tsx(валидация, стрипване наprovenance, 404 fallback). - Кръстосана верификация на зависимости: санитизацията на markdown,
entityHref, реалното съществуване на first-party/CSRF guard-а (обявен за преместен вassistant-contracts.mdv4), и идентичността на SQL низа между EXPLAIN проверката и изпълнението наrun_sql. - Нов външен адрес
https://api.bggpt.ai(BgGPT Whisper провайдър) — да се потвърди в whitelist-а на проекта.
Незадължителни наблюдения (ниска тежест)
Няколко технически дълга зад TODO(foundation-merge) (дублиране formatByHint/formatCell, ръчно огледало на контракта в contract.ts); stripLeakedMarkup per-part преди join; консервативно свръх-блокиране в denyAmplifyingStringChain; праг на isImplausibleRatio; R2 customMetadata дублира въпроса и може да провали put; валидиране формата на id в R2 ключа; Y-ос от minVal вместо 0 в TimeseriesBlock; подаване на тайни през --body на CLI (предпочете --body-file/stdin).
Заключение
Силен, добре тестван и сигурен PR по същество. Изисква корекции преди merge: празният transcribe.test.ts и неизясненият UNSAFE_CHARS (партида 13), плюс потвърждение/корекция на user.reports laundering (партида 20) и двете средно-важни находки по миграциите (партида 22). След адресирането им и потвърждаване на кръстосаните зависимости, PR-ът е готов за одобрение.
Обща присъда: REQUEST_CHANGES (заради празния тест на чувствителни функции и неизяснения regex; всички останали находки са незадължителни или за потвърждение).
| GROUP BY div | ||
| ORDER BY eur DESC | ||
| LIMIT 1; | ||
| `.trim(); |
There was a problem hiding this comment.
Минимална устойчивост (не блокиращо): SLOT2_SQL връща само топ-1 CPV дивизия (ORDER BY eur DESC LIMIT 1). Ако кодът на тази дивизия липсва в CPV_SECTORS, buildSlot2 връща null и slot 2 се пропуска изцяло — разширяването на прозореца (7→14→30) няма да помогне, защото топ дивизията най-вероятно остава същата непозната. Обмислете fallback към следващата позната дивизия (напр. LIMIT > 1 и избор на първата с известен етикет), за да не изчезва slot 2 при непокрит 2-цифрен CPV код.
| import { Link } from 'react-router'; | ||
| import { money } from '@sigma/shared'; | ||
| import type { ResolvedBlock, CellFormat } from '~/lib/assistant-contract/report'; | ||
| import { formatCell, entityHref } from '~/lib/assistant/render-format'; |
There was a problem hiding this comment.
Дребна бележка за консистентност: тук ResolvedBlock се внася от ~/lib/assistant-contract/report, докато тестът (ReportBlockRenderer.test.tsx) го внася от ~/lib/assistant/report-schema. Двата пътя сочат към един и същ re-export, така че няма функционален проблем, но README на договора изрично препоръчва лейновете да внасят ЕДИН тип през договора. Струва си да се уеднакви, за да не се заобикаля единственият източник на истина.
| truncated?: boolean; | ||
| format?: CellFormat; | ||
| }) { | ||
| if (points.length === 0) return <p className="chart-empty">Няма данни</p>; |
There was a problem hiding this comment.
Гранична стойност: max = Math.max(1, ...) и pct = (value/max)*100 не третират отрицателни стойности — при отрицателен value width става отрицателен (CSS го игнорира → празен бар). За суми по поръчки (≥0) е нисък риск, но ако диаграмата някога поеме делти/разлики, обмислете клампване към [0,100].
| const stripLeakedMarkup = (text: string): string => | ||
| text | ||
| .replace(/<(report|tool_call|tool_response)\b[\s\S]*?<\/\1>/gi, '') | ||
| .replace(/<(report|tool_call|tool_response)\b[\s\S]*$/i, ''); |
There was a problem hiding this comment.
Ниска важност (робустност): stripLeakedMarkup се прилага за всеки text part поотделно, преди extractText да ги слепи с join(''). Ако моделът при stream раздели изтекъл блок между два съседни text part-а (напр. <report> в единия и </report> в следващия), първият regex (изискващ отваряне и затваряне в един низ) няма да съвпадне, а вторият ще махне само до края на текущия низ — така затварящият таг може да остане видим в дока. Предвид MIN_LEN логиката parts често се комбинират, затова обмислете нормализацията да се прави върху вече конкатенирания текст.
Low severity (robustness): stripLeakedMarkup runs on each text part before extractText joins them. If a leaked block is split across two adjacent text parts during streaming, neither replace fully removes it and the closing tag can render literally. Consider stripping on the concatenated string instead.
| <div className="assistant-composer__actions-end"> | ||
| {canSend ? ( | ||
| <button type="button" className="assistant-composer__clear" onClick={clearDraft}> | ||
| Изчисти |
There was a problem hiding this comment.
Дребно UX: бутонът „Изчисти" се рендерира само когато canSend е true (!busy && trimmed !== ''). Ако диктуваният резултат съдържа само интервали/нови редове, trimmed === '', бутонът изчезва и потребителят не може да нулира черновата с едно действие — точно сценарият, за който clearDraft е предвиден. Обмислете показване при text !== '' && !busy.
Minor UX: the „Изчисти" (Clear) button renders only when canSend is true, so a whitespace-only dictated draft (trimmed === '') hides it, defeating the one-tap reset clearDraft was built for. Consider gating on text !== '' && !busy.
| @@ -61,6 +62,7 @@ function freshDb(): string { | |||
| const dir = mkdtempSync(resolve(tmpdir(), 'sigma-integrity-')); | |||
| const dbPath = resolve(dir, 'test.sqlite'); | |||
| readScript(dbPath, schemaPath); | |||
| readScript(dbPath, migration0002Path); | |||
There was a problem hiding this comment.
Backfill-ът на миграцията не се упражнява от този тест. Тук миграция 0002 се прилага преди CLEAN_FIXTURE (следващият ред), затова UPDATE contracts SET is_synthetic = ... FROM tenders върви върху празна таблица и е no-op. Всички редове от fixture-а, вмъкнати след това, получават DEFAULT 0 независимо от procedure_type на родителския им tender — така коректността на backfill логиката остава непокрита.
За реален тест на backfill-а: заредете fixture с поне един синтетичен tender (procedure_type = 'неизвестна') и неговите договори преди да пуснете 0002, после assert-нете is_synthetic = 1 за тях и 0 за нормалните. Иначе тестът верифицира само default-а.
| export const GATEWAY_ID = 'sigma-assistant'; | ||
| export const PROVIDER_SLUG = 'bggpt-voice'; | ||
| export const PROVIDER_NAME = 'BgGPT Voice'; | ||
| export const PROVIDER_BASE_URL = 'https://api.bggpt.ai'; |
There was a problem hiding this comment.
Нов външен адрес: https://api.bggpt.ai (custom provider за BgGPT Whisper). Моля потвърдете, че домейнът е в whitelist-а/одобрените upstream адреси на проекта — това е единственият нов изходящ адрес, въведен от партидата (освен легитимния api.cloudflare.com). Без блокиране, но заслужава изрична проверка.
| // (verified — custom providers are ACCOUNT-scoped, not under a gateway). Auth model: if | ||
| // VOICE_ASSISTANT_API_KEY is supplied we store it as the provider's Authorization header; if absent, the | ||
| // provider is created key-less (per-request-auth model — the app passes the key, like the chat `bggpt` | ||
| // provider) and we warn. Idempotent: when the provider already exists we do NOT re-PUT the secret every |
There was a problem hiding this comment.
При съществуващ provider с различен base_url кодът само предупреждава и оставя старата стойност (left as-is). Това позволява тих drift, ако конфигурацията е сгрешена веднъж — обмислете да върнете грешка/ненулев изход при несъответствие, за да не се разчита само на предупреждение в лог. Нисък приоритет.
| if (apply) { | ||
| console.log(`==> ${line}`); | ||
| try { | ||
| execFileSync('wrangler', cmd, { stdio: 'inherit' }); |
There was a problem hiding this comment.
catch {} умишлено „гълта" всички грешки при създаване на bucket-а (напр. вече съществуващ, или R2 не е активиран). Това е коректно за идемпотентност, но при истинска грешка (напр. невалиден токен) скриптът мълчаливо продължава с exit 0. Ако този скрипт се ползва в CesI gate, обмислете да разграничите „already exists" от други грешки.
| x.strategic, | ||
| -- Denormalized synthetic flag: 1 when the parent tender is a synthetic orphan header so | ||
| -- aggregate queries can filter with c.is_synthetic != 1 instead of JOIN+procedure_type check. | ||
| CASE WHEN (SELECT t.procedure_type FROM tenders t WHERE t.id = 't:' || x.unp) = 'неизвестна' |
There was a problem hiding this comment.
Маловажно / за потвърждение: тук is_synthetic се изчислява спрямо тендера 't:' || x.unp, докато precompute.sql и rollup-ите джойнват по c.tender_id (JOIN tenders t ON t.id = c.tender_id). Денормализираният флаг съвпада с JOIN-а само ако c.tender_id за c:e:/c:o: договорите винаги е точно 't:' || x.unp. Ако това е инвариант — предлагам кратък коментар, който го фиксира; ако в бъдеще се въведе lot-суфиксен tender_id, флагът ще се размине с агрегатните заявки и синтетични редове могат да изтекат (или обратно — валидни да бъдат изключени).
…mary
summaryContent folded `reports` from every collapsed message, including user
messages — which trimTranscript folds unverified (only role-labeled prose). A
user message's report refs are re-derived from client-controlled parts, so a
crafted `{id,title}` would be merged into the signed summary's `доклади:` list
with no role attribution and verify as authentic server history on the next
turn — the ADR-0011 credibility-laundering vector, in the report channel the
prose role-labeling otherwise closes.
Collect report chips only from verified server messages (`role !== 'user'`;
foldable is user + HMAC-verified assistant/tool, so that equals "verified").
User prose is still folded and role-labeled; only its report chips are dropped.
The path is currently latent (trimTranscript is unwired — client-side condense
is live and already role-gates report extraction), so this hardens E2 before it
reaches the request path. Discriminating test proves the fabricated user ref
never enters the summary while a verified server ref still does; sensitivity
confirmed (test fails without the fix).
ydimitrof
left a comment
ydimitrof
left a comment
There was a problem hiding this comment.
Обобщен преглед на PR — feat(ai-assistant): conversational analytic layer (BgGPT)
ВЕРДИКТ: COMMENT — няма блокиращи проблеми по сигурността в нито една партида; кодът е с високо качество и силна защитна нагласа. Финалното APPROVE зависи от потвърждение на CI (100% тестове/lint), покритие ≥90% и няколко точки за уеднаквяване преди merge.
Какво прави PR-ът
Въвежда разговорен аналитичен слой (BgGPT) над данните за обществени поръчки — чат-асистент, който превръща въпроси на естествен език в проверени, структурирани аналитични отчети. Обхваща:
- Фронтенд: чат-док (
assistant-dock), рендиране на отчети (bar/flows/timeseries блокове), санитизиран markdown, гласов вход (Turnstile-gated транскрибиране), експорт в DOCX, достъпност (WCAG 2.2 AA). - Бекенд/агент: agent loop с толерантна
emit_reportсхема, трислойна read-only SQL ограда (структурна → AST allowlist → EXPLAIN-opcode проверка), задължителни default филтри, темпорален резолвер, verifier за защита от prompt injection, HMAC-подписване на транскрипта, дедупликация/single-flight кеш, rate-limit и Denial-of-Wallet circuit breaker. - Инфраструктура: DB миграции (денормализиран
is_syntheticфлаг), provisioning скриптове, CI/deploy hardening, ADR-та и обширна документация.
Сигурност — чисто във всички 24 партиди
- Тайни: няма твърдо кодирани; всички ключове идват от env/
wrangler secret; dev-placeholder-ите са ясно обозначени. - SQL инжекция: всички заявки са параметризирани; моделно-генерираният SQL минава през fail-closed, quoting-proof трислойна ограда с PII denylist и memory-amplification guard.
- XSS: markdown се рендира без
dangerouslySetInnerHTML;hrefминава през allowlist (sanitizeLinkHref/isSafeHref) с adversarial тестове. - Криптография/интегритет: HMAC-SHA-256 с length-prefixed канонично кодиране и constant-time сравнение затваря „credibility laundering"; verifier може само да маха prose блокове, никога да вмъква, с fail-closed поведение.
- Fail-closed навсякъде: Turnstile, HMAC гейт, rate-limit, DoW breaker, feature kill-switch (по подразбиране OFF), gateway-only routing.
Тестовете са смислени и adversarial (не тривиални); покритието на новия код изглежда високо.
Точки за адресиране преди merge (не-блокиращи, но важни)
- Чупеща смяна на конфигурацията (Batch 7): преименувани env променливи (
BGGPT_API_KEY → ASSISTANT_API_KEY,BGGPT_MODEL → ASSISTANT_MODEL);AI_GATEWAY_BASE_URLвече е задължителен. Нужна е миграционна бележка и проверка, че секретите са налични във всички среди — иначе deploy fail-ва (503). - Несъответствие
is_synthetic↔procedure_type(Batch 8, 22, 24): каноничните заявки изискватc.is_synthetic != 1, но част от golden фикстурите изключват синтетичните чрезt.procedure_type != 'неизвестна'— рискуват CI провал. Освен товаNULL != 1в SQLite дава NULL: потвърдете колоната да еNOT NULL DEFAULT 0(или ползвайтеIS NOT 1), за да не изпаднат тихо реални договори от публичните суми. - Подвеждаща фикстура 06 (Batch 8): зарежда
regionкато NUTS3 код (BG411), което противоречи на самата промяна в схемата (region = ИМЕ). - Липсващо тестово покритие за проверка (Batch 13, 14, 15): новата логика в
assistant.chat.tsx(dedup, подписване, wiring) иverifier.tsнямат тестове в своите партиди — потвърдете, че са покрити другаде. - Приватност/GDPR (Batch 7): R2 provenance съхранява въпроса и snapshot на резултатите (възможни ЕИК/имена); provenance се маха преди хидратация към клиента, но остава на storage — потвърдете retention/достъп.
- Несъответствия в документацията (Batch 20, 24): уеднаквете import командите и поведението на
ASSISTANT_SEED_TOKEN/reindex; проследете риск #134 (публичен preview + код-ниво SQL guard + read-only D1 binding) като production launch-gate.
Дребни, незадължителни бележки
Разпръснати hardening предложения: по-строга валидация на reportId формат в localStorage (Batch 5), bidi isolate знаци в sanitizeName (Batch 1), capEvidence да cap-ва и facts блокове (Batch 13), таймаут за external BgGPT STT fetch (Batch 14), потенциални false-positive-и в touchesRollupTable/stripEntityIdPrefix/denyAmplifyingStringChain (Batch 9, 12), TODO(foundation-merge) шимове да се вържат към тикети (Batch 4, 18).
Заключение
Много добре проектиран, задълбочено тестван и OWASP-съобразен PR с изключително силна защитна и data-integrity нагласа. Няма открити уязвимости, бекдори или блокиращи дефекти в нито един от 24-те прегледани среза. Препоръчвам merge след адресиране на config-миграцията и уеднаквяването на is_synthetic семантиката/фикстурите, плюс потвърждение на зелен CI и покритието на не-прегледаните пътища.
| .normalize('NFC') | ||
| // zero-width (U+200B–200F), bidi overrides (U+202A–202E), line/para separators (U+2028/U+2029) | ||
| .replace(/[\u200B-\u200F\u202A-\u202E\u2028\u2029]/g, '') | ||
| .replace(/\s+/g, ' ') |
There was a problem hiding this comment.
Ниска тежест (defence-in-depth): регулярният израз премахва bidi-override знаците U+202A–202E, но не и по-новите bidi isolate знаци U+2066–2069 (LRI/RLI/FSI/PDI) и U+061C (ARABIC LETTER MARK). Име от подадения feed може да ползва изолати за визуален spoofing в етикета. Предложение: разшири класа, напр. добави - към заместването.
| JOIN tenders t ON t.id = c.tender_id | ||
| WHERE c.amount_eur IS NOT NULL | ||
| AND c.bids_received IS NOT NULL AND c.bids_received >= 1 | ||
| AND t.procedure_type <> 'неизвестна' |
There was a problem hiding this comment.
За сведение: t.procedure_type <> 'неизвестна' изключва от знаменателя и редовете с NULL procedure_type (защото NULL <> ... → NULL → изключено), не само буквалната стойност „неизвестна". Това вероятно е по замисъл (NULL също е „неизвестна"), но си струва изричен коментар и покриващ тест с NULL procedure_type, за да не се приеме по-късно за бъг.
| {items.map((it, ii) => ( | ||
| <li key={ii}>{renderInline(it)}</li> | ||
| ))} | ||
| </ul>, |
There was a problem hiding this comment.
Козметично: подреденият списък се рендира като <ol> без start, така че маркер 10. в средата няма да отрази началния номер (списъкът винаги брои от 1). Извън обхвата по документацията, но ако по-късно потрябва запазване на номерацията, ще е нужен start/value.
| format?: CellFormat; | ||
| }) { | ||
| if (points.length === 0) return <p className="chart-empty">Няма данни</p>; | ||
| const max = Math.max(1, ...points.map((p) => p.value)); |
There was a problem hiding this comment.
Дребна бележка за робустност (незадължителна): max = Math.max(1, ...values) в комбинация с pct = (value / max) * 100 ще даде отрицателна ширина на лентата, ако блокът някога получи отрицателни стойности. Днес данните са парични (≥ 0), затова не е проблем, но BarBlock е генеричен компонент. Помислете за Math.max(1, ...points.map(p => Math.abs(p.value))) или клампване на pct към [0, 100], за да остане устойчив при бъдещи типове данни (напр. делти).
| > | ||
| {message.role === 'assistant' ? ( | ||
| <MarkdownBlock md={text} className="assistant-message__text" /> | ||
| ) : ( |
There was a problem hiding this comment.
Тук се рендерира генериран от модела markdown през MarkdownBlock. Тестовете в този пакет доказват, че суров HTML/<script> остава инертен, но не покриват markdown-връзки със злонамерена схема, напр. [кликни](javascript:alert(1)) или data:-URL — класически XSS вектор през markdown. Моля, потвърдете, че MarkdownBlock allow-list-ва href схемите (http/https/mailto) и отхвърля javascript:/data:, и добавете тест за това. Тъй като MarkdownBlock е в друг пакет, отбелязвам го за проследяване, а не като блокер тук.
| FROM tenders t | ||
| WHERE t.id = contracts.tender_id; | ||
|
|
||
| CREATE INDEX idx_contracts_is_synthetic ON contracts(is_synthetic) WHERE is_synthetic = 0; |
There was a problem hiding this comment.
Частичният индекс покрива честата стойност (WHERE is_synthetic = 0), която важи за преобладаващото мнозинство редове. На практически константна колона това е индекс, близък до цялата таблица — носи write-cost без реална селективност, а публичните агрегати филтрират is_synthetic != 1 върху вече индексираните bidder_id/authority_id join-ове и не разчитат на него. По-полезно е да индексирате рядката стойност: WHERE is_synthetic = 1 (малък, селективен индекс за изолиране/изключване на синтетичните записи в integrity/admin проверки).
Забележка: текущото състояние на репото вече използва WHERE is_synthetic = 1 с точно този коментар — ако този diff е изостанал спрямо HEAD, наблюдението е решено и може да се игнорира.
| export const GATEWAY_ID = 'sigma-assistant'; | ||
| export const PROVIDER_SLUG = 'bggpt-voice'; | ||
| export const PROVIDER_NAME = 'BgGPT Voice'; | ||
| export const PROVIDER_BASE_URL = 'https://api.bggpt.ai'; |
There was a problem hiding this comment.
Нов външен базов URL (https://api.bggpt.ai), който се записва като base_url на custom provider и по който после текат аудио заявки с ключ. Моля потвърдете, че домейнът е в одобрения whitelist за изходящи повиквания и че използва HTTPS (тук е така). Положително: при --apply ключът се пази като Authorization header, а в dry-run режима dryRunFetch го маскира преди лог — така тайната не изтича в CI. Без промяна по кода — само потвърждение на whitelist.
| // namespace and create a duplicate that silently splits the cache. Returns [{ id, title }]. | ||
| export async function listNamespaces({ accountId, token, fetchImpl = fetch }) { | ||
| const out = []; | ||
| for (let page = 1; ; page++) { |
There was a problem hiding this comment.
Дребна забележка (нисък приоритет): for (let page = 1; ; page++) спира само при result.length < PER_PAGE. На практика е коректно, но липсва горна граница/предпазител — ако API някога върне пълна финална страница без празна следваща, цикълът не завършва. Обмислете таван на страниците или проверка на body.result_info за по-устойчиво поведение. Не е блокиращо.
| const r = | ||
| rows( | ||
| runner, | ||
| 'SELECT' + | ||
| ' (SELECT COALESCE(SUM(amount_eur), 0) FROM contracts WHERE amount_eur IS NOT NULL) AS clean_total,' + | ||
| ' (SELECT COALESCE(SUM(c.amount_eur), 0) FROM contracts c JOIN tenders t ON t.id = c.tender_id JOIN authorities a ON a.id = t.authority_id WHERE c.amount_eur IS NOT NULL) AS auth_attr,' + | ||
| ' (SELECT COALESCE(SUM(c.amount_eur), 0) FROM contracts c JOIN tenders t ON t.id = c.tender_id JOIN authorities a ON a.id = t.authority_id WHERE c.amount_eur IS NOT NULL AND c.is_synthetic != 1) AS auth_attr,' + |
There was a problem hiding this comment.
Забележка за целостта на данните: c.is_synthetic != 1 в SQLite изключва и редове с is_synthetic IS NULL (NULL != 1 → NULL, не TRUE). Днешните пътища на запис не дават NULL (normalize връща 0/1; refresh-slice използва COALESCE(...,0)), но ако схемната миграция е добавила колоната без NOT NULL DEFAULT 0, съществуващи редове ще са NULL и ще изпаднат тихо от auth_attr/bidder_attr И от съответните rollup суми — рекончилиацията пак ще мине (двете страни филтрират еднакво), маскирайки дрейфа. Препоръка: колоната да е NOT NULL DEFAULT 0, или ползвайте c.is_synthetic IS NOT 1.
| x.strategic, | ||
| -- Denormalized synthetic flag: 1 when the parent tender is a synthetic orphan header so | ||
| -- aggregate queries can filter with c.is_synthetic != 1 instead of JOIN+procedure_type check. | ||
| CASE WHEN (SELECT t.procedure_type FROM tenders t WHERE t.id = 't:' || x.unp) = 'неизвестна' |
There was a problem hiding this comment.
Тук is_synthetic се извежда чрез повторно построяване на tender id ('t:' || x.unp), а не чрез действителния tender_id на договора. В refresh-slice.sql авторитетният път join-ва по t.id = contracts.tender_id. Ако някога родителският tender id се различава от 't:'||x.unp (напр. синтетично заглавие с друг id), двата източника ще се разминат. Моля потвърдете, че за всички c:e:/c:o: редове tender_id = 't:'||x.unp.
# Conflicts: # apps/web/app/root.tsx # apps/web/vitest.config.ts # pnpm-lock.yaml
ydimitrof
left a comment
ydimitrof
left a comment
There was a problem hiding this comment.
Ревю на PR: feat(ai-assistant): conversational analytic layer (BgGPT)
Какво прави PR-ът
Този PR добавя разговорен аналитичен слой („BgGPT") — AI асистент, който отговаря на въпроси на естествен език за обществени поръчки и генерира структурирани, проверими отчети. Обхватът е широк (~50+ файла, прегледани в 24 партиди) и включва:
- Frontend слой (
assistant-dock): React чат-док, панел за отчети с достъпни chart блокове (bar/flows/timeseries, изложени и като реални<table>за екранни четци, WCAG 1.1.1/4.1.3), безопасен markdown рендер, гласов вход, starter prompts, dedup chip-ове и локална персистенция с narrowing на недоверени данни. - Backend слой (Cloudflare Workers): агент-оркестрация с детерминистична верификация,
run_sqlинструмент с многослойна защита, темпорален слой, транскрипция (Whisper/BgGPT STT), HMAC подписване на транскрипта, dedup/single-flight и rate-limiting. - Инфраструктура: DB миграции (synthetic-flag), deploy/provisioning скриптове, AI Gateway маршрутизация, kill-switch, ADR-та и обширна документация.
Обща оценка
Кодът е с последователно високо качество — добре документиран, с обмислена fail-closed нагласа и смислени, адверсариални тестове (не тривиални). Защитата в дълбочина е образцова на много места.
Сигурност (Phase 0) — ЧИСТО във всички партиди
- Няма твърдо кодирани тайни (всички ключове минават през
wrangler secret put/ env; placeholder-и в примерите). - Няма зловреден код, backdoor-и, обфускация или code injection.
deploy.ymlдори затваря вектор за shell-инжекция. - SQL инжекция: целият достъп до D1 е параметризиран.
run_sqlминава през солидна fail-closed верига (L1 структурна → L2 AST allowlist → L3 EXPLAIN-opcode +assertDefaultFilters+ rows-read бюджет + wall-time timeout). - XSS: изходът от модела минава през
MarkdownBlock/sanitize-markdownс href-санитизация и безdangerouslySetInnerHTML. - LLM-специфични защити: spotlighting fence с per-call nonce, strip-only верификатор (никога не вмъква съдържание), server-owned числа (VALUES_BY_REFERENCE), HMAC интегритет на транскрипта срещу replay/splice/credibility-laundering, Turnstile + circuit-breaker срещу denial-of-wallet.
Блокиращи / важни находки (за адресиране преди merge)
-
[Блокер — покритие]
transcribe.test.tsе празен (batch 13). Файлътtranscribe.tsтвърди, че е unit-тестван, но тестовете са с 0 реда — чувствителна логика (base64 валидация, MIME allowlist, byte-cap стрийминг, санитизиране) остава без покритие, което нарушава гейта ≥90%. -
[Коректност] Възможно деформиран
UNSAFE_CHARSregex вtranscribe.ts(batch 13). Двусмислен диапазон, който при буквална интерпретация може да хвърлиSyntaxErrorпри зареждане на модула или да изтрива легитимни ASCII символи. Липсата на тестове (т.1) го прави невидим. -
[Целостност на данните — потвърдете]
refresh-slice.sql(batch 24): rollup-ите (company_totals/authority_totals) са scope-нати към различни „touched" множества отis_syntheticUPDATE-а; ако флагът се обърне, но bidder/authority не е в съответното множество, rollup-ът задържа стара стойност. Освен товаc.is_synthetic != 1изключва и NULL редове — коректно само ако колоната еNOT NULL DEFAULT 0.
Точки за потвърждение (кръстосани зависимости / изнесени гаранции)
- Launch-gates от документацията (batch 21): физическият read-only D1 binding (
DB_RO, #134) все още липсва — защитата наrun_sqlднес е само на код-ниво; account-wide DoW circuit-breaker и Turnstile върху/assistant/transcribeса маркирани като hard launch-gate. - Golden фикстури срещу правилата (batch 8): фикстура 13 показва
c.idвъпреки правилото „използвай УНП, неc.id"; фикстура 06 ползва NUTS3 кодове вместо имена; няколко фикстури филтрират поprocedure_type != 'неизвестна'вместо каноничнияc.is_synthetic != 1— потвърдете, че gate-ът приема и двете форми. - Достъп до
/reports(batch 2):idне е граница за поверителност и/reportsизброява всички ID-та — потвърдете дали е приемливо спрямо чувствителността на данните. - Липсващ export (batch 9):
isImplausibleRatioот./report-schemaсе внася, но не е в прегледаните партиди — потвърдете, че съществува, иначе билдът пада. - Документация за CSRF/first-party guard (batch 22): v4 на contracts spec-а премахва документацията за 403/415/405 guard-овете — уверете се, че guard-овете реално още съществуват в кода.
Дребни, неблокиращи наблюдения
formatElapsedTimeпоказва невалиден0:60при точно 60 сек (batch 3).- Гола 4-цифрена сума („над 2000 лева") се разпознава като година в темпоралния слой → тих скрит филтър по
signed_at(batch 11). - Unmount race в
useVoiceInput.transcribe(изтекла заявка + React warning, batch 6). hasValidSlotне валидира елементите на масива →TypeErrorприreports: [null]вместо грациозно отпадане (batch 19).- Изтекла ревюърска анотация „(review, ydimitrof)" в постоянен коментар на
dedup.ts:90(batch 18). - Частичен индекс в миграция
0002върху честата стойностis_synthetic=0вероятно няма да се ползва; backfill-ът не е батчиран и не е re-apply safe (batch 22). renderJsonтихо пропуска stamp наBUILD_ID/ASSISTANT_ENABLED/ENVIRONMENTпри липсващvarsблок (batch 24).- Отрицателни стойности → отрицателна ширина на лентата в
BarBlock(batch 2); O(n²)trimMessages/JSON.stringifyпри текущи лимити е пренебрежимо (batch 5).
Заключение
Няма открити експлоатируеми уязвимости по сигурността в нито една партида — архитектурата е зряла и защитно написана. Преди merge трябва да се адресират: празният transcribe.test.ts и съмнителният UNSAFE_CHARS regex (batch 13), както и двете точки за целостта на rollup данните (batch 24). Препоръчително е също да се потвърдят launch-gate елементите (физически read-only D1, DoW breaker, Turnstile на transcribe) и разминаванията в golden фикстурите преди ASSISTANT_ENABLED=true. Цялостното покритие ≥90% и статусът на CI трябва да се верифицират на ниво целия PR.
| */ | ||
| export function slot1OutlierSuppressed(top: number, second: number | undefined): boolean { | ||
| if (second === undefined) return false; | ||
| return top >= SLOT1_OUTLIER_RATIO * second; |
There was a problem hiding this comment.
Гранична проверка с ниска тежест: slot1OutlierSuppressed защитава срещу second === undefined, но не и срещу second === 0. Ако вторият по големина договор в прозореца има amount_eur = 0 (валидна възможност — филтърът е само amount_eur IS NOT NULL, без > 0), то top >= SLOT1_OUTLIER_RATIO * 0 става top >= 0, което е винаги истина за положителен top. Резултат: името на институцията се потиска излишно и slot 1 пада към fallback без име, макар да няма реален outlier. Освен това редът за лог ratio: list[1] ? top.amount_eur / list[1].amount_eur : null ще даде Infinity при нулев list[1].amount_eur. Предложение: третирайте second <= 0 като „няма runner-up“ (if (second === undefined || second <= 0) return false;).
| export const STORED_REPORT_SCHEMA_VERSION = 1 as const; | ||
|
|
||
| export interface StoredReport { | ||
| schemaVersion: typeof STORED_REPORT_SCHEMA_VERSION; |
There was a problem hiding this comment.
Сигурност / контрол на достъпа (OWASP A01): коментарът заявява, че id не е граница за поверителност и че /reports изброява всички ID-та. Ако съхранените отчети могат да съдържат чувствителни аналитични данни за реални субекти, това ефективно ги прави публични чрез изброяване. Моля потвърдете спрямо изискванията на тикета: (1) дали изброяването на /reports е умишлено публично; (2) дали е нужна авторизация на ниво отчет/собственик. Ако отчетите са предназначени да са частни, случайният id не е достатъчна защита.
| format?: CellFormat; | ||
| }) { | ||
| if (points.length === 0) return <p className="chart-empty">Няма данни</p>; | ||
| const max = Math.max(1, ...points.map((p) => p.value)); |
There was a problem hiding this comment.
Ръбов случай (ниска тежест): max = Math.max(1, ...values) и по-долу pct = (p.value / max) * 100. При отрицателни стойности pct става отрицателен, което дава невалидна CSS ширина (лентата изчезва), а при смесени знаци пропорциите се изкривяват. Ако отрицателни стойности са възможни за bar блок, обмислете нормализиране спрямо Math.max(1, ...values.map(Math.abs)) или изрично отрязване до [0, 100].
| The SVG is aria-hidden; screen readers and text-only mode use this table instead. | ||
| AccessibilityWidget's SURVIVAL_CSS reveals .ts-data-table when text-only is active. */} | ||
| <table className="ts-data-table" aria-label="Данни от времевата редица"> | ||
| <thead> |
There was a problem hiding this comment.
Забележка за бъдеща устойчивост: таблицата за данни подравнява многосерийните клетки по индекс (s.pts[i]), приемайки, че всички серии споделят един и същ индекс на периодите. Коректно за единична серия (единствената емитирана днес и покрита от тестовете), но при бъдещо разрешаване на multi-series серии с различни/липсващи периоди клетките ще се разминат. Стойност: добавете тест за multi-series подравняване, когато контрактът започне да емитира series.
| /** Elapsed recording seconds → a short `0:SS` clock (capped at the 60s recording limit). */ | ||
| const formatElapsedTime = (seconds: number): string => | ||
| `0:${String(Math.min(seconds, 60)).padStart(2, '0')}`; | ||
|
|
There was a problem hiding this comment.
Дребна забележка (нисък приоритет): при точно 60 секунди Math.min(seconds, 60) показва 0:60, което не е валиден часовник — очаква се 1:00. Ако записът се спира автоматично на 60s, потребителят все пак може да види 0:60 за един тик. Обмислете форматиране като минути:секунди или спиране на брояча на 59.
| SIGMA_CSV_CACHE_NAME=sigma-csv-cache-dev node scripts/bootstrap-r2.mjs --apply | ||
| ``` | ||
|
|
||
| Copy the **`database_id`** UUID that `wrangler d1 create` prints — that is `SIGMA_D1_ID` for both |
There was a problem hiding this comment.
Дребно несъответствие команда/текст: тук се казва, че database_id се отпечатва от wrangler d1 create, но показаната команда в Step 2 (ред 34) е node scripts/bootstrap.mjs --apply. Ако bootstrap.mjs обвива wrangler d1 create, добре е да се уточни (напр. „което bootstrap.mjs изпълнява вътрешно“), за да не търси операторът отделна wrangler d1 create стъпка.
| | Preview | `https://sigma-pr-22.midt-crew.workers.dev` (Worker `sigma-pr-22`, shared **dev** D1/R2, read-only) | | ||
| | Branch / PR | `feat/deterministic-date-resolution` → #22 | | ||
| | Data coverage | 2020–2026 (2026 partial); last contract 01.07.2026; data refreshed 02.07.2026 | | ||
| | Model | `google/gemma-4-31b-it` (~31B) via Cloudflare AI Gateway → OpenRouter, temp 0.1, max 6 tool steps | |
There was a problem hiding this comment.
Несъответствие в маршрутизацията на модела спрямо останалата документация в PR-а: тук пише „via Cloudflare AI Gateway → OpenRouter“, докато docs/deploy-assistant.md §2 описва custom BgGPT провайдъри към https://api.bggpt.ai (не OpenRouter), а PR-ът е озаглавен BgGPT. Вероятно този тестов отчет е от по-ранната фаза (branch feat/deterministic-date-resolution, #22) преди преминаването към BgGPT. Струва си кратка бележка „(историческо: тогава през OpenRouter; сега BgGPT)“, за да не подведе бъдещ читател за действителния провайдър.
| FROM tenders t | ||
| WHERE t.id = contracts.tender_id; | ||
|
|
||
| CREATE INDEX idx_contracts_is_synthetic ON contracts(is_synthetic) WHERE is_synthetic = 0; |
There was a problem hiding this comment.
Частичният индекс индексира мнозинството редове: is_synthetic = 0 покрива нормалните договори (по-голямата част от таблицата). Заявки, филтриращи is_synthetic = 0 (напр. Slot 4 в starter-prompts), връщат почти всичко → планировчикът вероятно ще избере full scan и този индекс няма да се използва. Ако целта е бързо изключване на синтетичните редове, индексът обикновено се прави за рядката стойност (WHERE is_synthetic = 1). Моля потвърдете намерението.
| -- aggregate queries can filter synthetic orphan contracts without a JOIN to tenders. | ||
| -- Populated from the parent tender at insert time in normalize-raw.sql; 1 for synthetic tenders | ||
| -- (procedure_type='неизвестна'), 0 for all normal tenders. | ||
| ALTER TABLE contracts ADD COLUMN is_synthetic INTEGER NOT NULL DEFAULT 0; |
There was a problem hiding this comment.
За разлика от 0003, който умишлено е re-apply safe (CREATE TABLE IF NOT EXISTS), този ALTER TABLE ... ADD COLUMN ще падне с „duplicate column name" при повторно прилагане. Приемливо, ако migration runner-ът гарантира еднократно изпълнение, но е несъответствие в стила спрямо 0003. Освен това backfill UPDATE минава цялата таблица наведнъж — струва си да се провери спрямо ~30s CPU лимита на D1 при голяма contracts.
| -- NULL-safe: `value_flag <> 'annex_suspect'` alone drops NULL rows (NULL <> x is NULL, not TRUE), | ||
| -- which would exclude most normal contracts. The implemented SLOT1_SQL keys on `value_flag = 'ok'`, | ||
| -- which sidesteps the same NULL trap; keep this predicate NULL-safe if the spec form is ever used. | ||
| ORDER BY c.amount_eur DESC LIMIT 5; -- top-5 so the job can sanity-check the distribution |
There was a problem hiding this comment.
Коментарът твърди, че имплементираният SLOT1_SQL ползва value_flag = 'ok' и така „заобикаля NULL капана". Внимание: ако нормалните договори имат value_flag IS NULL (а не буквално 'ok'), тогава value_flag = 'ok' изхвърля повечето валидни реда — това е обратният NULL капан и би подбрал грешен „най-голям договор". Моля сверете реалния SLOT1_SQL спрямо разпределението на value_flag в данните (NULL vs 'ok') в съответната кодова партида.
…gler-render Shared-logic parity with the contracts deploy line: wrangler-render.mjs gains the `aiGatewayAccount` + `turnstileSiteKey` swap knobs. Inert on this line (no workflow passes SIGMA_AI_GATEWAY_ACCOUNT / SIGMA_TURNSTILE_SITE_KEY, and the committed gateway URLs are empty), but the render logic must stay byte-identical across feat/contracts. Distilled from cefothe's #79.
…-integrity gate Parity reflect of the contracts fix — keeps wrangler-render.mjs byte-identical. The comment's docs/dev-preview-account-split.md reference dangled here too (the runbook isn't on feat); made it self-contained. Knob behaviour unchanged.
ydimitrof
left a comment
ydimitrof
left a comment
There was a problem hiding this comment.
Финален преглед на PR — feat(ai-assistant): conversational analytic layer (BgGPT)
ОБЩ ВЕРДИКТ: COMMENT — няма блокиращи проблеми по сигурността в нито една от 24-те партиди; препоръчва се адресиране на няколко потвърждения по целостта на данните и дребни бележки преди финално APPROVE.
Какво прави PR-ът
Въвежда разговорен аналитичен слой („BgGPT") върху данните за обществени поръчки. Обхватът включва:
- Клиентски слой (React/TypeScript): асистент-док с чат, гласов вход, starter-промпти, рендер на справки (таблици, времеви серии, callout-и), достъпен по WCAG, с персистиране в localStorage и Turnstile гейт.
- Агентски/сървърен слой: LLM оркестрация през Cloudflare AI Gateway (fail-closed), инструменти
run_sql/find_entity/reconcile_rollup, многослойна read-only SQL защита, verifier роля, HMAC-подписване на транскрипта, транскрибиране на глас, rate-limiting и circuit breaker. - Данни/ETL:
is_syntheticфлаг за изключване на синтетични договори от rollup-ите, reconciliation tripwires, миграции. - Инфраструктура: provisioning скриптове (KV/R2/AI-Gateway/secrets), wrangler конфиг, CI deploy hardening, обширна документация и ADR-и (0007–0013).
Силни страни
- Сигурност по дизайн (Phase 0 чист навсякъде): няма твърдо кодирани тайни (само dev-плейсхолдъри и тестови фикстури), няма backdoor/обфускация/
eval, няма нови съмнителни зависимости. Всички секрети минават презwrangler secret put. - SQL защита в дълбочина: структурна проверка → AST guard с positive allowlist на функции и PII denylist → EXPLAIN-opcode проверка на компилирания план → задължителни default-филтри. Всички заявки са параметризирани; няма конкатенация на потребителски вход.
- XSS/markdown защита: React екраниране, изключен raw-HTML passthrough, allowlist за href (
isSafeHref/sanitizeLinkHref), CSP+nonce, адверсариални тестове. - Prompt-injection защита: spotlighting fence с per-call nonce, verifier с claim-id канал,
stripLeakedMarkup. - HMAC транскрипт (anti-„credibility laundering"): HMAC-SHA-256, канонично length-prefix кодиране, сравнение в константно време, покрити replay/reorder/cross-conversation/re-point вектори.
- Fail-closed политика за prod/staging при HMAC/Turnstile/rate-limit; DoW backstop.
- Тестово покритие: последователно високо, смислено и адверсариално (golden фикстури, SQL срещу реални миграции, forgery/tamper тестове) — не тривиални.
Нужни потвърждения преди финално APPROVE (целостта на данните)
Тези точки се повтарят в няколко партиди и заслужават внимание:
- Крехък sentinel
procedure_type = 'неизвестна'заis_synthetic(batch 23): използва се локализиран магически низ като бизнес-сигнал. Ако реален tender от източника има тази стойност, реален договор ще бъде мълчаливо изключен отcompany_totals/authority_totals/sector_totalsи от reconciliation → занижени публични суми. Препоръка: структурен признак вместо текстово сравнение. - Семантика на
is_synthetic != 1при NULL (batch 17/22/23):!= 1изключва и NULL редове; асиметрия е възможна, ако бъдещ insert остави NULL. Потвърдете, че колоната еNOT NULL DEFAULT 0; обмислетеIS NOT 1. - Дублирана дефиниция на
is_syntheticвnormalize-raw.sqlиrefresh-slice.sql— да се синхронизират, иначе reconciliation ще се чупи според пътя на изпълнение. - Backfill на
is_synthetic(миграция 0002) не се тества (batch 22): логиката е практически no-op в тестовия setup. - Golden фикстури vs. схема (batch 8): региони като NUTS3 кодове vs. име на област; стар филтър
procedure_type != 'неизвестна'vs. новияis_synthetic != 1— да се потвърди, чеassertDefaultFiltersприема двата варианта, иначе golden стъпки ще паднат.
Дребни бележки (незадължителни, неблокиращи)
- Дублиране на код зад
TODO(foundation-merge)(contract.ts,report-projection.ts/formatByHint): два източника на истина за форматиране/типове — да се обвърже с тикет за изтриване при merge на foundation. - Outlier гард при нулев runner-up (
suggested-prompts.ts):top >= RATIO * 0даваInfinity/фалшиво потискане — третирайтеsecond <= 0като „няма съпоставим". - Микрофонът остава активен при
busyturn (AssistantComposerMic); празен масив промпти не активира fallback (AssistantEmptyState). STREAM_TIMEOUT_MSе абсолютен, не idle-based — дълга легитимна генерация >90s се прекъсва като мрежова грешка.- Нови външни URL за whitelist:
api.bggpt.ai(гласов upstream),app.eop.bg(deep-link), Cloudflare Turnstile/Gateway домейни — легитимни, но да се добавят изрично. - Непоследователност
import.meta.env.PRODvs. runtimeENVIRONMENTмежду гейтовете — Turnstile на staging може да е fail-closed въпреки подвеждащ коментар. micrometer/O(n²) микро-неефективности (trimMessages,wrangler-renderaccount-id replace), R2customMetadataс суров въпрос на потребителя (PII/размер), timeout заявки заобикалящи rows-read бюджета — всички ниско-рискови.- Регекс
последн[а-я]*тримесечие (temporal.ts) хваща множествено число → мълчаливо грешен филтър; да се стесни до единствено число.
Заключение
Партиди 16–20 получиха APPROVE самостоятелно; останалите — COMMENT, предимно поради зависимости между партидите и изброените потвърждения по данните. Кодът е с последователно високо качество, добре документиран и защитно проектиран. Финалното APPROVE зависи от: (1) потвърждаване на семантиката и източника на is_synthetic, (2) whitelisting на новите външни URL, (3) обвързване на TODO(foundation-merge) дублиранията с тикети, и (4) преминаване на пълните CI гейтове (test, test:golden, ≥90% покритие), които не могат да се проверят на ниво отделна партида.
| */ | ||
| export function slot1OutlierSuppressed(top: number, second: number | undefined): boolean { | ||
| if (second === undefined) return false; | ||
| return top >= SLOT1_OUTLIER_RATIO * second; |
There was a problem hiding this comment.
Дребно: при runner-up с amount_eur = 0 (възможно, тъй като филтърът е само amount_eur IS NOT NULL) top >= SLOT1_OUTLIER_RATIO * second става top >= 0, което потиска името на институцията без реален outlier. Освен това по-надолу в generateSuggestedPrompts логът ratio: top.amount_eur / list[1].amount_eur ще запише Infinity при list[1].amount_eur === 0. Предложение: третирайте second <= 0 като липса на съпоставим runner-up (не потискайте и не смятайте отношение). Ефект: козметичен/наблюдаемост, не сигурност.
| html.a11y-textonly .sr-only{position:absolute!important;width:1px!important;height:1px!important;padding:0!important;margin:-1px!important;overflow:hidden!important;clip:rect(0,0,0,0)!important;clip-path:inset(50%)!important;white-space:nowrap!important;border:0!important} | ||
| html.a11y-textonly .brand-sub{display:inline-block!important;font-weight:700!important;font-size:1.1em!important} | ||
| html.a11y-textonly .skip:not(:focus){position:absolute!important;left:-9999px!important;top:-9999px!important;width:1px!important;height:1px!important;overflow:hidden!important} | ||
| html.a11y-textonly img,html.a11y-textonly svg,html.a11y-textonly video{display:none!important} |
There was a problem hiding this comment.
Премахнахте display:none!important от .sr-only в режим a11y-textonly. Съдържанието остава визуално скрито заради position:absolute;width:1px;height:1px;clip:..., така че видим ефект няма — но моля потвърдете, че промяната е нарочна (свързана с новото .ts-data-table правило), а не остатъчна редакция.
| format?: CellFormat; | ||
| }) { | ||
| if (points.length === 0) return <p className="chart-empty">Няма данни</p>; | ||
| const max = Math.max(1, ...points.map((p) => p.value)); |
There was a problem hiding this comment.
Дребна бележка (краен случай): max = Math.max(1, ...values) и pct = (value / max) * 100. При отрицателни стойности (напр. нетни/разлики) max остава 1, а pct става отрицателен → CSS width с отрицателна стойност се игнорира и лентата се рендира грешно. За текущите данни (суми ≥ 0) не се проявява, но ако диаграмата някога получи отрицателни стойности, обмислете нормализиране спрямо Math.max(1, ...values.map(Math.abs)) или изрично отхвърляне. Не е блокиращо.
| import { afterEach, describe, expect, it } from 'vitest'; | ||
| import { cleanup, render } from '@testing-library/react'; | ||
| import { ReportBlockRenderer } from './ReportBlockRenderer'; | ||
| import type { ResolvedBlock } from '~/lib/assistant/report-schema'; |
There was a problem hiding this comment.
Консистентност: тук ResolvedBlock се внася директно от ~/lib/assistant/report-schema, докато самият рендер (ReportBlockRenderer.tsx, ред 15) го внася през контракта ~/lib/assistant-contract/report. README-то на контракта постановява „импортирайте ЕДИН тип“ през re-export-а. Препоръчвам тестът също да минава през ./report, за да не се разминат при бъдещо преместване на схемата. Незадължително.
| </g> | ||
| ); | ||
| })} | ||
| </svg> |
There was a problem hiding this comment.
Бележка за бъдещата multi-series форма: редовете на таблицата се индексират по longestSeries.pts[i] за периода, а стойностите — по s.pts[i] за всяка серия. Това приема, че всички серии споделят един и същ индекс на период. Днес се излъчва само единична серия (документирано), така че не е дефект сега, но при активиране на multi-series серии с разминаващи се периоди ще се получи разместване период↔стойност. Струва си явно подравняване по ключ на период, когато формата се включи.
|
|
||
| ```bash | ||
| # secrets | ||
| gh secret set CLOUDFLARE_API_TOKEN --env dev --repo midt-bg/sigma --body '<YOUR_ROTATED_TOKEN>' |
There was a problem hiding this comment.
Незадължителна бележка за сигурностна хигиена (runbook). Задаването на секрети през gh secret set ... --body '<стойност>' поставя стойността като аргумент на командния ред, което я излага в shell history (~/.bash_history) и в списъка на процесите (ps). Тъй като този документ учи операторите как да боравят с реалните ротирани токени, по-добре е да се препоръча четене от stdin или файл, напр.:
gh secret set CLOUDFLARE_API_TOKEN --env dev --repo midt-bg/sigma < token.txt
# или интерактивно (подкана, без ехо в history):
gh secret set CLOUDFLARE_API_TOKEN --env dev --repo midt-bg/sigmaСъщото важи за редове 57–58 и за аналогичните команди в Стъпка 5. Не е блокер — placeholder-ите и предупреждението „never commit them" по-горе са добри; това е просто по-безопасен шаблон за реалните стойности.
| FROM tenders t | ||
| WHERE t.id = contracts.tender_id; | ||
|
|
||
| CREATE INDEX idx_contracts_is_synthetic ON contracts(is_synthetic) WHERE is_synthetic = 0; |
There was a problem hiding this comment.
Партиалният индекс WHERE is_synthetic = 0 индексира колона, чиято стойност в самия индекс винаги е 0 — нулева селективност спрямо филтъра is_synthetic = 0, който в slot-4 заявката пасва на почти всички редове. Практически не ускорява нищо. Ако целта е бързо изключване на синтетичните редове, по-полезен е партиален индекс върху рядката стойност (WHERE is_synthetic = 1) или композитен индекс, включващ signed_at. Не блокира — но така индексът е мъртва тежест при запис.
| -- (procedure_type='неизвестна'), 0 for all normal tenders. | ||
| ALTER TABLE contracts ADD COLUMN is_synthetic INTEGER NOT NULL DEFAULT 0; | ||
|
|
||
| UPDATE contracts |
There was a problem hiding this comment.
CREATE INDEX (ред 12) и ALTER TABLE ADD COLUMN тук нямат IF NOT EXISTS, докато 0003 ползва CREATE TABLE IF NOT EXISTS защитно. Повторно прилагане на 0002 ще се провали. Приемливо за run-once миграции, но изравнете конвенцията за идемпотентност между двата файла.
| @@ -61,6 +62,7 @@ function freshDb(): string { | |||
| const dir = mkdtempSync(resolve(tmpdir(), 'sigma-integrity-')); | |||
| const dbPath = resolve(dir, 'test.sqlite'); | |||
| readScript(dbPath, schemaPath); | |||
| readScript(dbPath, migration0002Path); | |||
There was a problem hiding this comment.
Тук миграцията 0002 се прилага върху ПРАЗНИ таблици (schema-та тъкмо е създадена), а CLEAN_FIXTURE се зарежда на следващия ред. Затова UPDATE contracts SET is_synthetic ... FROM tenders е no-op и is_synthetic за fixture редовете идва единствено от DEFAULT 0 — backfill логиката (procedure_type='неизвестна' → 1) реално не се тества. Ако интеграционна проверка разчита на коректно маркирани синтетични договори, ще даде фалшива увереност. Обмислете зареждане на fixture-а ПРЕДИ миграцията, или отделен тест, който вкарва синтетичен tender+contract и после прилага 0002, за да покрие backfill-а.
| if (typeof obj.vars[key] === 'string') { | ||
| obj.vars[key] = obj.vars[key].replace( | ||
| /(gateway\.ai\.cloudflare\.com\/v1\/)[0-9a-f]{32}/, | ||
| `$1${names.aiGatewayAccount}`, |
There was a problem hiding this comment.
Минорна забележка за твърдост (не блокира). names.aiGatewayAccount се вмъква като replacement низ в String.prototype.replace, където $ има специално значение ($1, $&, $$). За разлика от provision-environments.sh, тук стойността от process.env.SIGMA_AI_GATEWAY_ACCOUNT не се валидира преди употреба. При коректна 32-hex стойност няма проблем, но:
- Ако env стойността случайно съдържа
$, замяната ще произведе тих, изкривен URL. - По-важно — липсва проверка на формата: сгрешена стойност пасва на
if (names.aiGatewayAccount), но regexът[0-9a-f]{32}няма да съвпадне и URL-ът ще остане със стария account id, без предупреждение — точно обратното на намерението при deploy към друг акаунт.
Препоръка: валидирайте ^[0-9a-f]{32}$ (както се валидират reviewer-ите в provision-environments.sh) и/или подайте функция за замяна вместо низ: .replace(/(...)[0-9a-f]{32}/, (_, p1) => p1 + names.aiGatewayAccount). Същата бележка важи по-слабо за buildId и turnstileSiteKey — обмислете леко валидиране за консистентност с строгия стил на останалата част от PR-а.
ydimitrof
left a comment
ydimitrof
left a comment
There was a problem hiding this comment.
Not logged in · Please run /login
| const stripped = s | ||
| .normalize('NFC') | ||
| // zero-width (U+200B–200F), bidi overrides (U+202A–202E), line/para separators (U+2028/U+2029) | ||
| .replace(/[\u200B-\u200F\u202A-\u202E\u2028\u2029]/g, '') |
There was a problem hiding this comment.
Защита в дълбочина (не блокиращо): регулярният израз премахва bidi override знаците (U+202A–U+202E), но не и bidi isolate знаците U+2066–U+2069 (LRI/RLI/FSI/PDI), нито U+061C (ALM) и U+FEFF (BOM/zero-width no-break space). Именно комбинацията override + isolate е класическият Trojan Source вектор (CVE-2021-42574) за визуален спуфинг на имена на институции. Тъй като този слой е обявен като защита срещу bidi/zero-width спуфинг, предлагам да разширите класа, напр.: /[---
]/g, и да обновите коментара/JSDoc-а съответно. XSS остава покрит от React ескейпването, така че това е само за целостта на визуализацията.
|
|
||
| export interface StoredReport { | ||
| schemaVersion: typeof STORED_REPORT_SCHEMA_VERSION; | ||
| id: string; // random, unguessable — do not treat as a privacy boundary; /reports enumerates all IDs |
There was a problem hiding this comment.
Бележка по сигурност/поверителност (не блокира тази партида): коментарът гласи, че id е случаен и непредвидим, но /reports изброява всички ID-та, т.е. случайният ID НЕ е граница на достъп. Ако справките съдържат чувствителни изводи за реални субекти, това означава, че всяка съхранена справка е достъпна за всеки, който има достъп до списъка. Моля потвърдете, че това е умишлено (публични данни) и че маршрутът /reports/:id прилага подходяща авторизация — това трябва да се провери в партидата с самия route (OWASP A01: Broken Access Control).
| if (points.length === 0) return <p className="chart-empty">Няма данни</p>; | ||
| const max = Math.max(1, ...points.map((p) => p.value)); | ||
| const rows = points.map((p) => ({ | ||
| label: p.label == null || p.label === '' ? '—' : String(p.label), |
There was a problem hiding this comment.
Дребно: pct: ((p.value / max) * 100) при отрицателни value дава отрицателна ширина на лентата (max е клампнато на минимум 1, но не и стойностите). В текущия домейн (парични суми ≥ 0) няма ефект, но при бъдещи метрики със знак си струва Math.max(0, ...) за ширината.
| // The SVG scales responsively via a fixed viewBox and CSS `width: 100%`. | ||
|
|
||
| import type { CellFormat } from '~/lib/assistant-contract/report'; | ||
| import { formatCell } from '~/lib/assistant/render-format'; |
There was a problem hiding this comment.
Дребна консистентност: таблицата за екранни четци форматира стойностите чрез formatCell(..., format ?? 'money'), но SVG tooltip-ът (<title>) по-долу показва суровата числова p.value. За еднакво възприятие между визуалния и достъпния изглед е добре и tooltip-ът да минава през formatCell.
| const reuseChipProps = ( | ||
| dedup: DedupData, | ||
| ): { title: string; leadStat: string | null; href: string } => { | ||
| const indexed = loadReportIndex().find((entry) => entry.id === dedup.reportId); |
There was a problem hiding this comment.
Незадължително (не блокиращо): loadReportIndex() чете localStorage синхронно и reuseChipProps се извиква директно в тялото на render (в .map) за всяко dedup съобщение, както и в effect-а за обявяване. При по-дълъг разговор с няколко cache-hit хода това означава многократно синхронно четене/парсване на localStorage при всеки render. Обмислете мемоизиране на индекса (напр. useMemo върху loadReportIndex()) или прочитане веднъж на ниво компонент. Функционално е коректно.
| // The user-facing failure line the dock shows for a rejected report — also the mask for a thrown one. | ||
| // A TECHNICAL failure, not a data statement — hence REPORT_FAILED_MESSAGE, never the | ||
| // insufficient-data wording (the data may exist; only the compose step failed). | ||
| const REPORT_FAILED_TEXT = REPORT_FAILED_MESSAGE; |
There was a problem hiding this comment.
Дребна забележка (код-стил / без дублиране): const REPORT_FAILED_TEXT = REPORT_FAILED_MESSAGE; е излишен псевдоним — навсякъде по-долу може директно да се използва REPORT_FAILED_MESSAGE. Индиректността не добавя стойност и създава два имена за една и съща стойност. Предложение: премахнете алиаса и ползвайте REPORT_FAILED_MESSAGE в redactEmitReportOutput.
| * "unverifiable", never as a pass. | ||
| */ | ||
| export async function assertReadOnlyPlan( | ||
| db: { prepare(sql: string): { all<T = unknown>(): Promise<{ results?: T[] }> } }, |
There was a problem hiding this comment.
Целият L3 гард разчита D1 да върне EXPLAIN редове с попълнено поле opcode през .all(). Ако D1 върне 0 реда или ключ, различен от opcode, guardOpcodes ще върне отказ за ВСЯКА заявка (fail-closed → тотален false-deny, макар и без риск от пропусната запис). Това е добре признато в коментарите и в ръчния VERIFY_D1_OPCODES тест, но тъй като този тест е skip-нат по подразбиране и CI никога не докосва реален D1, силно препоръчвам да се изпълни ръчно срещу реален D1 (VERIFY_D1_OPCODES=1 ... --remote) преди мърдж, за да се потвърди, че read-опкод вселената на D1 ⊆ READ_ONLY_OPCODES и че редовете реално се връщат през .all().
| * truncation (`отрязани от базата`, from RESULT_BYTE_CAP) and preview truncation (`показани първите N`). | ||
| */ | ||
| export function forModel(r: QueryResult, previewCap = MODEL_PREVIEW_BYTE_CAP): string { | ||
| const preview = capRows(r.rows, previewCap); |
There was a problem hiding this comment.
Гранична бележка (ниска важност): ако capRows върне 0 реда, когато дори първият ред надхвърля previewCap (напр. много широк ред с дълъг Cyrillic blob), preview.rows ще е [] и моделът ще получи [] като извадка, а заглавието ще покаже „показани първите 0". Имената на колоните все още са в заглавието, така че моделът може да състави справката, но си струва да се потвърди, че capRows гарантира поне 1 ред, за да не остане моделът без реален пример за данните. Ако вече е гарантирано — игнорирай.
|
|
||
| // Control chars (C0/C1) + bidi overrides/isolates that can spoof a transcript's visible order, plus | ||
| // zero-width chars (U+200B-200D) and the BOM (U+FEFF) that are invisible in the textarea/chat. | ||
| const UNSAFE_CHARS = /[\u0000-\u001F\u007F-\u009F\u200B-\u200F\u202A-\u202E\u2066-\u2069\uFEFF]/g; |
There was a problem hiding this comment.
Моля, потвърдете, че този регулярен израз се компилира и покрива описаните диапазони. Началната част [\^@-\^_�-\^_...] изглежда подозрителна: \^ е литерал ^ (0x5E), а �-\^ би бил обърнат диапазон (0x7F→0x5E), който хвърля SyntaxError: Range out of order при зареждане на модула — което би счупило целия /assistant/transcribe route. Ако намерението е било да се премахнат C0/C1 контролните символи, очакваният израз е нещо като \x00-\x1F\x7F-\x9F. Тъй като transcribe.test.ts е празен, липсва тест, който да улови това. Моля, добавете unit тест, който проверява, че sanitizeTranscript премахва контролни/bidi/zero-width символи (напр. �, , , ).
|
|
||
| function categoryMapping(input: string, normalized: string, key: string): SectorMapping { | ||
| const category = CPV_CATEGORIES.find((c) => c.key === key)!; | ||
| const divisions = [...category.divisions]; |
There was a problem hiding this comment.
Non-null assertion-ът (CPV_CATEGORIES.find((c) => c.key === key)!) прескача проверката: ако запис в CATEGORY_SYNONYMS сочи към ключ, който вече не съществува в CPV_CATEGORIES (преименувана/премахната категория), find връща undefined, ! го замълчава и следващият достъп до category.divisions хвърля TypeError при реален потребителски вход (напр. mapSectorWord('инфраструктура') → 500). Тестът hardcoded category synonyms still resolve... улавя дрейфа в CI, но по-устойчиво е явна проверка с graceful връщане към unknownMapping(input, normalized) вместо хвърляне. Препоръка: const category = CPV_CATEGORIES.find((c) => c.key === key); if (!category) return unknownMapping(input, normalized);
Pulls the euro-annex conversion fix (midt-bg#245/midt-bg#261), canonical value base (midt-bg#259), identity canonicalization + Bulstat checksum + joint procurement (midt-bg#251-253), app-layer read-only D1 guard (midt-bg#225), JSON-LD escaping (midt-bg#212), react-router 7.18.0. Non-trivial resolutions: - normalize-raw.sql: kept upstream's amendment_winner currency CTE + our is_synthetic column (both additive, one column-list collision). - integrity-checks.mjs: upstream's (await rows()) wrapper carrying our is_synthetic != 1 filter on auth/bidder attribution. - Migration collision: our 0002_contracts_is_synthetic renumbered to 0006 (upstream took 0002 for current_value_currency); tests load all migrations. - refresh-slice.test seedReattrContract: upstream's authority params + our real-tender-header seed so reattr contracts stay non-synthetic and reconcile. - root.tsx/assistant.chat.tsx: adopted getDb read-only chokepoint + kept dock. - describe-schema DATA_TRAPS: upstream canonical-base rule + our NULL detail.
|
Този клон е в конфликт с |
9 participants
feat(ai-assistant): conversational analytic layer (BgGPT)
A Bulgarian-language conversational analytics layer over the СИГМА procurement dataset. It answers
questions about authorities, companies, contracts and money flows, and returns bound, verifiable
reports — every number traces to a server-executed query. Integrity-first: the model orchestrates and
narrates, but never authors figures.
Feature surface
Chat —
POST /assistant/chatMAX_STEPS), streamed over SSE with a phased progress line.run_sql(read-only D1),semantic_search(RAG),find_entity(FTS entity resolution),eop_fetch(live ЦАИС ЕОП open-data),describe_schema,source_link,reconcile_rollup,emit_report(bound report), andanswer_directly(non-data turns — greetings/out-of-scope, no junk query).signed_atbounds atrequest time (no stale model-prior dates); freshness caveat for still-open periods.
amount_eur IS NOT NULL,procedure_type != 'неизвестна') and rollupreconciliation before any total is stated.
SQL integrity — read-only, three layers
rejects
SELECT *over personal-data tables (public idseik/bulstatstay queryable).EXPLAINon the live D1 and allowlist read opcodes only — a physical backstop thatcatches any write a parser miss might let through.
Reports — bound & verifiable
resultId/row/col),never to model-emitted text.
XSS-safe markdown rendering.
/reports/:id(48-bit random id,noindex,Cache-Control: no-store),with a report-export path.
docs/spec/ai-assistant-dedup.md):DEDUP_KV+ReportSingleFlightDO, layers L0–L2.5, gated on stable (settled-period) bounds only so a recent,still-filling period is never frozen.
Voice —
POST /assistant/transcribefallback (ADR-0013 — provider endpoints, not dynamic routes).
composer (never auto-sent), so audio can't inject straight into a query.
RAG grounding
sigma-assistant,@cf/baai/bge-m3, 1024-dim)via the token-gated seed
POST /assistant/reindex.semantic_searchretrieves schema/rule chunks; falls back to the full dictionary when RAG is empty.Assistant dock (UI)
/assistant/prompts+ ETLsuggested-prompts).Suspense+ an error boundary, so a dock render-throw can't take down page chrome.Transcript integrity — §9.3 (ADR-0011, ADR-0012)
conversationId/turnIndex/position;verify-then-strip (filter-on-ingest) on the next turn.
ENVIRONMENTbinding (notimport.meta.env.PROD);fail-open in preview/dev. Key rotation via
ASSISTANT_HMAC_KEY_PREVIOUS.Abuse & safety
/assistant/chatand/transcribe.BgGptCircuitBreakerDOcapping paid BgGPT turns (
BGGPT_RATE_LIMIT_RPM).prompt; no-fabrication and no-internal-fields rules.
ASSISTANT_ENABLED— returns a controlled503when off or unprovisioned.Design
docs/spec/ai-assistant.md(§1–9),ai-assistant-agent-team.md(bounded role graph +prompt-injection model),
ai-assistant-dedup.md,assistant-contracts.md,assistant-starter-prompts.md.0007(dedup, settled periods),0010(dedup on stable bounds),0011/0012(transcript HMACsigning + enforcement),
0013(voice via AI Gateway).🚀 Go-live — infra & env provisioning (required before enabling)
Full runbook:
docs/deploy-assistant.md. Nothing is baked into source; an operator provisions perenvironment. Summary:
Cloudflare resources
sigmaDBsigma-reportsREPORTSSIGMA_DEDUP_KV_ID)DEDUP_KVsigma-assistant(1024-dim, cosine)VECTORIZEAIsigma-assistant+ custom providers (custom-bggptchat,custom-bggpt-voicevoice)AI_GATEWAY_*Durable Objects (
ReportSingleFlight,BgGptCircuitBreaker) provision on deploy via migrations; ratelimiters are config-only.
Secrets (
wrangler secret put …, never committed):ASSISTANT_API_KEY,ASSISTANT_HMAC_KEY,TURNSTILE_SECRET,LOG_IP_KEY; optionalVOICE_ASSISTANT_API_KEY(falls back toASSISTANT_API_KEY),ASSISTANT_HMAC_KEY_PREVIOUS(rotation),ASSISTANT_SEED_TOKEN(gates/assistant/reindex).Vars:
AI_GATEWAY_BASE_URL(mandatory; empty ⇒ 503, fail-closed),AI_GATEWAY_ID,BGGPT_STT_BASE_URL,ASSISTANT_MODEL,TURNSTILE_SITE_KEY,ENVIRONMENT(drives HMAC fail-closed;not
import.meta.env.PROD),BGGPT_RATE_LIMIT_RPM, and guardrailsMAX_STEPS/RUN_SQL_TIMEOUT_MS/
D1_ROWS_READ_BUDGET.ASSISTANT_ENABLEDstays"false"until go-live.Sequence: create resources → put secrets → set vars → deploy (DO migrations apply) → apply D1
migrations + seed →
POST /assistant/reindex(BearerASSISTANT_SEED_TOKEN) to seed Vectorize →flip
ASSISTANT_ENABLED=true.Infra follow-ups (tracked, not blockers): read-only D1 credential (
DB_RO, #134) as the physicalbackstop behind the L3 SQL opcode guard; R2 report retention/erasure.