test(assistant): дрифт-пазач — речникът на данните срещу реалните миграции - #330
test(assistant): дрифт-пазач — речникът на данните срещу реалните миграции#330nedda76 wants to merge 48 commits into
Conversation
|
Прегледах новия комит Проверено:
Стакнат на #321 (мърдж ред #321 → 330) и разчита на речника оттам, за да е зелен. Превръща следващия мълчалив дрифт от находка „на око" в червен build — по духа на #294. 👍 |
f4a18c3 to
224d7fa
Compare
ydimitrof
left a comment
There was a problem hiding this comment.
Обобщение на прегледа
Какво прави PR-ът: Добавя „дрифт-пазач" — тест, който сверява речника на данните (describe-schema.ts) срещу реалните миграции, за да не се разминават. Освен това идват няколко целенасочени продукционни промени и обширно тестово покритие: нов Vectorize namespace слой за RAG, log-safety защита, схема-валидация, разширяване на SQL guard денилиста, замразяване на отчетната схема и безусловно инжектиране на hardTraps() в RAG-клона.
Обща оценка: Зряла, добре обмислена промяна с изключително силно тестово покритие. Не са открити проблеми със сигурността, целостта на данните или коректността, които да блокират мърджа. Няма злонамерен код, нови мрежови/CI стъпки, зависимости, secret-и или опити за инжекция в diff-а.
Съществени положителни моменти:
- log-safety.ts — тотален (никога не хвърля в catch), сваля stack/cause, редактира ехото на въпроса и капва реда по code points.
requestBodyValues(системен prompt + съобщения) никога не попада в лога — точно инвариантът, койтоagent.stream-error.test.tsпази. - RAG слой — преминаването към нативен Vectorize namespace (
schema-v2/entity-v1) вместо metadatafilterе коректно предвид липсата на provisioned metadata index (#317); неиндексирана среда безопасно пада към пълния статичен речник.Number.isFiniteвместо?? 0за прага е правилен избор. - sql-guard.ts — разширяването на денилиста с низовите агрегати и опционалния клас за кавички затваря байпаса с цитирани идентификатори; денилистът е догонваща игра, но това е признато и позитивният allowlist е проследен отделно.
- report-schema.ts — изричното пресъздаване на колоните и възстановяването на
linkсамо отkind/idColпази замразения отчет от промъкване на непознати ключове. - system-prompt.ts — безусловното инжектиране на
hardTraps()в RAG-клона гарантира, че RAG-ход не е по-малко ограничен от fallback-а. - describe-schema-drift.test.ts — полезен drift guard с добри негативни контроли.
Блокиращи концерни: Няма.
Незадължителни бележки (не блокиращи):
- Дискусионна точка относно обхвата на редакцията при multi-turn разговор — вж. инлайн коментара в
agent.ts. - Две дребни, незадължителни бележки по drift теста.
- Проверка за консистентност: добавените колони в
describe-schema.ts(parties,amendments) и смяната наdata_freshnessот view на таблица трябва да съвпадат едно към едно с реалните миграции — потвърдено в drift теста от партида 2.
Заключение: COMMENT / Одобрено — чиста промяна с една дискусионна точка и няколко незадължителни бележки, без дефекти за поправка.
… + дрифт-пазачът да не пропуска цитирана колона - redact вече се строи от userTexts(opts.messages) — всички user ходове. Провайдърска грешка може да цитира която и да е част от prompt-а, а multi-turn prompt носи и по-ранните въпроси; редакция само на ctx.userQuestion ги оставяше нередактирани в tail лога. userTexts е експортнат и тестван (вкл. че игнорира не-user роли и празни текстове; негативен контрол: 'само първия' чупи теста). - Дрифт-пазачът сваля SQL кавички преди да чете идентификатора: цитирана колона на топ ниво („col", [col], ) досега тихо се пропускаше и не се сверяваше срещу реалната схема — guard, който фейлва ОТВОРЕНО. Днес речникът не цитира; така остава затворен, ако някога го направи. - format НЕ става условен: validateEmitShape изисква isFormat(c.format) за всяка колона, а EmitTableColumn го типизира задължителен, така че 'format: undefined' е недостижим — предпоставката на бележката не важи. Записано като коментар на самото място, за да не се пита пак. Бележки от ревюто на @ydimitrof по midt-bg#330.
224d7fa to
7152370
Compare
… + дрифт-пазачът да не пропуска цитирана колона - redact вече се строи от userTexts(opts.messages) — всички user ходове. Провайдърска грешка може да цитира която и да е част от prompt-а, а multi-turn prompt носи и по-ранните въпроси; редакция само на ctx.userQuestion ги оставяше нередактирани в tail лога. userTexts е експортнат и тестван (вкл. че игнорира не-user роли и празни текстове; негативен контрол: 'само първия' чупи теста). - Дрифт-пазачът сваля SQL кавички преди да чете идентификатора: цитирана колона на топ ниво („col", [col], ) досега тихо се пропускаше и не се сверяваше срещу реалната схема — guard, който фейлва ОТВОРЕНО. Днес речникът не цитира; така остава затворен, ако някога го направи. - format НЕ става условен: validateEmitShape изисква isFormat(c.format) за всяка колона, а EmitTableColumn го типизира задължителен, така че 'format: undefined' е недостижим — предпоставката на бележката не важи. Записано като коментар на самото място, за да не се пита пак. Бележки от ревюто на @ydimitrof по midt-bg#330.
7152370 to
7fcd982
Compare
|
Без промени по собствената делта на #330 (дрифт-пазачът + редакцията на всички потребителски ходове). Клонът е пребазиран върху обновения #321 (prose gate-ът е fail-closed за млн/млрд/трлн; „pre-cap" тестът вече минава през pre-cap-а) и обновения #223 (jsonb + comment-desync байпасът на функционалния денилист). Проверката срещу реалните миграции остава зелена. |
… + дрифт-пазачът да не пропуска цитирана колона - redact вече се строи от userTexts(opts.messages) — всички user ходове. Провайдърска грешка може да цитира която и да е част от prompt-а, а multi-turn prompt носи и по-ранните въпроси; редакция само на ctx.userQuestion ги оставяше нередактирани в tail лога. userTexts е експортнат и тестван (вкл. че игнорира не-user роли и празни текстове; негативен контрол: 'само първия' чупи теста). - Дрифт-пазачът сваля SQL кавички преди да чете идентификатора: цитирана колона на топ ниво („col", [col], ) досега тихо се пропускаше и не се сверяваше срещу реалната схема — guard, който фейлва ОТВОРЕНО. Днес речникът не цитира; така остава затворен, ако някога го направи. - format НЕ става условен: validateEmitShape изисква isFormat(c.format) за всяка колона, а EmitTableColumn го типизира задължителен, така че 'format: undefined' е недостижим — предпоставката на бележката не важи. Записано като коментар на самото място, за да не се пита пак. Бележки от ревюто на @ydimitrof по midt-bg#330.
7fcd982 to
5ada70b
Compare
|
Пребазирано върху текущия |
lyubomir-bozhinov
left a comment
There was a problem hiding this comment.
Прегледах собствената делта на #330 (2 комита над #321, при HEAD 5ada70bf) — log-safety + drift-guard.
redact-every-turn (5ada70b) — коректно: новият userTexts(messages) (agent.ts) вади текста на ВСЕКИ user ход (role==='user', само text части), и runAssistant подава redact = userTexts(opts.messages) вместо само последния въпрос. Реален leak fix: BgGPT error body може да отрази обратно всяка част от подадения промпт, а в multi-turn разговор по-ранните въпроси също са в промпта — редакция само на последния би оставила по-ранен въпрос в tail log-а. Правилно филтрира до user (assistant/tool не са PII needles); log-safety length floor-ът маха твърде късите фрагменти.
drift-guard тест (describe-schema-drift.test.ts) — реален, не cheater: прилага ВСИЧКИ packages/db/migrations/*.sql по ред в throwaway sqlite3, после проверява речника (TABLES/CANONICAL_QUERIES) срещу реалната схема през pragma_table_info/sqlite_master; assert-ва че всяка изброена колона/таблица съществува + →tenders ref-а. Бъдеща миграция, която преименува/дропне речникова колона, гърми тук — точно guard-ът за drift-а, който #223 поправи.
(report-schema.ts промяната е само пояснителен коментар, без логика.)
Merge-ред: връх на стека #223→#319→#320→#321→#330. Нямам блокери по собствената делта.
ydimitrof
left a comment
There was a problem hiding this comment.
Прегледах партида 2/2 — RAG слоя, SQL guard-овете, prose-number регулярните изрази, дрифт-теста и route/логовете.
Като цяло това е много добре обмислена, защитно написана промяна с богати обосновки и силно тестово покритие. Не открих сигурностни или data-integrity дефекти, които да блокират сливането:
- SQL guard-ове (sql-guard.ts / sql-ast-guard.ts). Преминаването към общ
quotedSpanEndза четирите форми на цитиране затваря реален bypass (comment-desync зад"x'y") и правилно моделира удвоените затварящи кавички; поведението при непрекъснат span е fail-closed. Denylist-ът на функции е дефиниран веднъж и се проверява и лексикално, и върху разрешеното от парсера име на всяка дълбочина, с fail-closed при неизвестна форма — солидна двуслойна защита.string_agg/json(b)_group_*покриват модерния SQLite на D1. - RAG (rag.ts). Нативните версионирани namespace-и (
schema-v2/entity-v1) вместо metadatafilterса коректни — старите вектори остават в default namespace и не могат да попаднат в topK. ПраговетеMIN_*_SCORE+ fallback към пълния речник,Number.isFiniteвместо?? 0, и best-effortreportStats(с defusing на promise) са издържани. - system-prompt.ts. Безусловното инжектиране на
hardTraps()затваря регресията, при която RAG-ход оставаше с по-малко ограничения от no-RAG fallback — добре покрито отsystem-prompt.test.ts. - report-schema.ts. Разширяването на magnitude-суфиксите (закотвени към числовите префикси, за да не хваща „павилион“) и клонът дума+млн/млрд/трлн са премислени; over-flag посоката е безопасната.
Оставям няколко необвързващи въпроса като inline коментари. Няма следи от prompt-injection или враждебен код в diff-а — коментарите в кода са легитимни технически обосновки.
The curated dictionary the model treats as hard fact had drifted from packages/db/migrations/0000_init.sql: - amendments: no contract_id column — it links via unp/contract_number - parties: no role column — real cols are party_key, eik, ocid, party_id, name… - value_flag enum was missing value_low - amount_eur IS NULL was described as meaning value_suspect; it actually has several causes (FX-rateless foreign / value_suspect w/o estimate / no signing+current), and the unconfirmed count is value_flag='value_suspect' (home_totals.suspect), not NULL-amount rows - data_freshness is a table, not a view Drift here misleads a weak model into wrong joins or a wrong integrity KPI.
…e retrieval Two grounding gaps that could leave a RAG turn LESS constrained than the no-RAG fallback: - buildSystemPrompt used the retrieved chunks INSTEAD of the dictionary, so a retrieval that missed the money-sum trap dropped the SUM(amount_eur) rule entirely. Inject the short imperative DATA_TRAPS unconditionally; RAG now only selects the extra tables/example-queries for the question. - retrieveSchemaContext had no relevance floor — top-K returned its K least-distant chunks even when all were off-topic. Add MIN_SCHEMA_SCORE; below it we return fewer/zero chunks, and zero falls back to the full dictionary (the safe outcome).
group_concat / json_group_array / json_group_object collapse an entire full-table scan into one huge cell that materialises in Worker memory before capRows can measure it (and capRows keeps the first row whole) — the same memory-amplification class already blocked for printf/format/randomblob, one level up. Add them to the scalar blocklist.
- Prose number-gate missed трилион/билион/квадрилион: '3 трилиона лева' slipped the whole gate (the digit can't reach 'лева' across the Cyrillic word), an unbound order-up figure on a public report — the '12 млрд.' vector one magnitude higher. Add them to the spelled-magnitude stem. - Validate the optional column align against a left|right whitelist, and build resolved table columns explicitly instead of spreading the model object, so no unknown/unvalidated property reaches the renderer. - Cap model-emitted array lengths (blocks, items, columns) in validateEmitShape.
…h prompt paths
renderTraps() now owns the numbered-list rendering that describeSchema (full
dictionary) and the RAG hard-traps block duplicated, so the two paths cannot
drift, and the full-dictionary heading is harmonised to match the RAG block
("Задължителни правила за данните"). No behaviour change — string assembly only.
retrieveSchemaContext relied on `m.score` always being numeric. If an index backend ever returns a match without a `score`, the comparison was falsy and the chunk was dropped — the correct, safe outcome, but only incidentally. Make it explicit with `(m.score ?? 0) >= minScore` and a comment so a future refactor can't strip the guard, and cover it with a test. Addresses the review note on rag.ts robustness (ydimitrof).
…vers квинтилион+) The prose-number gate listed magnitudes explicitly and stopped at квадрилион, so "3 квинтилиона лева" slipped. Match the shared suffixes instead — милион⊃"илион", милиард⊃"илиард" — which covers the whole family (милион…секстилион…, милиард…) and closes the row upward for good rather than chasing an endless list. Addresses the review note on report-schema.ts (ydimitrof).
… the SQL guard string_agg(X, sep) is the official SQLite 3.44 synonym of group_concat and reaches the same code path on D1's modern SQLite, so it bypassed the scalar/aggregate denylist and achieved the same memory amplification (whole scan into one cell before capRows) the guard just closed for group_concat. Add it to the regex and the adversarial test. Addresses the review note on sql-guard.ts (ydimitrof).
An over-cap blocks/items/columns array is exactly the unbounded structure the ceilings guard against, yet validateEmitShape recorded the length error and then walked the whole array anyway — doing the very scan the cap exists to refuse. Return before the per-block scan on oversized blocks, and skip the per-element scan on oversized items/columns. Behaviour is unchanged for valid reports (ok:false either way); this only stops the wasted walk. Test asserts a single cap error with no per-element errors, proving the array is not scanned. Addresses lyubomir-bozhinov's review note on PR midt-bg#223.
…d RAG retrieval DATA_TRAPS are injected into the system prompt unconditionally (hardTraps), so indexing them in the schema corpus let retrieval hand the same rule back as "context" and render it twice. Traps are no longer indexed, and retrieveSchemaContext drops kind:'trap' matches a previously deployed index may still hold. Retrieval's job stays selecting relevant tables/queries. (review note, ydimitrof)
…ace instead of a runtime trap filter Self-review of the previous commit found the client-side kind:'trap' filter ran AFTER Vectorize's server-side topK cut, so legacy trap vectors (12 of ~37 in a pre-change index, and the most money-question-similar text in the corpus) could eat up to all six retrieval slots — leaving the turn with fewer tables/queries than the no-RAG fallback, silently and permanently, since upsert never deletes the stale ids. Replaced with a versioned NATIVE namespace (SCHEMA_NS = 'schema-v2') on both the upserted vectors and the query: native namespaces need no metadata index and exclude every stale cohort at the source, so no topK slot is ever spent on a discarded match and the filter is gone. The version is in the vector ids too, so re-indexing writes a new cohort and a Worker rollback keeps working against the old one. An un-reindexed environment gets zero matches → the documented full-dictionary fallback. Also from the self-review: the stale module header still said trap-rules are embedded; system-prompt tests fed trap strings retrieval can no longer produce; and no test entered through the composed seam — added a retrieveSchemaContext → buildSystemPrompt test seeded with the real corpus asserting every DATA_TRAP renders exactly once (negative-controlled: re-adding traps under a disguised id/kind fails it and the new corpus-length assertion). README provisioning now documents the re-index-on-bump requirement.
Gap-sweep on the namespace fix found the composed exactly-once test was weaker than advertised: it hand-mirrored the write mapping instead of running indexSchemaCorpus, sliced only the first topK chunks (so a trap appended at the corpus tail escaped it), and hard-coded a 0.9 score silently coupled to MIN_SCHEMA_SCORE. It now routes through the real write path into a recording fake, retrieves the WHOLE corpus, and derives its score from the floor — so the write→read metadata contract (text key, ids, namespace) is under test and a trap re-added at any position under any id/kind fails it (negative-controlled again with a tail-appended, table-kind trap). Also: remaining fixtures moved off pre-v2 unversioned ids; the semanticSearch test title no longer claims a namespace it does not use (it pins the entity METADATA filter); the module header no longer claims the bindings satisfy the structural types (the route casts — drift is not tsc-checked); the new-cohort rollback guarantee is now correctly stated as bump-only, with an explicit WHEN TO BUMP rule (in-place upserts, positional query ids, orphan risk); the README no longer suggests purging a cohort inside its rollback window and notes delete-vectors needs an explicit id list; dropped the stale '150 теста' verification claim.
… величините
Единствените near-collisions на суфиксния шаблон са думи на -лион
(напр. „Илион") — приети съзнателно: gate-ът нарочно флагва в повече,
а в регистъра на поръчките такива думи почти не се срещат. Записан е
изходът при евентуални фалшиви отхвърляния: \p{L} lookaround граница
(JS \b е ASCII-only), а не списък с изключения. Изброяването на
-илиард величините е сведено до реалните форми на „милиард"
(бележка от ревюто).
… gate-а „Дванадесет млрд. лева" нямаше нито цифра (за \d…млрд шаблона), нито пълнословен суфикс — изписано числително + абревиатура се промъкваше покрай целия gate. млрд/млн влизат в стем шаблона (флагват и без цифра; негативен контрол: тестът пада без промяната). Остатъкът „хил." без цифра остава приет — хилядите не са defamation-мащабният вектор (бележка от ревюто на midt-bg#320).
…enylist SQLite (D1) резолва "group_concat"(x), [group_concat](x) и `group_concat`(x) до същия built-in, а регексът изискваше голо име непосредствено пред скобата — цитиран идентификатор минаваше L1. Опционален quote клас след името затваря и трите форми (adversarial тестове; негативен контрол: падат без промяната). Идентификатор с padding в кавичките е РАЗЛИЧЕН за SQLite и не резолва built-in — не изисква обработка (бележка от ревюто).
…ема пътя Без флор, щом entity корпусът се напълни, top-K връща K-те най-близки съседа ДОРИ когато всички са off-topic, и те стигат до модела като реални hits. MIN_ENTITY_SCORE (симетричен на MIN_SCHEMA_SCORE) реже под прага; match без score се чете като под флора и отпада — същото защитно правило като схема пътя. Тестовете деривират скоровете от флора ± ε (бележка от ревюто на midt-bg#319).
…-namespace кохорта - Number.isFinite вместо ?? 0 във флор филтъра на semanticSearch: (undefined ?? 0) >= 0 промъкваше match без score като 'hit' при изричен minScore = 0, а истински score 0 при флор 0 е легитимен — двата случая вече са разграничени (+ тест). След филтъра score е гарантирано число и DTO-то няма нужда от fallback. - README: 'стар кохорт' изрично включва и оригиналния pre-namespace кохорт (id-та в DEFAULT namespace отпреди версионирането) — за първите среди той също е orphan за чистене (бележки от ревюто).
…та да не зависи от стойността на флора) Този PR въвежда entity флора с Number.isFinite точно за да не пропусне scoreless match при minScore = 0 — но остави схема пътя на (m.score ?? 0), т.е. асиметрия, въведена в същия PR. При подразбиращия се 0.35 двата се държат еднакво, но извикване с minScore = 0 би пропуснало match без score като „контекст". Изравнено; тест точно за minScore = 0 (негативен контрол: връщането на ?? 0 го чупи). Бележка от ревюто на @ydimitrof.
…s' на route границата (midt-bg#316) env.VECTORIZE вече се присвоява на VectorIndex БЕЗ каст: metadata на VectorRecord е стеснен до стойностите, които Vectorize приема, а неизползваемият metadata filter отпадна от интерфейса — така tsc доказва присвоимостта и дрейф между rag.ts и worker-configuration.d.ts чупи typecheck-а, не продукцията (негативен контрол: върнат filter член → TS2322 на самото присвояване). env.AI не може да удовлетвори EmbeddingRunner структурно (run() връща per-model union), затова route-ът минава през типизиран адаптер, който вика реалния @cf/baai/bge-m3 overload — също проверен от компилатора — и подава само embeddings члена; embed() и без това fail-fast-ва при малформен data. Кастовете към AgentEnv остават: BGGPT_API_KEY е secret и не присъства в генерирания Env — отделен въпрос от AI/Vectorize биндингите. Closes midt-bg#316
- EmbeddingRunner.run вече взима model: typeof EMBED_MODEL (литерала), а адаптерът го препраща в реалния Ai.run overload — втори, различен модел би бил компилационна грешка, не тихо embed-ване с грешния модел. - Адаптерът е изнесен в bindings.ts (embeddingRunnerFor) — единственият модул, който познава и двете страни на границата; unit тестван, включително [] случаят, който инлайн версията оставяше непокрит. - При неочаквана форма на отговора адаптерът хвърля именувана грешка (само ключовете, без payload — error envelope може да ехне въпроса) вместо да връща [], което се четеше като 'провайдърът не embed-на нищо'. - Header коментарът на rag.ts е разделен по интерфейс: VectorIndex е присвоим от VectorizeIndex (без каст), EmbeddingRunner нарочно НЕ е; поправено и погрешното твърдение, че Vectorize няма filter поле. - README provisioning gate-ът вика indexSchemaCorpus през embeddingRunnerFor — голият env.AI вече не typecheck-ва там.
…е като успех
[] е truthy — проверка само за присъствие връщаше { data: [] } за
непразен вход и embed() после обвиняваше '0 embeddings' вместо реалната
причина: провайдър, отговорил с празен batch. Празният масив вече е
именуван отделен случай в грешката на адаптера (+ тест; бележка от
ревюто на midt-bg#320).
Ранното връщане при `texts.length === 0` е това, което прави вярно „адаптерът никога не се вика с празен вход" за ВСЕКИ извикващ — не само за днешните три. Досега контрактът беше негласен (споменат само в bindings.ts); сега е записан на самото място, което го гарантира, с указание да не се мести под run(). Тестът в rag.test.ts вече закова, че моделът не се вика за []. (бележка от ревюто на midt-bg#320, ydimitrof)
…rieval-а + логнати деградации retrieveSchemaContext подава RetrievalStats (matched срещу kept) през опционален onStats hook; route-ът ги логва и вече не поглъща тихо грешката при retrieval (console.error + fallback, в стила на другите деградационни пътища на файла). kept=0 при matched>0 е точно тихият fallback към пълния речник, който досега беше неразличим от работещ RAG. Позиционните topK/minScore станаха opts обект — така сигнатурата носи и hook-а без опашка от undefined аргументи. Флорът MIN_SCHEMA_SCORE остава непипнат: коментарът му вече казва изрично, че е калибриран срещу пре-v2 корпуса и чака емпирично премерване — това е втората половина на midt-bg#318, която ще стъпи върху тези логове. Част от midt-bg#318
…вюто - onStats се вика в try/catch (reportStats) — хвърлящ metrics sink не може да струва на хода неговите чънкове (инвариантът на request-log.ts); закован с тест. - Третият деградационен път (!vec — embed без използваем вектор) вече също докладва, с нули — иначе е неразличим от 'статистиката не е вързана'. - kept се раздели от aboveFloor: aboveFloor > kept сигнализира счупен metadata.text контракт (бъг в индексирането), не флор — двата класа повреди имат противоположни поправки. - Логът на route-а е структуриран JSON (стилът на request-log.ts), а error логът подава само message — суров error обект може да ехне въпроса на потребителя в логовете. - Тестът за статистика деривира скоровете от MIN_SCHEMA_SCORE ± ε и затвърждава и върнатите чънкове, не само броячите. Част от midt-bg#318
…isFinite и по схема пътя
- semanticSearch подава SemanticSearchStats (matched/kept) през същия
best-effort reportStats hook; tools.ts ги логва структурирано
({evt:'assistant.semantic'}) — след напълването на entity-v1 (Фаза 2)
'флорът отряза всичко' и 'празен namespace' са различими и там.
- Схема флорът също мина на Number.isFinite (симетрия с entity ръба при
изричен minScore = 0 в RetrieveOptions).
- Error логът на semantic_search подава само message — провайдърски
error envelope може да ехне текста на заявката (същата дисциплина
като route-а). (бележки от ревюто на midt-bg#321)
…а модула Третият catch в route-а още логваше суровия error обект, докато същият файл вече два пъти пази 'само message' заради ехо на въпроса в логовете. Същият клас имаше и в run_sql (D1 грешка носи SQL-а, построен от въпроса) и в stream onError (BgGPT грешка носи prompt-а). Вместо трети copy-paste на тернара — errorText() в log-safety.ts, използван на ВСИЧКИТЕ пет catch/лог места: само message (никога stack, никога cause веригата, никога обекта), с таван от 300 знака срещу гигантско тяло от провайдър. Тестът закова точно тези инварианти (негативен контрол: връщане на stack/cause го чупи). Бележка от ревюто на @lyubomir-bozhinov по midt-bg#321.
…вариант пазеше грешната ос Само-ревюто на предишния комит намери три дефекта в него: 1. НЕ беше тотален: String(Object.create(null)) хвърля (проверено), а Proxy/хостилен message getter — също. Помощник, който живее в пет catch блока, не може да хвърля: изключението щеше да избяга от catch-а, да убие graceful fallback-а (пълния речник / стрийм линията) и да върне 500, при това с целия обект в лога на framework-а. 2. Филтрираше по грешната ос: махаше stack-а (който по конструкция НЕ носи потребителски текст) и пазеше message-а (където точно се появява ехото от провайдъра). Затова сега сайтовете, които знаят входа си, го подават за РЕДАКЦИЯ: въпросът (route + stream през ctx.userQuestion) и заявката (semantic_search). Това е частта, която реално затваря ехото; тапата и без-stack-а само ограничават щетата. 3. Тестът твърдеше 'обект се свежда до непрозрачен таг' — невярно за обект със собствен toString (проверено). Тестът вече документира реалното поведение, а не желаното. Освен това: collapse на нови редове (многоредово съобщение чупеше prefix-базиран grep — продълженията нямат [assistant] префикс), рязане по кодови точки (не режем емоджи на самотен surrogate), и връщане на stack frames там, където те са диагностиката — setup грешката в route-а е конфигурационна, не провайдърска. Негативни контроли: махането на try/catch чупи теста за тоталност, махането на редакцията чупи теста за ехото.
…о на съобщението
errorText свиваше съобщението до един ред преди split, но подаваният needle
(въпросът) оставаше суров — latestUserText прави само join(' ').trim(). Въпрос с
shift-enter, таб или двоен интервал, ехнат от провайдъра, вече не съвпадаше и
влизаше дословно в лога — точно гаранцията „no verbatim copy", която модулът
декларира.
Сега needle-ът минава през същото collapseWhitespace, а прагът MIN_REDACT_CHARS
се преценява по свитата форма (whitespace не може да издуе къс needle над него).
typeof guard пази тоталността срещу не-низ през unknown границата.
Тестове: repro на ревюто (needle с \n/\t/двоен интервал → «редактирано»),
floor по свитата форма, тоталност при не-низ. Негативен контрол: repro тестът
пада срещу старата имплементация.
…d/отрязано ехо Две дупки в log-safety, намерени при ревю на клона: 1. stackHead приемаше, че съобщението заема само ред 0 на stack-а. V8 печата ЦЯЛОТО (многоредово) съобщение преди първата рамка, така че `.slice(1, 1 + frames)` връщаше продълженията на съобщението — нередактирани и без таван — точно до редактирания errorText на същия ред в route-а. Сега се котви на първия ` at ` ред и пази само рамки; без разпознаваема рамка (друг runtime, hostile getter) връща '' — fail closed. 2. errorText редактираше само дословно (whitespace-свито) копие на needle-а. Провайдър, който връща JSON тялото като message, ехва входа escaped (`"` → `\"`, нов ред → двата знака `\n`); embed пътят праща отрязан вход; и двете минаваха покрай split-а. Сега се търсят и JSON-escaped формата на суровия needle, и прозорци от 24 знака за дълъг needle (отрязано ехо се бланкира като един run). Plus: суровото съобщение се отрязва до 19 200 UTF-16 единици (surrogate-safe) ПРЕДИ O(n) паса — многомегабайтово тяло вече не струва стотици ms в catch блок; capCodePoints пропуска Array.from при къс вход. Тестове: многоредово съобщение → нито ред от него в stackHead; stack без рамки → ''; JSON-escaped ехо и отрязано ехо → «редактирано»; дълъг needle при пре-cap → един run; несвързано съобщение непокътнато; surrogate на границата на пре-cap-а. Негативен контрол: 4 от новите тестове падат срещу старата имплементация.
… тестове през вратата на потребителя Ревю на клона намери, че „въпросът не влиза в лога" не е вярно на три места, въпреки errorText: 1. streamText има СОБСТВЕН default onError = console.error(error) — суровият APICallError с requestBodyValues (system prompt + съобщенията на потребителя) и responseBody. agent.ts подаваше onError само на toUIMessageStreamResponse, така че SDK-то продължаваше да пише суровия обект при всяка провайдърска грешка. Сега hook-ът е и на streamText (една редактирана линия на грешка, дедуп по идентичност), а UI hook-ът само решава какво вижда клиентът. Бонус: линията носи класа и HTTP статуса (и през RetryError) — 401 vs 429 vs 5xx пак са различими, само идентификатори, никога тяло. 2. Route-ът: catch-ът на RAG retrieval викаше errorText(error) БЕЗ needle, а точно той embed-ва въпроса — най-вероятното място за ехо. Вече подава [question] като останалите три места. 3. bindings.ts: `'data' in out` хвърля при не-обект, а V8 слага операнда в съобщението на TypeError-а — gateway, върнал 200 с текст/примитив, вкарваше тялото в лога през „keys-only" пътя. Сега се именува само типът. Тестове през вратата на потребителя (CLAUDE.md: „at least one test must enter through the same door as the user"): apps/web/app/routes/assistant.chat.test.ts кара POST-а през action() с фалшиви AI/VECTORIZE — stats линията е само броячи, RAG-провалът с ехо е редактиран, setup-провалът дава 503 с редактирана и само-рамки линия. agent.stream-error.test.ts кара runAssistant през реалния streamText с MockLanguageModelV3 — точно една низова линия, никога суровият обект. Негативни контроли: без needle-а route тестът пада; срещу старото agent.ts console.error се вика два пъти (втория път с обекта).
Три дупки от ревюто на клона по gate-овете на отчета: - Прозата: голият суфикс -илион хващаше „павилион(и)" — рутинен предмет на поръчка — и отхвърляше легитимно заглавие като несвързано число, което моделът не може да пренапише. Суфиксите вече са котвени към числителните представки (м/б/тр/квадр/квинт/секст/септ/окт/нон/дец) — затворено нагоре до 10^33, „Илион" и „билярд" вече не флагват, „милионер" още (безопасната посока). - emit-report-schema: `sub` на facts не се проверяваше по тип — число минаваше shape-а и хвърляше TypeError в bindReport (непрозрачна tool грешка към модела вместо retryable съобщение, и несканирана цифра). `align: null`/`link: null` („не е зададено" на модела) вече се приемат като отсъстващи, вместо да обръщат иначе валиден отчет в retry. Трикратният copy-paste на cap guard-а е един cappedArray helper със същите съобщения (диференциално проверени). - bindReport: `link` се пресъздава от двете си известни полета, а не се копира по референция — допълнителен ключ вътре (href) иначе влизаше в замразения отчет въпреки „само известни полета стигат до рендера". Тестове за всяко; негативен контрол: новите тестове падат срещу старите файлове. JSONB близнаците в SQL денилиста (jsonb_group_*), първоначално част от този комит, са пренесени в основата на стека (midt-bg#223), където денилистът се въвежда.
…агва само след числително Две находки от втория преглед на клона: - describe-schema: новият запис за `amendments` изброяваше value_before/after/ delta като обикновени колони, без да каже, че са в `currency` (не в EUR — не се сумират между валути), че `value_suspect = 1` редовете носят удвоена, ненадеждна стойност, която самият сайт бланкира (миграция 0007), и че връзката към contracts е по ДВЕ колони (t.source_id = am.unp AND c.contract_number = am.contract_number) — само unp дава декартово преброяване. Точно класата SUM(amount) капан, заради която речникът съществува, отворена върху таблицата, която PR-ът току-що направи привлекателна. Записът вече носи и value_suspect, и value_restated. - report-schema: голите стемове `млрд|млн` (добавени за „дванадесет млрд.") флагваха и чисто мерни заглавия като „Стойност (млн. €)" — стилът на собствените колони на сайта, без никакво число — и обръщаха валиден отчет в retry, докато „(хил. €)" минаваше. Абревиатурата флагва само след изписано числително (затворен клас: 1–19, десетици, стотици + няколко/ десетки/стотици); „Сто-йност" не е „сто" (word-edge lookaround). Тестове: двете посоки за млн/млрд (числително → флаг; само единица → не), bindReport с header „Похарчено (млн. €)" минава.
…ции вместо позиционни за semantic_search - reportStats ловеше само синхронен throw; `(stats) => void` приема и async sink, чийто reject избягваше като unhandled rejection (runtime-ът го пише като грешка — обратното на best-effort). Върнатият promise вече се обезврежда, извикването остава синхронно (старият тест за хвърлящ sink пак минава). - Нито един тест не заковаваше `returnMetadata: 'all'`: фалшивият индекс връщаше metadata.text без да е поискан, така че махането на опцията оставаше зелено, докато реалният Vectorize (default 'none') дава text-less съвпадения, kept=0 и тих fallback към пълния речник на всеки ход. Двата namespace теста го pin-ват, а фалшивият индекс в system-prompt.test дава metadata само при 'all' — seam-ът вече доказва и четящата страна. - semanticSearch взимаше (topK, minScore, onStats) позиционно, докато сестринската retrieveSchemaContext — опции обект; единственият извикващ пишеше `undefined, undefined, cb`. Вече SemanticSearchOptions, огледално. - Фикстурите на два floor теста бяха твърди 0.6/0.1 — рекалибрация (midt-bg#318) над 0.6 щеше да ги събори за несъществуваща регресия; изведени от константата. - tools.test: semantic_search през runTool — рендер на hit-овете, stats линията само броячи, needle-ът на tool-а редактира ехо на заявката. - README/rag.ts: „редакция на текст минава без bump" подвеждаше — retrieval-ът връща metadata.text от индексирането, така че ВСЯКА промяна в корпуса иска повторно indexSchemaCorpus; bump-ът решава само нов кохорт vs. презапис. Таблицата на модулите вече изброява bindings.ts и log-safety.ts. Негативни контроли: старият reportStats → 1 unhandled rejection уловен; без returnMetadata → 3 теста падат.
…tream грешки - „три трлн лева" се промъкваше през целия prose gate: няма цифра (за \d…-шаблона), няма пълнословен -илион стем и няма млн/млрд. Добавено към същия numeral+абревиатура клон. Само реално използваните в български финансов текст съкращения — измислено „квдрлн" би бил шаблон, който никой не пише, а пълната дума вече се лови от стема. - Дедупът на stream грешките ползваше WeakSet, т.е. работеше само за обектни грешки; примитив (низ/число), видян и от двата onError hook-а, се логваше двойно. Примитивите вече се дедупват по вече-редактирания текст, а обектите остават по идентичност (две различни грешки с еднакъв текст заслужават по ред). Логиката е изнесена в makeStreamErrorLogger, за да е тествана без реален стрийм — 4 теста, вкл. редакцията. Бележки от ревюто на @ydimitrof по midt-bg#321.
…еният списък числителни течеше Клонът „числително + абревиатура" пазеше със ЗАТВОРЕН списък кардинални числителни. Обикновена българска финансова проза минаваше покрай него и замразяваше необвързана сума в публичния отчет: „два и половина млрд. лева" (думата пред единицата е „половина"), „двайсет млн.", „трийсет млрд.", „стотина млн.", „десетина млн.", „четвърт млрд.", „дузина млн.", „няколкостотин млн." — всички връщаха ok:true през bindReport. Точно „12 млрд."-векторът, за който gate-ът съществува, с изписано число. Правилото вече е обърнато: абревиатурата флагва след ВСЯКА дума, освен след предлог за мерна единица (в/във/на/по/от/до/за/към/при/с/със/и/или — „Стойност в млн. лв.", „изразени във млрд."). Гол остава само след пунктуация/начало на ред — „Стойност (млн. €)", „Похарчено, млрд. лв." — стилът на собствените ни колони. „Стойност млн. €" (съществително директно пред единицата) вече флагва: без пълен речник на числителните е неразличимо от „стотина млн.", а безопасната посока е over-flag (моделът слага единицата в скоби). „трлн" влиза и в цифровия клон — „12 трлн. лева" / „1,5 трлн" (формата, която моделът най-често пише) минаваше целия gate, макар „три трлн лева" да се хващаше (ревю на ydimitrof по абревиатурния клон). Тестове: десетте формулировки + bindReport с текстов блок; предлозните и скобените форми остават чисти; негативен контрол: и трите променени/нови теста падат срещу стария regex. (независим преглед след ревюто на midt-bg#321)
…з pre-cap-а Фикстурата беше 18 105 UTF-16 единици при праг MAX_RAW_CHARS = 19 200, така че preCap() връщаше суровото съобщение непроменено и тестът „Pre-cap regression guard" никога не влизаше в пътя, който коментарът му твърди, че пази — регресия в реда pre-cap/collapse/redact щеше да остане зелена. Фикстурата вече е ~21 100 единици, MAX_RAW_CHARS е експортнат и тестът заковава `question.length > MAX_RAW_CHARS`, за да не може да се смъкне тихо под прага отново. Очакваният изход остава същият: една редактирана линия „err: «редактирано»". (независим преглед след ревюто на midt-bg#321)
…i cap-а, ok пътя на emit_report и отказите на route-а Клонове от кода в този PR, които суитът не докосваше след пребазирането върху новия coverage baseline на main (midt-bg#254): - errorTag: RetryError, който обвива НЕ-APICallError — тагът е само причината, без статус; - log-safety: needle между MIN_REDACT_CHARS и REDACT_WINDOW се редактира по точен match, а съобщение с повече UTF-16 единици от cap-а, но по-малко кодови точки, не се реже; - agent wiring: валиден emit_report минава по ok клона и връща вързаната справка; - route door: и петте отказа на входа (обявен/реален над-cap body, невалиден JSON, нула ходове след филтъра на ролите, един гигантски ход) връщат 4xx, без да стигат до модела. Content-Length се подава през незащитен Headers обект, защото истинският Request го изпуска.
…ната към типа
Изричното изграждане на колоната в bindReport (вместо `{ ...c }`) копира
фиксиран списък полета. Ако EmitTableColumn някога получи ново незадължително
поле, старият код би го изпуснал тихо — без TS грешка (ревю на midt-bg#330,
ydimitrof). REBUILT_COLUMN_KEYS/REBUILT_LINK_KEYS са `Record<keyof …, true>`
обекти: липсващо или излишно свойство е компилационна грешка, така че
списъкът не може да изостане от типа. Тестът закова и runtime страната —
напълно зададена колона излиза с точно тези ключове, а непознат ключ (вкл.
`href` вътре в `link`) не оцелява. Негативен контрол: `width?: number`,
добавено към интерфейса, чупи `tsc -b` на пина.
…рифт-пазач) Речникът, който моделът чете (describe-schema.ts), дрифтна тихо веднъж: amendments.contract_id никога не е съществувала, а parties.role — също, и двете се откриха само на око при ревюто на midt-bg#321. Всяка фантомна колона там влиза във всеки prompt, а полученият SQL пада в runtime с грешка, която моделът „поправя" с ново налучкване. Тестът прилага ВСИЧКИ packages/db/migrations/*.sql поред върху временна sqlite3 база (same harness като packages/db/src/migrations.test.ts) и проверява: - всяка таблица от речника съществува; - всеки водещ идентификатор от всеки columns низ е реална колона (pragma_table_info; скобените бележки със запетаи/кавички се игнорират); - всяка →таблица референция е реална таблица; - всяка канонична заявка КОМПИЛИРА срещу схемата (EXPLAIN) — тях моделът копира дословно като отправна точка; - негативни контроли: историческият дрифт ('id, contract_id→contracts, …') се хваща, а parser-ът не се лъже от бележки в скоби. Проверено срещу main-версията на речника: пазачът докладва точно amendments.contract_id и parties.role. Понеже поправката им живее в midt-bg#321, клонът е стакнат върху него (мърдж ред: midt-bg#321 → този). Тестът живее в apps/web/test/ и е включен в tsconfig.node.json (Node типове за child_process), за да не се наливат Node globals в Workers кода на приложението; vitest include покрива test/**.
… + дрифт-пазачът да не пропуска цитирана колона - redact вече се строи от userTexts(opts.messages) — всички user ходове. Провайдърска грешка може да цитира която и да е част от prompt-а, а multi-turn prompt носи и по-ранните въпроси; редакция само на ctx.userQuestion ги оставяше нередактирани в tail лога. userTexts е експортнат и тестван (вкл. че игнорира не-user роли и празни текстове; негативен контрол: 'само първия' чупи теста). - Дрифт-пазачът сваля SQL кавички преди да чете идентификатора: цитирана колона на топ ниво („col", [col], ) досега тихо се пропускаше и не се сверяваше срещу реалната схема — guard, който фейлва ОТВОРЕНО. Днес речникът не цитира; така остава затворен, ако някога го направи. - format НЕ става условен: validateEmitShape изисква isFormat(c.format) за всяка колона, а EmitTableColumn го типизира задължителен, така че 'format: undefined' е недостижим — предпоставката на бележката не важи. Записано като коментар на самото място, за да не се пита пак. Бележки от ревюто на @ydimitrof по midt-bg#330.
Класът на идентификатора беше само ASCII (`[A-Za-z_]`), така че колона с кирилско име щеше да се филтрира ПРЕДИ проверката срещу схемата — пазачът да фейлва отворено точно за колоната, която трябва да хване (ревю на midt-bg#330, ydimitrof). SQLite допуска такива идентификатори. Класът вече е `\p{L}`/`\p{N}` с `u` флаг, симетрично и за →референциите. Негативен контрол: кирилска фантомна колона се докладва срещу served схемата; извлеченото от днешния речник не се променя (проверено).
5ada70b to
5dab94d
Compare
Свързан issue
Няма closing issue — находката идва от ревюто на PR #321, където историческият дрифт (
amendments.contract_id,parties.role) се откри на око; този PR превръща следващия такъв в червен build. По духа на #294: контролите на самия пазач са изпълними тестове в suite-а (небалансирана скоба пада шумно, анти-вакуумна проверка на →референциите), а не само твърдения в това описание.Какво
Дрифт-пазач: речникът на данните, който моделът чете преди да пише SQL (
describe-schema.ts), се заковава срещу РЕАЛНАТА served схема — всичкиpackages/db/migrations/*.sql, приложени поред върху временна sqlite3 база (същият harness катоpackages/db/src/migrations.test.ts).Защо
Речникът дрифтна тихо:
amendments.contract_idникога не е съществувала,parties.role— също, и двете се откриха само на око при ревюто на #321. Всяка фантомна колона там влиза във всеки prompt, а полученият SQL пада в runtime с грешка, която моделът „поправя" с ново налучкване. Следващият такъв дрифт вече е червен build.Как
TABLESсъществува (sqlite_master);columnsниз е реална колона (pragma_table_info; скобените бележки със запетаи/кавички се игнорират от parser-а);→таблицареференция е реална таблица, с анти-вакуумна проверка (речникът ДНЕС реферираtenders, така че умряла екстракция пада, вместо тестът да минава на празен списък завинаги);EXPLAIN) — тях моделът копира дословно като отправна точка;'id, contract_id→contracts, …') се хваща; бележките в скоби не лъжат parser-а; небалансирана скоба пада шумно (иначе всяка колона след нея тихо изпада от пазача — точно failure mode-ът, който suite-ът лови, скрит в самия suite).Разположение
Тестът е в
apps/web/test/и е включен вtsconfig.node.json(Node типове заchild_process), а не вapp/**— иначе Node globals щяха да се налеят в typecheck-а на Workers кода.describe-schema.tsе изрично изброен в composite проекта (нула import-и; коментар в tsconfig-а казва какво да се направи, ако някога добие import —tsc -bго именува с TS6307).Проверено
apps/web,pnpm --filter web typecheck→ 0, Prettier чист.amendments.contract_id+parties.role; изкорменаdictionaryRefs(връща[]) → 2 теста падат; махнат guard за небалансирана скоба → тестът за това пада.sqlite3CLI е вече изискване на CI (packages/db/src/migrations.test.ts,scripts-test.yml) — нищо ново за средата.