feat(ai-assistant): conversational analytic layer (BgGPT)#79
Open
lyubomir-bozhinov wants to merge 83 commits into
Open
feat(ai-assistant): conversational analytic layer (BgGPT)#79lyubomir-bozhinov wants to merge 83 commits into
lyubomir-bozhinov wants to merge 83 commits into
Conversation
4 tasks
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 midt-bg#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 midt-bg#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.
fix(web): report page bug fixes — mirror #36 to feat
…nistic gates (#38) Adds a risk-scaled LLM verifier as role ④ behind the deterministic gates: every report (model path + deterministic fallback) passes through verify() before persist/stream, so nothing reaches the user unverified. Strip-only output channel.
feat(assistant): risk-scaled LLM verifier (role ④) — mirror #38 to feat
…e hardening (#37) Mirror #37 into the upstream line for dual-PR parity. Ports the feature delta only — dock markdown renderer + link allowlist, report dedup (Lane F: single-flight, freshness-folded KV, settled-period-only gate), rpm-window + global BgGPT circuit-breaker, per-IP rate-limit refactor, and the report-schema chart-col + href adversarial test coverage. The deploy layer stays on -contracts: deploy.yml/preview.yml, the ephemeral-env scripts (ensure-kv-namespace, wrangler-render), and the CI script-tests step are excluded. wrangler.jsonc carries the new KV/DO bindings + vars but keeps the gateway URL / account id neutralized (empty AI_GATEWAY_BASE_URL/ID, <account-id> placeholder) per the upstream-line boundary.
feat(assistant): dock markdown, report dedup (Lane F), launch-gate hardening (#37 mirror)
Hop 2 of the upstream sync wave. Brings upstream 9e6edbf..832a8db onto the upstream-tracking feature line. Conflict resolutions: - rate-limit.ts: took upstream's normalizedPathname — both sides independently added the same `.data` strip (midt-bg#184 == our strict-review fix); functionally identical, upstream's is canonical. - assistant-rate-limit.test.ts: kept ours (asserts the exact limiter key, strictly stronger than upstream's midt-bg#184 duplicate; no coverage lost). - ci.yml: union — keep our node 22.23.0 pin (assistant opcode-guard) and Golden reports gate, add upstream's Docs integrity gate. - 0000_init.sql: took upstream (comment-only divergence, DDL identical; upstream adds value_low to the value_flag enum + an etl.md ref fix). - ADR: renumbered our 0001-report-dedup to 0007 under upstream's ADR index (BG, per _template), updated the spec's inbound link. - Indexed the assistant feature docs so upstream's new check-docs gate passes.
sync: feat/ai-assistant ← upstream/main (2026-07-06)
Mirror #47 and #48 onto the upstream feature line for dual-PR parity. Feature files only — the mid-stream gateway-429 classifier/shed message (stream-errors.ts), and the accessibility pass for the report blocks + dock (aria-live turn-settle announcement, bar-chart hidden-table parity, abort-flag lifecycle, focus management, 2.2-delta target sizes). Deploy layer stays on -contracts. Byte-identical to the merged -contracts versions (2d1e5db, fc834e8).
…ode + coverage (#57) Parity mirror of #52 onto the upstream line (dual-PR model). rag.ts DEVIATION→ADOPTED (ADR-0008); account-wide BgGPT cap documented as the BgGptCircuitBreaker DO, not AI Gateway (ADR-0009, incl. stream-errors.ts); ADRs 0008/0009 + spec corrections; condense/byte-pressure/data-dictionary tests (DATA_TRAPS pinned to 12).
… minter (#58) Mounts the invisible Turnstile widget (execute-per-send minter), attaches cf-turnstile-response, widens CSP to challenges.cloudflare.com, exposes TURNSTILE_SITE_KEY via the root loader. Pairs with the server gate to complete the bot-check. Includes useTurnstileGate hook tests. (cherry picked from commit 6a93716)
- Render a report chip on a dedup cache hit (data-dedup stream part) instead of a blank turn; enrich from the local report index, WCAG settle announcement. - Gate L1 report dedup on stableBounds (explicit absolute un-clamped bounds) not recencyCaveat; parse explicit ISO ranges (ADR-0010). - Strip leaked <report>/<tool_call>/<tool_response> pseudo-XML from dock prose. - Narrow the untrusted dedup layer against the known set; correct the periodSettling field doc.
The server-side fallback finalizer published a report from any non-empty run_sql result, gated only on rows.length > 0. On a greeting the weak model ran a stray SELECT division probe returning one CPV code (45); the fallback rendered it as an authoritative one-cell report titled Справка по наличните данни — meaningless output. Add a quality bar in buildFallbackReport: a single-row result with no numeric measure carries no figure to surface (the finalizer's whole charter), so refuse it. Multi-row lists and any result with a measure are unaffected. This reverses the prior behaviour of publishing a lone text row (e.g. a bare entity name) — such a row is no more an answer than the CPV code. agent.ts routes a refused fallback through the same empty-completion affordance as the no-data case, so a probe-only turn with no prose is never left blank.
…ip junk run_sql The step-0 forced tool call (chooseToolChoice → 'required') stops a weak model narrating run_sql as prose, but forces a greeting/meta turn to call *something* too. With only data tools available the model invents a junk probe (SELECT 1), whose lone numeric cell the fallback then publishes as a hollow "totals: 1" report — the residual left after the Division/45 fallback bar. Give such a turn a valid non-query choice: answer_directly, a no-arg no-op that signals "no DB needed" and returns a prose nudge. It touches no ctx.results, so the fallback finds nothing to synthesize and the model answers in plain prose on the next (auto) step. Step-0 forcing is unchanged; the anti-prose-narration guarantee stands. - tools.ts: answerDirectlyTool (no args, no result-state side effect) - system-prompt.ts: NON_DATA_TURN_RULE + ROLE bullet routing non-data turns - agent.ts: refresh the stale prepareStep comment - tests: registry + no-op behavior; prompt-routing assertion
#66) Feat-line mirror of #66 (merged on feat/ai-assistant-contracts). Feature-only: - excludes deploy layer (ci/deploy/preview workflows, scripts/ensure-voice-provider) - wrangler.jsonc carries the voice vars + TRANSCRIBE_RATE_LIMITER binding, but BGGPT_STT_BASE_URL is empty (opt-in per deploy) — NO CF account id on this line. Stacked on the #64 mirror; transcribe route uses the 3-arg turnstileRejection.
Feat-line mirror of #68 (merged on feat/ai-assistant-contracts). Feature-only: - excludes deploy layer (deploy/preview workflows, scripts/ensure-worker-secret, scripts/wrangler-render, docs/deploy.md). - wrangler.jsonc gains the ENVIRONMENT gate binding (feature); NO CF account id. Stacked on the voice mirror; gateTranscript gate coexists with the breaker + turnstile.
…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
reviewed
Jul 9, 2026
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.