Conversation
ydimitrof
left a comment
There was a problem hiding this comment.
Ревю на PR: fix(assistant): semantic_search през native Vectorize namespace вместо metadata филтър
Обща оценка: 9.5/10 — APPROVE (с една незадължителна, насочена към бъдещето бележка).
Фаза 0 — Сигурност (задължителна, блокираща): ЧИСТО ✅
- Няма твърдо кодирани тайни.
BGGPT_API_KEYсе подава само презwrangler secret put(интерактивно), README изрично отбелязва „никога не се комитва". - Няма нови/съмнителни URL-и.
@cf/baai/bge-m3е Workers AI идентификатор на модел, не мрежов адрес. - Няма зловредни шаблони (backdoors, инжекции, обфускация).
- Зависимости: единствената промяна е
osv-scanner.toml— игнориране наGHSA-f88m-g3jw-g9cj(libvips CVE вsharp<0.35.0). Обосновката е коректна: транзитивна, само-dev зависимост през miniflare, извън разгърнатия Worker, без достъпна поправка в диапазона (^0.34.5), сignoreUntilдата. ✅ - Подобрение на сигурността: в
report-schema.tsрезолвнатите колони вече се строят ЯВНО поле по поле вместо{ ...c }— това спира пренасянето на непознати, модел-подадени свойства към рендера (validateEmitShape не отхвърля непознати ключове). Заедно с whitelist-аisAlign(left|right) това затваря вектор за атрибут-инжекция. Отлично.
Тестове (3.0/3.0) ✅
Изключително силно тестово покритие, писано да разкрива дефекти, не да минава тривиално:
rag.test.tsпинва литералаschema-v2/entity-v1(неSCHEMA_NS), така че bump-ът е съзнателен акт; проверяваtoHaveBeenCalledTimes(1)+expect.not.objectContaining({ filter })— exhaustively изключва скрит filter-based fallback; покрива scoreless match и relevance floor.system-prompt.test.tsтества през реалния write→read seam (indexSchemaCorpus → retrieveSchemaContext → buildSystemPrompt) и гарантира, че всеки trap се рендира точно веднъж (регресията с двойно рендиране).sql-guard.test.ts,report-schema.test.ts,emit-report-schema.test.tsпокриват новите пътища (агрегати, магнитуден суфикс, cap-ове, align).
Код-качество (2.0/2.0) ✅
renderTraps()е споделен междуdescribeSchema()иhardTraps()— премахва дублиране и drift на trap-текста между двата пътя (изпълнява NO CODE DUPLICATION).- Промяната от
/милиард|милион/към суфиксите/илион|илиард/е коректна (мил-ион ⊃ илион,мил-иард ⊃ илиард), затваря реда нагоре (квинтилион/секстилион) вместо ръчен списък; свръх-флагването е в безопасната посока за gate. - sql-guard разширението (
group_concat/string_agg/json_group_array/json_group_object) коректно адресира същия OOM-amplification клас катоprintf. Заслужава похвала честната бележка, че denylist-ът е catch-up игра и allowlist е трайната поправка.
Документация (2.0/2.0) ✅
README е обстоен: правилото „WHEN TO BUMP", версионираният native namespace, rollback-семантиката (нов кохорт до стария), безопасният fallback (0 чънка → пълен статичен речник) и предупреждението, че entity корпусът е data-derived и НЕ следва append-only bump правилото. Ясен операционен runbook преди deploy.
Производителност (2.0/2.0) ✅
MIN_SCHEMA_SCOREfloor предотвратява „частично grounding, по-слабо от no-RAG" — реален коректностен и качествен gain.- Native namespace изключва stale кохорти на източника (никакви пропилени topK слотове), без нужда от provisioned metadata index.
- sql-guard спира single-row OOM на изолата.
Security (agent-level, 1.0/1.0) ✅
Няма нови уязвимости; входната валидация и error handling са затегнати.
Дребни / незадължителни бележки (не блокират)
- Асиметрия във floor-а (вж. inline):
retrieveSchemaContextимаMIN_SCHEMA_SCORE, ноsemanticSearchняма релевантен праг — само деградира липсващ score до 0. Днес е безвредно (entity namespace е празен по дизайн), но когато entity indexer-ът от Фаза 2 напълни корпуса, нискорелевантни попадения ще стигат до модела като „hits". Препоръка: симетричен праг при активиране на индексера. - Почистване на стар кохорт е ръчно:
wrangler vectorize delete-vectorsиска изричен списък id-та, възстановими от git историята наbuildSchemaChunks— крехко, но документирано и небезопасно само при преждевременно триене (rollback остава без RAG). Приемливо за v1. - Структурни типове vs реални bindings: коментарът честно отбелязва, че
as unknown asкастът в route-а скрива drift от tsc спрямоworker-configuration.d.ts— заслужава внимание при бъдещи промени поVectorIndex.
CLAUDE.md съответствие: без частични имплементации, без TODO-симплификации, без дублиране, без мъртъв код, смислени тестове, консистентно наименуване, разделени концерни. Промяната е атомарна и фокусирана върху единствен концерн (native namespace миграция + свързаните hardening follow-up-и от предходни ревюта).
Препоръка: APPROVE — може да се мърдж-не безопасно; бележка №1 да се адресира заедно с entity indexer-а във Фаза 2.
ceb1942 to
b40f475
Compare
ydimitrof
left a comment
There was a problem hiding this comment.
Ревю на PR: fix(assistant): semantic_search през native Vectorize namespace
Фаза 0 — Сканиране за критична сигурност: ЧИСТО ✅
- Твърдо кодирани тайни: няма.
BGGPT_API_KEYнавсякъде се третира катоwrangler secretи изрично „никога не се комитва". - URL промени: няма нови URL-и;
@cf/baai/bge-m3е име на Workers AI модел, не мрежов адрес. - Зловредни шаблони: няма backdoor/eval/обфускация.
- Промени в зависимости: няма нови пакети.
osv-scanner.tomlдобавя потискане на CVE (sharp <0.35.0,GHSA-f88m-g3jw-g9cj) — обосновката е коректна (транзитивна dev-only зависимост през miniflare, не влиза в деплойнатия Worker; няма upstream fix в диапазона) и имаignoreUntil, който ще я извади наяве. Прието.
Резултат: не се блокира. Преминаваме към пълно ревю.
Обобщение по измерения
Архитектура / коректност (силно): Смяната от metadata filter: { ns } към native Vectorize namespace е правилният подход — native namespace-ите работят без provisioned metadata индекс (issue #317) и изключват стари кохорти на ниво заявка, вместо да хабят topK слотове. Версионирането в namespace-а И в id-тата (schema-v2:) прави ре-индекса cohort-safe спрямо Worker rollback. Правилото „WHEN TO BUMP" е ясно документирано и разграничението, че за entity-корпуса (data-derived) то НЕ важи едно към едно, е точно.
Сигурност (agent-level):
sql-guard.ts— добавянето наgroup_concat/string_agg/json_group_array/json_group_objectкъм денилиста затваря реален клас на memory-amplification (цял scan → една клетка предиcapRows). Правилно е разпознатstring_aggкато SQLite ≥3.44 синоним. Забележката, че позитивен allowlist е трайното решение, е коректна.emit-report-schema.ts— таваните за масиви (MAX_BLOCKS/ITEMS/COLUMNS) + short-circuit преди per-item scan-а е добра защита срещу необвързан вход;isAlignwhitelist предотвратява out-of-enum стойност да стигне до renderer, който я интерполира в атрибут/стил.report-schema.ts— експлицитното изграждане на колоните (вместо spread) блокира преминаването на непознати model-supplied ключове; суфиксният шаблон-илион/-илиардзатваря магнитудите нагоре. Приемането на near-collision (напр. „Илион") е съзнателен fail-toward-flagging избор — приемливо.
Тестове: Много силно покритие — floor-филтъра, scoreless match, exhaustive corpus-length проверка (никой trap chunk не се преиндексира), и композиционен тест през реалния write→read seam (indexSchemaCorpus → retrieveSchemaContext → buildSystemPrompt) с проверка „всеки trap се рендира точно веднъж". Тестовете са смислени, не тривиални.
Документация: README-секцията за ре-индексиране и почистване на стари кохорти е изчерпателна и обвързва операционните последствия (rollback прозорец) с кода.
Забележки (незадължителни)
- Натрупване на orphan вектори: старите
schema:*вектори (default namespace) остават в индекса завинаги след bump. Безвредни за retrieval (namespace ги изключва), но растат разхода/размера. README го признава като „не е задължително" почистване — ОК, но си струва проследяване в беклога. MIN_SCHEMA_SCORE/MIN_ENTITY_SCORE = 0.35са консервативни и добре обосновани; тъй като entity-корпусът е още непопълнен, стойността е неизбежно неоткалибрирана до първите реални данни — предвидете преразглеждане при пускане на entity indexer-а.
Вердикт
Няма блокиращи или критични находки. Кодът е чист, добре тестван и добре документиран. Слагам COMMENT вместо APPROVE само защото quality-gate изисква реално изпълнение на тестовия пакет (pnpm --filter web test) и typecheck, което не мога да изпълня в тази среда — препоръчвам APPROVE след потвърждение, че CI е зелен.
b40f475 to
9c2a59c
Compare
ydimitrof
left a comment
There was a problem hiding this comment.
Ревю на PR: fix(assistant): semantic_search през native Vectorize namespace вместо metadata филтър
Обобщение
PR-ът мигрира scoping-а на RAG извличането от metadata filter: { ns } към native Vectorize namespaces (schema-v2, entity-v1), премахва зависимостта от несъществуващ metadata index (#317), въвежда версиониране на корпуса, relevance floor за двата пътя (MIN_SCHEMA_SCORE / MIN_ENTITY_SCORE), изважда DATA_TRAPS от векторния корпус и ги инжектира безусловно чрез hardTraps(). Придружен е от подсилване на sql-guard, emit-report-schema капове, разширяване на prose-number gate-а и osv-scanner ignore за dev-only sharp CVE.
Обща оценка: ~9.4/10 — препоръка APPROVE. Промяната е атомарна, добре мотивирана и изключително добре покрита с тестове. Не открих блокиращи проблеми.
Phase 0 — Security-Critical Scan: CLEAN
- Hardcoded secrets: няма.
BGGPT_API_KEYсе борави само презwrangler secret put(документирано, интерактивно, некомитвано). - URL промени: няма нови външни URL-и;
@cf/baai/bge-m3е Workers AI capability, не мрежов ендпойнт. - Malicious patterns: няма backdoor/обфускация/eval.
- Dependencies: няма нови пакети; osv ignore за
GHSA-f88m-g3jw-g9cjе за транзитивен dev-only sharp през miniflare, извън деплойнатия Worker, сignoreUntilдата и ясен REMOVE WHEN — обосновката е коректна.
Security (agent-level): силна
sql-guardразширява денилиста с string-building агрегати (group_concat/string_agg/json_group_array/json_group_object) — реален memory-amplification клас, спрян предиcapRows. Затварянето на quoted-identifier байпаса ("group_concat"(x),[..],`..`) е точна находка. Регексът fail-ва към флагване, което е правилната посока.emit-report-schema:isAlignwhitelist + експлицитното изграждане на колони вreport-schema.ts(без{ ...c }) премахва канал за пропускане на непроверени model-полета към рендера — добро defense-in-depth срещу attribute/style injection.- Prose-number gate: покриването на
-илион/-илиардсуфиксите (вместо явен списък) затваря defamation-scale вектора нагоре; над-флагването на „Илион/Троя" е приемливо за register-а.
Tests: отлични
- Всеки нов клон има целеви тест: namespace литерали пиннати (умишлено не през константата), relevance floor (над/под/scoreless), cap short-circuit, quoted-identifier байпас, композиционен тест през реалния write→read seam с проверка „всеки trap точно веднъж". Тестовете са смислени, не тривиални.
- Забележка: не изпълних тестовия пакет в тази среда (typecheck/
pnpm --filter web testне са пуснати) — валидацията стъпва на статичен прочит на diff-а. Preмерджа потвърдете зеления пакет.
Code Quality / CLAUDE.md: съответства
renderTraps()премахва дублиране междуdescribeSchema()иhardTraps()— единен източник, не могат да дрейфнат.- Няма partial impl / TODO / dead code; коментарите документират инвариантите (WHEN TO BUMP, forensic-only metadata) добре.
Незначителни наблюдения (non-blocking) — виж inline коментарите
- Регекс-денилистът е по своята същност догонваща игра; positive allowlist остава трайното решение (вече е трекнато).
- Миграционен detail за orphan вектори в default namespace от евентуално предишно индексиране — уверете се, че README cleanup пътят го покрива.
Performance: без регресии
Relevance floor намалява шума в prompt-а; native namespace изключва stale кохорти на ниво индекс (не хаби topK слотове). Няма нови N+1 или неограничени скани — каповете (MAX_BLOCKS/ITEMS/COLUMNS) добавят горна граница там, където преди нямаше.
Documentation: пълна
README покрива ре-индексиране, версиониране, rollback прозорец и разликата entity-vs-schema BUMP правилото. Точно и актуално.
5a3dc4c to
5d05e3d
Compare
…ема пътя Без флор, щом entity корпусът се напълни, top-K връща K-те най-близки съседа ДОРИ когато всички са off-topic, и те стигат до модела като реални hits. MIN_ENTITY_SCORE (симетричен на MIN_SCHEMA_SCORE) реже под прага; match без score се чете като под флора и отпада — същото защитно правило като схема пътя. Тестовете деривират скоровете от флора ± ε (бележка от ревюто на midt-bg#319).
5d05e3d to
d312569
Compare
…ема пътя Без флор, щом entity корпусът се напълни, top-K връща K-те най-близки съседа ДОРИ когато всички са off-topic, и те стигат до модела като реални hits. MIN_ENTITY_SCORE (симетричен на MIN_SCHEMA_SCORE) реже под прага; match без score се чете като под флора и отпада — същото защитно правило като схема пътя. Тестовете деривират скоровете от флора ± ε (бележка от ревюто на midt-bg#319).
d312569 to
3c85cae
Compare
…ема пътя Без флор, щом entity корпусът се напълни, top-K връща K-те най-близки съседа ДОРИ когато всички са off-topic, и те стигат до модела като реални hits. MIN_ENTITY_SCORE (симетричен на MIN_SCHEMA_SCORE) реже под прага; match без score се чете като под флора и отпада — същото защитно правило като схема пътя. Тестовете деривират скоровете от флора ± ε (бележка от ревюто на midt-bg#319).
ffa2d29 to
96d0100
Compare
ydimitrof
left a comment
There was a problem hiding this comment.
Прегледах PR-а като инженер на Sigma. Основната промяна — semantic_search и schema retrieval да минават през нативен Vectorize namespace (entity-v1 / schema-v2) вместо metadata filter { ns } — е коректна и добре обоснована: metadata филтър изисква provisioned metadata индекс, какъвто репото няма (#317), а нативните namespace-и изключват старите кохорти още на източника, така че pre-v2 trap-векторите не могат да заемат topK слотове. Версионирането на namespace-а и id-тата (нов кохорт до стария) пази rollback на Worker-а, а неиндексирана среда пада безопасно към пълния статичен речник.
Проверих приоритетно:
- Злонамерен код / сигурност: няма exfiltration, нови мрежови извиквания или подозрителни зависимости. Добавянето в osv-scanner.toml (sharp през miniflare, dev-only, с ignoreUntil) е добре мотивирано и легитимно. Няма prompt-injection към ревюто; текстът за игнориране на инструкции в тестовете е защитна клауза, не команда.
- Инжекции: SQL guard-ът е разширен смислено — string-building агрегатите (group_concat / string_agg / json_group_*) и quoted-identifier байпасите вече се хващат; регексът fail-ва към блокиране, което е правилната посока. XSS повърхността при align е затворена с whitelist в validateEmitShape плюс експлицитно изграждане на колоните в bindReport (без spread на непознати ключове).
- Data integrity: hard-traps се инжектират безусловно през hardTraps(), така че RAG turn никога не остава с по-малко ограничения от no-RAG fallback — добро решение на регресията с двойно рендиране/липсващ trap. Relevance floor-овете предпазват от инжектиране на off-topic контекст.
- Коректност: cap-овете на масивите (blocks/items/columns) и short-circuit при over-cap са тествани и разумни; Number.isFinite в entity пътя коректно отхвърля scoreless match при всеки floor.
Едно оперативно уточнение (не блокира): вече deploy-натите среди трябва да пуснат indexSchemaCorpus наново след мърдж, иначе schema-v2 е празен и RAG grounding-ът временно пада към пълния речник — безопасно, но добре е да е стъпка в runbook-а на deploy. README го покрива.
Кодът е чист, добре тестван и добре документиран. Оставям само една дребна бележка за консистентност.
96d0100 to
ac8840f
Compare
lyubomir-bozhinov
left a comment
There was a problem hiding this comment.
Прегледах #319 (собствените 5 комита над #223, при HEAD ac8840ff) — фокус: native Vectorize namespacing + relevance floor.
Data isolation — чисто: и трите Vectorize извиквания подават явен, hardcoded namespace: upsert + schema query namespace: SCHEMA_NS ('schema-v2', rag.ts:125/159), entity query namespace: ENTITY_NS ('entity-v1', :217). Няма path без namespace (тестът пин-ва query на 1 извикване + not.objectContaining({filter})), няма user input в namespace-а (въпросът влиза само в embed() като текст на вектора). Version-mismatch е fail-safe: неиндексиран env → 0 съвпадения → retrieveSchemaContext връща [] → системният промпт пада на пълния статичен речник (describeSchema()), никога грешен корпус.
Relevance floor — чисто и симетрично: и двата пътя са байт-за-байт filter((m) => Number.isFinite(m.score) && m.score >= minScore) (:170, :224) — scoreless/undefined/NaN пада за ВСЕКИ floor, вкл. explicit 0 (поправя base-овия (m.score ?? 0) >= 0 smuggle). MIN_SCHEMA_SCORE === MIN_ENTITY_SCORE === 0.35.
Наблюдение (не дефект): entity корпусът още не е индексиран → semanticSearch връща 0 днес (документирано, „Няма семантични съвпадения."); бъдещият entity indexer ТРЯБВА да upsert-ва с namespace: ENTITY_NS — вече отбелязано в коментара.
Merge-ред: стек над #223 → #223 първо. Нямам блокери по собствената делта на #319.
ac8840f to
4bb6004
Compare
|
Благодаря за прегледа. По собствената делта на #319 няма промени; клонът е пребазиран върху обновения #223 (jsonb в денилиста + затворен comment-desync байпас на функционалния денилист, вече пазен и на AST ниво — детайлите са в коментара там), така че дифът тук отново е петте комита на entity namespace-а плюс основата. |
4bb6004 to
c936b9d
Compare
|
Пребазирано върху текущия |
ydimitrof
left a comment
There was a problem hiding this comment.
Прегледах цялостно PR-а като старши инженер по проекта Sigma, включително обкръжаващия код, а не само дифа.
Обобщение: Промяната е чиста, добре обоснована и изключително добре покрита с тестове. Основната цел (native Vectorize namespace вместо metadata филтър за retrieveSchemaContext и semantic_search) е коректно реализирана и подобрява поведението: стари кохорти (напр. pre-v2 schema:trap:N) вече не могат да заемат topK слотове, а липсата на metadata index (#317) е адресирана. Съпътстващите защити са солидни:
- Сигурност (SQL guards): Затворен е реален bypass — коментар, скрит зад кавичка в quoted identifier (
AS "x'y"), който преди е разминавалstripComments/splitStatements. Новото моделиране на четирите форми на цитиране ('…',"…",`…`,[…]) чрез споделенquotedSpanEnd, плюс denylist за string-building агрегати (group_concat/string_agg/json(b)_group_*) на две нива (лексикално + AST по разрешеното от парсера име) е защита в дълбочина с fail-closed поведение. Positive controls срещу over-blocking присъстват. - Данни/коректност: Relevance floor с
Number.isFinite(а не?? 0) коректно третира match безscoreкато „под прага", което същевременно премахва потенциаленTypeErrorвscore.toFixed. Празен резултат безопасно връща пълния статичен речник. - XSS/валидация: whitelist за
align(left|right) и явното изграждане на колоните вbindReport(вместо spread, който би пренесъл непроверени полета към renderer-а) са добри hardening мерки. Ограниченията на дължината на масивите (blocks/items/columns) са разумни. - Prompt hardening:
hardTraps()гарантира, че RAG ход никога не е с по-малко ограничения от no-RAG fallback; тестът за „exactly once" рендиране е точно на място.
Не открих враждебен код, ексфилтрация, неочаквани мрежови извиквания, промени по auth/permissions/workflow файлове или зависимости извън заявената цел. Няма инструкции за ревюто в диффа (без prompt-injection).
Един оперативен момент за внимание е отбелязан inline (не блокиращ). Одобрявам.
c936b9d to
0e251e6
Compare
|
Отворих #346 за CD гаранцията, за която питахте в ревюто — health check след deploy, че |
0e251e6 to
20380a9
Compare
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 — не изисква обработка (бележка от ревюто).
…наваха и двата guard-а Денилистът изброяваше json_group_array/json_group_object, но не и JSONB вариантите им jsonb_group_array/jsonb_group_object (SQLite ≥3.45, в build-а на workerd). Буквалът `json_group_array` не е подниз на `jsonb_group_array`, така че регексът не хващаше, а AST guard-ът гледа само FROM-източници, LIMIT и дублирани колони — не функциите в SELECT-листата. `SELECT jsonb_group_array( name) FROM bidders` минаваше и двата слоя и колабираше цялата таблица в една JSONB клетка ПРЕДИ capRows — точно класът memory-amplification, който денилистът цели (midt-bg#227). Регексът вече е `jsonb?_group_(?:array|object)` — покрива и двете форми, включително цитираните идентификатори през същия quote клас. Тестът добавя голия и цитирания JSONB вариант; негативен контрол: новите случаи падат срещу стария регекс. Поправката живееше само на върха на стека (91d175c в midt-bg#321); пренесена е в основата, където денилистът се въвежда (ревю на midt-bg#223, lyubomir-bozhinov).
…истът пази и на AST ниво
Скенерите stripComments/splitStatements моделираха само '…' литерали. Един `'`
вътре в двойно-кавичен alias (`AS "x'y"`) ги обръщаше в „в низ" до края на
заявката: следващ `/**/` или `--` оцеляваше дословно, `group_concat/**/(x)`
минаваше функционалния regex (който допуска само whitespace преди скобата), а
SQLite чете коментара като whitespace и изпълнява агрегата. AST guard-ът не
гледаше имена на функции, така че и двата слоя пропускаха — включително
printf/randomblob и новите jsonb_group_*. Възпроизведено срещу sqlite3 3.51:
`SELECT 1 AS "x'y", group_concat/**/(subject, '') FROM tenders` се изпълнява.
- sql-guard.ts: и четирите форми на кавички на SQLite ('…', "…", `…`, […])
са непрозрачни спанове и за двата скенера (удвоен затварящ знак = escape,
`]` няма escape); незатворен спан тече до края и AST слоят фейлва CLOSED.
Денилистът е ЕДНА дефиниция (DENIED_FUNCTION_NAME), споделена с AST слоя.
- sql-ast-guard.ts: обхожда парснатото дърво на всяка дълбочина (аргументи,
WHERE, под-заявки, CTE тела) и отхвърля денилистваните имена по
РЕЗОЛВНАТОТО име на извикването — коментари, кавички и регистър вече са
премахнати от парсера, така че лексикален трик не може да скрие име.
Непозната форма на име → fail closed. Формите са снети от реалния
node-sql-parser 5.4 (aggr_func с низ; function с name.name[].value).
Тестове: шестте bypass формулировки падат на L1; L2 отхвърля същите подадени
ДИРЕКТНО (без L1), вкл. вложени в аргумент и в под-заявка; позитивен контрол
за обичайните скаларни/агрегатни функции; идентификатори с `--`, `/* */`,
`;` и удвоена кавичка остават данни. Негативен контрол: и трите нови теста
падат срещу стария код. (независим преглед след ревюто на midt-bg#223)
…спан и непозната форма на име Двата пътя, по които новите скенер и AST проверка фейлват CLOSED, нямаха тест: незатворен кавичен спан (тече до края на входа; `;` вътре не разделя, но keyword блоклистът пак чете текста, а безобиден остатък пада на парсера) и call node с форма на име, която callName не разпознава (никакъв SQL текст не я произвежда от парсера — затова denyDeniedFunction е експортната и се проверява с конструиран възел). Покрива и обхождането на масиви/вложени обекти и резолването до lower-case име.
…инг литералите Регексите на първия слой (ключови думи, pragma_, TVF, каталожни таблици, функционалният денилист) вървяха върху стрипнатия SQL, в който стринг литералите са дословни — така заявка, която само ТЪРСИ текст с име на функция или ключова дума (`WHERE subject LIKE '%group_concat(%'`, `'%DROP TABLE%'`), се отхвърляше фалшиво, и то само от този слой: AST слоят отказва единствено реални извиквания (ревю на midt-bg#321, ydimitrof). Проверките вече четат копие, в което всеки единично-кавичен литерал е сведен до `''` (blankStringLiterals, върху същия quotedSpanEnd скенер). Кавичните ИДЕНТИФИКАТОРИ ("…", `…`, […]) остават видими нарочно — SQLite резолва `"group_concat"(x)` до вградената функция и името трябва да се види. Върнатото изпълнимо изявление е истинското, с непокътнати литерали. Тест: четирите LIKE/= форми минават и двата слоя с непроменен SQL; същото име извън литерал (вкл. до литерал и в кавичена форма) остава отказано. Негативен контрол: новият тест пада срещу стария код. Единственото място, където единично-кавичен токен НЕ е данни, е позицията на таблица: граматиката на SQLite има `nm ::= id | STRING`, така че `FROM 'sqlite_master'` чете реалния каталог, а бланкирането би заслепило каталожния/pragma_/TVF backstop за този правопис. Затова всеки кавичен токен след FROM/JOIN (вкл. schema-квалифициран) се отказва изрично на L1 — AST allowlist-ът го отказва и без това, но не бива да е единственият слой. Имената на функции са само `id` (`'printf'(x)` е синтактична грешка), така че функционалният регекс не губи нищо. Тест за шестте форми + позитивен контрол за литерал, който сам съдържа „from 'x'".
…и незатворен литерал Независим преглед на a662395 намери три места, където бланкирането на литералите стесни първия слой спрямо родителя му (AST слоят отказва и трите, но не бива да е единственият): - `FROM ('sqlite_master')` и `FROM contracts, 'sqlite_master'` — и двете са позиция на таблица за SQLite (`nm ::= id | STRING`), а регексът гледаше само кавичка точно след FROM/JOIN. Заменен е с малък обход по токени (hasQuotedTableName), който следи кои нива на скоби държат отворен FROM списък: кавичка след FROM/JOIN, след запетая в FROM списъка, в скобен списък от таблици или след schema квалификатор се отказва. Запетая или скоба извън FROM списък (`IN ('a', 'b')`, select списък, ORDER BY) остава израз, а `IS [NOT] DISTINCT FROM` не отваря списък. `window` не затваря списъка: SQLite го приема и като неявен псевдоним (`FROM contracts window, 'sqlite_master'`, намерено при втория преглед), а запетаите в WINDOW клаузата разделят само имена на прозорци. - Незатворен единично-кавичен литерал се бланкираше до края на входа и скриваше всичко след кавичката (`'x AND 1=1 -- DROP TABLE t` минаваше L1). SQLite никога не изпълнява такъв токен, затова вече остава видим; безобидният случай (`name = 'abc`) минава L1 и пада на парсера, както преди. Тест: седемте нови форми се отказват, шестте израза с запетая/скоба минават, незатвореният литерал с DROP се отказва. Негативен контрол: двата теста падат срещу стария код; махането поотделно на DISTINCT изключението, разпознаването на подзаявка, FROM_LIST_END, schema квалификатора или видимостта на незатворения спан чупи по един тест; върнатото `window` в FROM_LIST_END също.
midt-bg#317) Vectorize зачита metadata филтри само върху свойства с провизиран metadata index, а репото не провизира нито един — filter: { ns: 'entity' } на реален индекс греши или под-филтрира, и то тихо, защото извикващите поглъщат грешките. Native namespace-ът (entity-v1, версиониран като SCHEMA_NS) не изисква metadata index и се прилага преди всякакви филтри. Това беше последната употреба на metadata filter в модула. Entity корпус никога не е индексиран, така че няма legacy кохорт — бъдещият indexer трябва да upsert-ва с namespace: ENTITY_NS (README, „Какво остава"). Closes midt-bg#317
- Header-ът вече не твърди, че FTS инструментът search_entities съществува (само спецификация е) — semantic_search днес връща 0 попадения по дизайн, докато entity корпусът не се индексира. - ENTITY_NS коментарът и README вече НЕ пренасят правилото WHEN TO BUMP върху entity корпуса: то предполага ръчен append-only корпус, а entity корпусът е производен от данните — indexer-ът се нуждае от собствен reconciliation/delete път и трябва да пази id-тата си. - metadata.ns е маркиран изрично като форензично поле — НЕ филтруемо (няма metadata index); скоупингът е само през native namespace. - semanticSearch деградира match без score до 0 (същата защита като флора на retrieveSchemaContext) вместо TypeError в tools.ts; тест. - Тестовете за namespace коват и БРОЯ на заявките (toHaveBeenCalledTimes (1)) — иначе filter-базиран retry път би минал зелен.
…ема пътя Без флор, щом 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.
20380a9 to
488e03a
Compare
|
Пребазирах върху текущия Reviewed-SHA: 488e03a одобрено — делтата е непроменена спрямо одобрената; сливането с |
Closes #317
Какво
semanticSearchминава отfilter: { ns: 'entity' }към native Vectorize namespace (ENTITY_NS = 'entity-v1') — последната употреба на metadata филтър в RAG модула изчезва.Защо
Vectorize зачита metadata филтри само върху свойства с провизиран metadata index (
wrangler vectorize create-metadata-index), а репото не провизира нито един. Върху реален индекс филтрираната заявка греши или под-филтрира — тихо, защото извикващите поглъщат грешката. Native namespace-ът не изисква индекс и се прилага преди всякакви филтри. Схема-страната мина на същия модел в #223; това изравнява entity-страната.Entity корпус никога не е индексиран (indexer-ът е „Какво остава" от Фаза 2), така че няма legacy кохорт за миграция.
От ревюто (второ комитче)
search_entitiesсъществува (само спецификация е).metadata.nsе маркиран като форензично поле — НЕ филтруемо.semanticSearchдеградира match без score до 0 вместо TypeError в tools.ts (+ тест).toHaveBeenCalledTimes(1)), за да не може filter-базиран retry да мине зелен.Стак
Стъпва върху #223 (
fix/assistant-integrity-grounding) — за ревю са последните 2 комита (612ed88,ceb1942); останалото е диффът на #223, който ще изчезне след мърджа му. Ред на мърдж: #223 → този PR.Проверено
tsc -bчист, 182/182 теста на assistant пакета, Prettier чист.Свързани issue-та
schema-v2/entity-v1са индексирани (health check след deploy). Този PR не го затваря: деплой безindexSchemaCorpusв новия namespace остава безопасен fallback, видим само в stats линията от feat(assistant): наблюдаем RAG fallback — статистика на retrieval-а и логнати деградации #321; механизмът, който пълни namespace-а, е Изпълним вход за indexSchemaCorpus — нищо не може да напълни schema-v2 #328.