feat(assistant): наблюдаем RAG fallback — статистика на retrieval-а и логнати деградации

[nedda76]

Част от #318 — наблюдаемостта, върху която стъпва предстоящото премерване на флора. Самата рекалибрация на MIN_SCHEMA_SCORE/topK остава за втори PR, след като логовете съберат реални данни (issue-то остава отворено).

Какво

• retrieveSchemaContext подава RetrievalStats през опционален onStats hook — три брояча, за да са различими двата класа повреди с противоположни поправки:
  • matched — сурови namespace-скоупнати съвпадения (0 = празен/неиндексиран namespace или embed без вектор);
  • aboveFloor — над релевантния флор (matched > aboveFloor = флорът реже);
  • kept — реално стигнали до prompt-а (aboveFloor > kept = счупен metadata.text контракт, бъг в индексирането, НЕ флор).
• Route-ът логва статистиката като структуриран JSON (дисциплината на workers/request-log.ts — агрегируеми полета, само броячи, никога текста на въпроса) и вече не поглъща тихо грешката при retrieval (console.error само с message — суров error обект може да ехне въпроса).
• Позиционните topK/minScore станаха opts обект (RetrieveOptions).
• kept=0 при matched>0 е точно тихият fallback към пълния речник, който досега беше неразличим от работещ RAG.

Гаранции (от ревюто)

• onStats се вика в try/catch — хвърлящ metrics sink не може да струва на хода неговите чънкове (инвариантът на request-log.ts); закован с тест.
• И третият деградационен път (!vec — embed без използваем вектор) докладва, с нули — иначе е неразличим от „статистиката не е вързана"; тест.
• Тестът за броячите деривира скоровете от MIN_SCHEMA_SCORE ± ε (рекалибрацията няма да го прекатури) и затвърждава и върнатите чънкове, не само числата.

Стак

Стъпва върху #320 (→ #319 → #223). За ревю са последните 2 комита (feat + review fixes). Ред на мърдж: #223 → #319 → #320 → този PR.

Проверено

tsc -b чист, пълният пакет на apps/web: 510/510 теста, покритие над baseline (91.24 lines / 83.19 branches), Prettier чист.

[ydimitrof]

Обобщение на ревюто (feat(assistant): наблюдаем RAG fallback)

Много добре изпипано и изчерпателно тествано PR. Промяната добавя наблюдаемост на retrieval-а (RetrievalStats/onStats, логнати деградации в route-а — #318), маха последния as unknown as каст чрез боундъри-адаптер (bindings.ts — #316), сменя метаданни-filter с нативни версионирани namespace-и (schema-v2/entity-v1 — #317) и въвежда безусловно инжектиране на hard-trap-овете плюс relevance floor за RAG. Придружено е с целенасочени тестове за всеки нов клон.

Фаза 0 — security scan: ЧИСТО

• Няма закодирани тайни — BGGPT_API_KEY минава през wrangler secret put, изрично никога не се комитва.
• Няма нови/променени външни URL-и извън одобрените.
• Няма backdoor/обфускация/инжекции. Точно обратното — денилистът в sql-guard.ts се разширява (group_concat/string_agg/json_group_array/json_group_object), а report-schema.ts затваря дупка за неограничени числа в публичен отчет.
• Промените в зависимостите са само в osv-scanner.toml (игнориране на sharp libvips CVE-та) — обосновано като транзитивна, само-dev зависимост на miniflare, извън деплойнатия Worker, с дата за преразглеждане.

Силни страни

• Сигурност/устойчивост: логовете носят само броячи и error.message, никога текста на въпроса или payload-а (последователно в bindings.ts, rag.ts, assistant.chat.tsx) — правилна защита срещу изтичане на потребителски вход в логовете.
• Коректност: align е whitelist-нат, spread-ът на колони е заменен с експлицитно копиране (не пропуска непознати model-подадени полета към рендера), array-length таваните късат сканирането при препълване, score ?? 0 защитава срещу TypeError надолу по веригата.
• Без дублиране: renderTraps() е единствен източник за trap-овете; тестът „exactly-once" покрива регресията с двойно рендиране през реалния write→read seam.
• Тестове: смислени, а не тривиални — derived-from-floor фикстури, за да не се чупят при рекалибрация на MIN_SCHEMA_SCORE; проверки на call-count, за да не се промъкне filter-базиран fallback.

Съображения (не блокиращи)

1. MIN_SCHEMA_SCORE = 0.35 е некалибриран за v2 корпуса (изрично отбелязано, #318). Рискът е контролиран: под флора → [] → безопасен fallback към пълния речник, а fallback-процентът вече е наблюдаем. Препоръка: рекалибрирайте с реални въпроси, преди да разчитате на RAG grounding в прод.
2. Почистване на стар кохорт изисква ръчно възстановяване на списъка с id-та от git историята на buildSchemaChunks — операционно чупливо, но документирано и незадължително (namespace-ът изолира старите кохорти).
3. Денилистът за SQL функции по същество е догонваща игра — авторите сами отбелязват, че позитивен allowlist е трайното решение (проследено отделно).

Препоръка: COMMENT

Кодът е готов за мърдж и не чупи main. Оставям COMMENT (вместо APPROVE) единствено заради висящата рекалибрация на relevance floor-а (#318) — тя е предпоставка да се вярва на RAG grounding в новия режим, макар че текущото поведение при провал е безопасно.

[ydimitrof]

Обобщение на ревюто — feat(assistant): наблюдаем RAG fallback

Обща оценка: 9.3/10 — APPROVE. PR-ът е с висока чистота, добре тестван и защитен на всяка граница. Промените са атомарни и в един ясен концерн (наблюдаемост на RAG + версионирани namespace-и + няколко review follow-up хардънинга).

Сигурност (Фаза 0 — автоматично сканиране)

• Няма hardcoded тайни. wrangler secret put BGGPT_API_KEY е интерактивен; README изрично казва „никога не се комитва".
• Няма нови/променени външни URL-и. Единственият модел-литерал е @cf/baai/bge-m3 (Workers AI capability), не мрежов endpoint.
• Няма зловредни шаблони / обфускация / backdoors.
• Зависимости: osv-scanner.toml добавя ignore само за транзитивна dev-only уязвимост (sharp<0.35.0 през miniflare), с обосновка и ignoreUntil дата. Приемливо — не е в деплойнатия Worker.
• Резултат: CLEAN.

Силни страни

1. emit-report-schema.ts — тавани на масивите (MAX_BLOCKS/ITEMS/COLUMNS) със short-circuit преди per-element сканирането; тестовете доказват, че се връща точно кап-грешката, а не 101 паразитни грешки. Затваря реален DoS/amplification вектор.
2. isAlign whitelist + експлицитно изграждане на колоните в bindReport (без {...c} spread) — спира пренасяне на непроверени model-подадени полета към рендера. Отлична defense-in-depth срещу атрибутна инжекция.
3. sql-guard.ts — блокиране на string-building агрегати (group_concat/string_agg/json_group_*) — правилно затваря същия memory-amplification клас като printf едно ниво нагоре, вкл. string_agg alias за SQLite ≥3.44 (D1).
4. system-prompt.ts — hardTraps() безусловно — коригира реалната регресия „RAG turn с по-малко ограничения от no-RAG fallback". Композиционният тест през реалния write→read seam (indexSchemaCorpus → retrieve → buildSystemPrompt) проверява „точно веднъж" за всеки trap — силен тест.
5. bindings.ts — премахване на as unknown as (issue #316) с реален compiler-checked bridge и keys-only диагностика, която не логва потребителски текст.
6. Наблюдаемост (#318): RetrievalStats с три отделни брояча (matched/aboveFloor/kept) различава двата класа деградация; best-effort try/catch около sink-а; логовете носят само броячи, никога текста на въпроса.

Незадължителни watch-items (не блокират мърдж)

• MIN_SCHEMA_SCORE и MIN_ENTITY_SCORE = 0.35 (rag.ts). Кодът сам отбелязва „RECALIBRATION PENDING (#318)": прагът е калибриран срещу pre-v2 корпуса (12 къси trap chunk-а), а v2 е 25 по-дълги query/table chunk-а, които скорират различно под bge-m3. Рискът е тих fallback към пълния речник за много въпроси — но е безопасният изход и вече е наблюдаем чрез assistant.rag лога. Препоръка: измерете реалните разпределения на скора преди да фиксирате прага; обмислете отделни стойности за схема vs entity.
• entity-v1 namespace е празен by design (няма entity indexer). Инструментът връща 0 попадения — коректно документирано в README/rag.ts, включително предупреждението, че „WHEN TO BUMP" правилото НЕ се пренася към data-derived entity корпуса. Добра проактивна бележка за бъдещия indexer.
• Denylist подходът в sql-guard.ts е inherently catch-up игра срещу нови aliases — PR-ът сам признава, че positive allowlist е трайният фикс (tracked separately). ОК за сега.

CLAUDE.md съответствие

Няма partial implementation, TODO-simplification, дублиране (renderTraps() дедуплицира trap рендера между describeSchema и hardTraps), мъртъв код или resource leaks. Тестовете са смислени (не cheater — проверяват граници, floor drop-ове, метадата bug сигнали), не тривиални. Именуването е консистентно.

Препоръка: APPROVE. Watch-items-ите са наблюдателни follow-up-и, не дефекти.

[ydimitrof]

Ревю на PR: наблюдаем RAG fallback — статистика на retrieval-а и логнати деградации

Фаза 0 — Сигурност (блокираща): ЧИСТО ✅

• Няма твърдо кодирани тайни. BGGPT_API_KEY минава през wrangler secret put и изрично се коментира „никога не се комитва". Няма API ключове/пароли/токени в дифа.
• Няма нови външни URL-и. EMBED_MODEL = '@cf/baai/bge-m3' е Workers AI идентификатор на модел, не мрежов адрес.
• Няма зловредни шаблони (backdoor/инжекция/обфускация).
• Зависимости: единствената промяна е добавяне на два IgnoredVulns записа в osv-scanner.toml за sharp<0.35.0 (libvips CVE-та). Обосновката е коректна — транзитивна, само-dev зависимост през miniflare, не влиза в деплойнатия Worker, няма in-range upstream fix; има ignoreUntil дата за ре-триаж. Приемливо.

Общо качество: много силен PR

Промяната е атомарна и добре обоснована — цялата се върти около една тема (наблюдаемост на RAG fallback + втвърдяване на границите). Забележителни силни страни:

• Сигурност на данните към логовете: навсякъде се логват само броячи/съобщения, никога текстът на въпроса или суровият error обект (bindings.ts, assistant.chat.tsx, rag.ts). Това е последователно спазено и изрично документирано.
• bindings.ts (issue #316): премахването на as unknown as в полза на типизиран адаптер е реално подобрение — грешка на границата вече се хваща от tsc, а не в продукция. Адаптерът разграничава „липсващ ключ" от „празен data масив за непразен вход", което пази долната embed() count-проверка от подвеждащо съобщение.
• rag.ts: преходът от metadata filter към нативен namespace е правилен (metadata филтър изисква provisioned metadata index, който репото няма — #317). Версионирането на namespace + id (schema-v2) и правилото „WHEN TO BUMP" коректно пазят rollback прозореца. Релевантностният праг (MIN_SCHEMA_SCORE) решава реален проблем — иначе top-K връща K-те най-близки чънка дори при изцяло off-topic заявка и прави grounding-а по-слаб от no-RAG fallback-а.
• system-prompt.ts: hardTraps() инжектира императивните капани безусловно — това затваря реалната дупка, при която RAG turn можеше да остане с по-малко ограничения от fallback-а. Тестът renders every hard trap exactly once минава през реалния write→read seam и пази срещу двойно рендиране.
• sql-guard.ts: разширяването с string-building агрегати (group_concat/string_agg/json_group_*) и покриването на quoted-identifier bypass ("group_concat"(x), [group_concat](x)) е добре обмислено; авторът честно отбелязва, че denylist е catch-up игра и позитивен allowlist е трайният fix.
• Покритие с тестове: отлично. Всеки нов клон има целеви тест (праг, scoreless match, onStats broene, throwing sink, cap short-circuit, quoted-name bypass, spelled magnitudes до секстилион). Тестовете са проектирани да ловят регресии, не да минават тривиално.

CLAUDE.md съответствие

Няма частична имплементация, дублиране (renderTraps() дедупlicира rendering-а на капаните), мъртъв код или смесени концерни. Ресурсите се почистват (best-effort try/catch около onStats).

Дребни, неблокиращи бележки

1. semanticSearch не получи onStats наблюдаемостта, която retrieveSchemaContext придоби. Днес е безвредно (entity корпусът е празен по дизайн), но когато entity indexer-ът от Фаза 2 се появи, ще е полезна същата симетрия (виж инлайн).
2. MIN_SCHEMA_SCORE е изрично отбелязан за рекалибрация (#318) спрямо v2 корпуса — това е следеният, очакван компромис, а не скрит TODO; просто да не се забрави преди да се разчита на прага в новия режим.

Вердикт

Няма блокиращи или critical находки. Сигурност: чисто. Тестове, наблюдаемост и документация са налице и последователни. Препоръка: APPROVE.

[lyubomir-bozhinov]

Малка непоследователност в leak-safety патерна, който този PR въвежда: assistant.chat.tsx:128-132 редактира до .message (защото Workers AI/Vectorize грешка може да ехне embedded-ната заявка — обосновката на :129-131), но третият catch в същия файл, :145-148, още логва суровия error обект:

} catch (error) {
  console.error('[assistant] turn failed to start', error);

Заварено (непроменено спрямо main), но след като серията установява и тества „само .message" два пъти точно в този файл за точно този клас изтичане, третото място си струва да е огледално:

const message = error instanceof Error ? error.message : String(error);
console.error('[assistant] turn failed to start:', message);

Иначе #319→#320→#321 е издържана серия: namespace-ът е приложен и на insert, и на query (rag.ts:125,159 — без cross-namespace leak), DATA_TRAPS grounding-ът остава непокътнат на fallback пътя (hardTraps() безусловно), stats-овете са само броячи (без PII), а quoted-identifier bypass fix-ът в sql-guard.ts вече покрива и "…"/[…]/backtick. Merge ред: 319 → 320 → 321 (стекнати).

[nedda76]

Прав си — и не само третият catch. Проверих всички лог места в модула и същият клас беше още на две: tools.ts run_sql (D1 грешката носи SQL-а, който моделът е построил от въпроса) и agent.ts stream onError (BgGPT грешката носи prompt-а).

Затова вместо трети copy-paste на тернара — errorText() в новия log-safety.ts, използван на ВСИЧКИТЕ пет catch/лог места: само message, никога stack, никога cause веригата, никога обекта, плюс таван от 300 знака срещу гигантско тяло от провайдър. Тестът заковава точно тези инварианти (негативен контрол: връщане на stack/cause го чупи). c7ec503.

Благодаря и за прегледа на серията — merge редът е потвърден: 319 → 320 → 321.

[nedda76]

Допълнение към горното — само-ревюто на първия ми комит намери, че той пазеше грешната ос, затова го поправих в cecdf2e:

1. Помощникът не беше тотален. String(Object.create(null)) хвърля (проверено), Proxy или хостилен message getter — също. Функция, която живее в пет catch блока, не може да хвърля: изключението щеше да избяга от catch-а, да убие graceful fallback-а и да върне 500 — при това с целия обект в лога на framework-а, т.е. точно обратното на целта. Вече е тотален (фиксиран таг при неконвертируема стойност).
2. Махането на stack-а не адресира ехото. Stack frames по конструкция не носят потребителски текст — ехото от провайдъра е в message. Затова сайтовете, които знаят входа си, вече го подават за редакция: въпросът (route-ът и stream onError през ctx.userQuestion) и заявката (semantic_search). Това е частта, която реално затваря „грешката ехна въпроса"; тапата и без-stack-а само ограничават щетата. Където входът не е известен на catch-а (D1 грешка с model-построен SQL) message-ът се логва съзнателно — иначе губим единствената корелация към проблемната заявка; записано е изрично в log-safety.ts.
3. Тестът твърдеше нещо невярно („обект се свежда до непрозрачен таг") — вярно е само за обект с наследен toString. Сега тестът документира реалното поведение.

Плюс: collapse на нови редове (многоредово съобщение чупеше prefix-базиран grep — продълженията нямат [assistant] префикс), рязане по кодови точки, и връщане на stack frames при setup грешката в route-а, където дефектът е конфигурационен и рамката е диагностиката.

Негативни контроли: махането на try/catch чупи теста за тоталност, махането на редакцията — теста за ехото. 524 теста зелени, покритието над baseline.

Остава като съзнателен пропуск: корелация на run_sql провалите (fingerprint на заявката) — по-скоро observability, отколкото leak-safety; ако смяташ, че си струва, отварям отделен issue.

[lyubomir-bozhinov]

Патчовете c7ec503a/cecdf2e2 затварят находката от предишния ревю — и трите catch-а вече минават през errorText, stackHead дава само рамки. Проверих при HEAD cecdf2e2. ✅

Един остатъчен ръб в errorText (log-safety.ts), който тестовете не покриват:

let out = raw.replace(/\s+/g, ' ').trim();
for (const needle of redact) {
  if (needle && needle.length >= MIN_REDACT_CHARS) out = out.split(needle).join(REDACTED);
}

out е whitespace-collapse-нат, needle-ът (question) — не. latestUserText() прави само join(' ').trim(), без да свива вътрешния whitespace, затова многоредов въпрос (shift-enter в composer-а) или таб/двоен интервал остават в needle-а. Ако provider-ът ехне такъв въпрос в съобщението на грешка, collapse-натият out вече не съвпада с needle-а → split не намира нищо → въпросът влиза дословно (само с интервали вместо \n) в лога. Точно гаранцията „no verbatim copy of that input", която модулът декларира.

Repro:

const question = 'колко плати\nобщина Пловдив на фирма Х';
errorText(new Error(`400 invalid input: ${question}`), [question]);
// → "400 invalid input: колко плати община Пловдив на фирма Х"  (нередактирано)

Фикс — свий needle-а по същия начин преди split:

const n = needle.replace(/\s+/g, ' ').trim();
if (n.length >= MIN_REDACT_CHARS) out = out.split(n).join(REDACTED);

Minor (само server логове, иска ехо от provider-а), но пада точно в основния control за ехнат вход — и е един ред тест: needle с \n.

[nedda76]

Приложено в f8af0b6 — needle-ът за редакция минава през същото collapseWhitespace като съобщението, прагът MIN_REDACT_CHARS се преценява по свитата форма (whitespace не може да издуе къс needle над него), а твоето repro (\n във въпроса) е тест с негативен контрол: пада срещу старата имплементация.

Покрай това минах клона още веднъж отгоре до долу — гаранцията „въпросът не влиза в лога" беше пробита на още три места, всички затворени:

• streamText има собствен default onError = console.error(error) — суровият APICallError, чиито собствени полета носят requestBodyValues (system prompt + съобщенията) и responseBody. Hook-ът беше подаден само на toUIMessageStreamResponse, така че SDK-то продължаваше да пише суровия обект при всяка провайдърска грешка. Сега логът е една редактирана линия с клас/HTTP статус (401 vs 429 vs 5xx остават различими, само идентификатори), дедупната по идентичност на грешката (13ab613).
• Catch-ът на RAG retrieval в route-а викаше errorText(error) без [question] — точно мястото, което embed-ва въпроса и е най-вероятното ехо (13ab613).
• stackHead приемаше едноредово съобщение, а V8 печата ЦЯЛОТО многоредово съобщение преди първата рамка — продълженията излизаха нередактирани точно до редактирания errorText на същия ред. Вече се котви на първия at ред и без разпознаваема рамка връща '' (fail closed). Редакцията междувременно лови и JSON-escaped (\", literal \n) и отрязано ехо през прозоречно съвпадение, с bounded pre-cap срещу многомегабайтово тяло (4a9f5de).
• Бонус в същата класа: bindings.ts правеше 'data' in out без обектен guard — при не-обект V8 слага операнда в съобщението на TypeError-а, т.е. тяло от gateway влизаше в лога през „keys-only" пътя; вече се именува само типът (13ab613).

Всичко това беше минало под радара, защото нито един тест не влизаше през вратата на потребителя — добавени са route-door тестове (POST през action() с фалшиви AI/VECTORIZE: stats линията е само броячи, RAG-провалът с ехо е редактиран, setup-провалът дава 503 с рамки-only линия) и stream-error тест през реалния streamText с MockLanguageModelV3. И двата са негативни контроли за горните находки (13ab613).

Останалото от само-ревюто (b54a86b…56a24a2): „павилион(и)" вече не флагва като число (суфиксите са котвени към числителните представки — \p{L} lookaround не различава „пав-илион" от „секст-илион"); голо млн./млрд. в мерно заглавие („Стойност (млн. €)" — стилът на собствените ни колони) също не, флагва само след изписано числително; jsonb_group_* влезе в денилиста (SQLite ≥3.45 в build-а на workerd, минаваше и двата слоя); sub на facts се проверява по тип (число минаваше shape-а и хвърляше TypeError в bindReport — непрозрачна tool грешка вместо retryable съобщение); align/link: null се приемат като „не е зададено"; link се пресъздава от двете си известни полета (href отвътре не стига до замразения отчет); async stats sink вече не тече като unhandled rejection; returnMetadata: 'all' е закован в тестовете (махането му оставаше зелено срещу фалшивия индекс, а в прод дава text-less съвпадения → kept=0 → тих fallback на всеки ход); записът за amendments в речника носи value_suspect/валутния/join капаните от новите миграции (#305/#307); и мъртвото osv изключение за sharp е махнато — main вече override-ва до 0.35.3 от #226, записът идваше от стар комит, пребазиран отгоре.

549 теста зелени, tsc чист, Prettier чист. Отворени като follow-up, не в този PR: изпълним вход за indexSchemaCorpus (нищо в репото не може да напълни schema-v2 — всяка среда е на full-dictionary fallback до тогава) и дрифт-тест, който заковава колоните от TABLES срещу реалните миграции през pragma_table_info.