fix(ai): show live reindex progress so the embeddings counter resets to 0 and climbs #242
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#242
Reference in New Issue
Block a user
Delete Branch "fix/embeddings-reindex-progress"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Баг: счётчик «Indexed N of N» не обнуляется при Reindex
Симптом (репорт владельца со скриншотом): в Workspace → AI → карточка Embeddings/Semantic search футер «Indexed 478 of 478 pages» + кнопка «Reindex now». При нажатии реиндекс идёт, но счётчик НЕ сбрасывается в 0 и не растёт с нуля — висит «478 of 478», следить за прогрессом нельзя.
Корень
Статус, который поллит клиент, отдаёт
indexedPages = pageEmbeddingRepo.countIndexedPages()— число РАЗЛИЧНЫХ неудалённых страниц, у которых есть ≥1 строка эмбеддинга. Реиндекс (BullMQ-джобаWORKSPACE_CREATE_EMBEDDINGS→EmbeddingIndexerService.reindexWorkspace) идёт по страницам и для каждой делает HARD-replace (delete+insert) в СВОЕЙ маленькой транзакции. Поэтому в любой момент почти у всех страниц строки на месте → distinct-count держится ≈ total всю пробежку. Клиент при этом УЖЕ корректно поллит каждые 3с и останавливается при indexed≥total — проблема чисто серверная: статус никогда не отдаёт число ниже total.Фикс — отдавать ЖИВОЙ прогресс пробежки (0→total)
EmbeddingReindexProgressServiceповерх Redis (переиспользован стандартныйRedisService/ioredis, что уже бэкит BullMQ и лимитеры — нового конфига нет). HASHai:reindex:progress:<workspaceId>{total,done,startedAt}, TTL 1ч,doneчерез атомарныйHINCRBY. Всё best-effort: на ошибке Redis деградирует к старому DB-счётчику.AiSettingsService.reindex()сидит{total: countEmbeddablePages, done:0}ДО enqueue → первый поллинг сразу показывает 0.reindexWorkspaceв try/finally:start(total)на старте,incrementпосле каждой обработанной страницы,clearв finally (краш/abort/несконфигурировано не оставляют залипший статус). Per-page реиндекс (правка одной страницы), delete-cap/abort — не тронуты. Массового up-front удаления НЕТ (поиск не ломается).indexedPages=done,totalPages=total(+reindexing:true); иначе — прежнее поведение (countIndexedPages/countEmbeddablePages). Когда done==total клиент останавливается, finally чистит запись, дальше fallback к DB-счёту (== total) — стабильный вид сохранён.Тесты
embedding-indexer.service+ai-settings.service— 2 suites / 17 tests pass: total на старте, инкремент per-page, clear в finally (успех / fatal-abort без инкремента / unconfigured early-return); getMasked отдаёт прогресс при активной записи и fallback при отсутствии. server tsc + client tsc чисто.Closes #— (баг семантик-серча на develop)
🤖 Generated with Claude Code
The "Indexed X of Y pages" counter stayed stuck at "478 of 478" during a manual "Reindex now" run instead of resetting to 0 and climbing. The status reports indexedPages = countIndexedPages (DISTINCT pages with >=1 embedding row), but reindex hard-replaces each page in its OWN small transaction, so nearly all pages always have rows -> the count never drops. Add a per-workspace live reindex-progress record in Redis (reusing the existing global ioredis client via RedisService, no new Redis config): - EmbeddingReindexProgressService: start/increment/clear/get over a Redis hash with a 1h TTL self-clean; all best-effort/cosmetic so a Redis failure degrades to the existing DB-count behavior. - AiSettingsService.reindex seeds {total, done:0, startedAt} at enqueue time so the very first poll already reports done=0. - EmbeddingIndexerService.reindexWorkspace overwrites total with the real page count at start, increments done per processed page (success or handled failure), and clears the record in a finally (covers success, fatal abort, and the unconfigured early-return) so a failed run never sticks. - AiSettingsService.getMasked returns the live run numbers when a progress record is active (plus an optional reindexing flag), else falls back to countIndexedPages/countEmbeddablePages. Per-page edits (reindexPage) never touch the workspace progress record, and no mass up-front delete is introduced (search availability preserved). Tests: indexer sets/increments/clears progress (incl. fatal abort and unconfigured early-return); status reports run progress when active and falls back when not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Make the "Indexed N of N" counter update near-realtime during a reindex by tracking the server's active-run state instead of a pure time window: - Set REINDEX_POLL_INTERVAL to 5000ms (kept bounded by the cap). - Extract two pure, exported, unit-tested helpers: - nextReindexPollInterval: keep polling while the server reports an ACTIVE run (reindexing===true) OR within the deadline and not yet done; stop once the run is finished AND fully indexed (reindexing===false && indexed>=total) or the deadline cap is hit (the cap always wins, so a stuck/never-clearing progress record can't poll forever). - isReindexComplete: deadline-clear predicate mirroring that stop condition. - Wire the refetchInterval and the deadline-clearing effect to those helpers. - Keep the Reindex button spinner active for the whole run (loading also while settings.reindexing), reusing the existing loading prop; also blocks a redundant mid-run re-trigger (server de-dupes regardless). No SSE/websockets: polling keyed on the reindexing flag is the intended scope. The counter now tracks the actual active-reindex state and stops promptly when the server reports the run is done. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Code review — живой прогресс реиндексации эмбеддингов
Вердикт: Request changes. Фикс по сути рабочий — счётчик действительно начинает расти с нуля, поллинг останавливается корректно, конструкторная DI-разводка и
try/finallyне ломают существующее поведение. Но есть один блокирующий дефект: добавленный комментарий внутренне противоречит коду (описывает поведение, которого нет), а корень этого — два разных источникаtotal, из-за чего знаменатель счётчика во время прогона «прыгает». Критичных проблем (outage / потеря данных / эксплойт) нет, жёсткой блокировки мерджа не требуется.Область ревью: дифф PR #242 (база
developc5109aa, головаfix/embeddings-reindex-progress21cc2e9), 10 файлов, сервер + клиент. Security, conventions, regressions, simplification и architecture — чисто (LGTM).🔴 Must fix before merge
[documentation/stability] Привести
totalв воркере к тому же источнику, что и сид/статус — иначе знаменатель счётчика прыгает, а комментарий лжёт —apps/server/src/integrations/ai/ai-settings.service.ts:105-112(+ воркер вapps/server/src/core/ai-chat/embedding/embedding-indexer.service.ts).Сид при enqueue считает
totalчерезpageRepo.countEmbeddablePages(только не-удалённые страницы с текстом ИЛИ уже имеющимся эмбеддингом), а воркер перезатираетtotalчерезpageRepo.getIdsByWorkspace().length(ВСЕ не-удалённые страницы, без фильтра по тексту). Для любого воркспейса, где есть пустые/безтекстовые/ещё-не-проиндексированные страницы,getIdsByWorkspace().length > countEmbeddablePages. В итоге знаменатель, который видит UI, идёт478 → 500 → 478(сид → активный прогон → откат к DB-счёту послеclear), что прямо подрывает заявленную цель «плавный рост 0 → total». При этом комментарий в том же блоке утверждает и «The worker overwritestotalwith the real page count», и «the counter denominator matches» — это взаимно исключающие утверждения.Fix: сделать так, чтобы воркер и считал, и итерировал тот же набор, что
countEmbeddablePages(либо везде использовать один источникtotal), чтобы живой и стационарный знаменатели совпадали; как минимум — исправить комментарий, убрав ложное «the counter denominator matches».⚠️ При правке: если просто поменять источник
totalв воркере, не меняя того, что он итерирует (getIdsByWorkspace), тоdoneпревыситtotal(счётчик уйдёт за 100%) — менять надо согласованно и источник счёта, и набор итерации.[warning][stability] Не сбрасывать
doneв 0 при повторном запуске реиндекса на ходу —apps/server/src/integrations/ai/ai-settings.service.ts:108-117.reindex()безусловно зовётreindexProgress.start(workspaceId, total)(HSETdone='0') перед enqueue, но если воркер уже работает,add()дедуплицирует активную джобу — второй воркер не стартует. Итог повторного клика / второго админа / второй вкладки: видимый счётчик отбрасывается на0 / total, пока живой воркер продолжает инкрементить с нуля, и до конца прогона прогресс занижен. Новый guard «кнопка крутится, покаreindexing === true» гасит только одиночный дабл-клик, но не мульти-таб / мульти-админ.Fix: сеять прогресс только когда активной записи нет (
reindexProgress.get()вернулnull), либо отдать сбросdoneисключительно воркеру (start()на старте прогона), а не пути enqueue.[suggestion][stability] TTL прогресса обновляется только в
increment()—apps/server/src/integrations/ai/embedding-reindex-progress.service.ts:50-52.1-часовой TTL рефрешится только при обработке страницы. Если один embedding-вызов зависнет дольше TTL (предусловие маловероятное — обычно секунды), запись истечёт на ходу:
getMaskedвернётreindexing:false, клиент перестанет поллить, а воркер ещё реально «висит» на странице. Риск низкий; помечаю как замечание о свойстве дизайна «TTL привязан к прогрессу».🧪 Test coverage
Покрытие основной логики добротное:
reindexWorkspace(старт-total, инкремент per-page,clearвfinallyна успехе / fatal-abort / non-fatal / unconfigured),getMasked(живой прогресс vs DB-fallback) и клиентские чистые функции — все с осмысленными ассертами. Пробелы:[warning] Нет прямого юнит-теста для нового
EmbeddingReindexProgressService—apps/server/src/integrations/ai/embedding-reindex-progress.service.ts. Сервис исполняется только черезjest.fn()-моки в чужих сьютах, его собственная логика не выполняется под тестом. Непокрыты веткиget()(нет ключа /total === undefined→null;Number()-коэрсия; не-числовойtotal/done→null; не-конечныйstartedAt→0) и контракт деградации (любой Redis-throw глотается,get()→null).Fix: добавить
embedding-reindex-progress.service.spec.tsс фейковым ioredis: валидный хэш →ReindexProgress; пустой / безtotal→null; не-числовойtotal→null; не-конечныйstartedAt→0;hgetallбросает →null;start/incrementшлютhset/hincrby+expireи глотают исключение.[warning] Не протестирован сид прогресса в
reindex()—apps/server/src/integrations/ai/ai-settings.service.ts:105-112. Это несущая часть фикса («первый поллинг сразу показывает 0»); если вызовcountEmbeddablePages → start()уберут или переставят послеaiQueue.add, тест не упадёт.Fix: тест на
service.reindex(WORKSPACE_ID), проверяющий, чтоreindexProgress.startвызван с(WORKSPACE_ID, <count>)и ДОaiQueue.add(порядок вызовов).[suggestion] Тест живого прогресса
getMaskedне различает источникtotalPages—apps/server/src/integrations/ai/ai-settings.service.spec.ts:633-647. В кейсе live-прогрессаprogress.total(478) совпадает с DB-счётом (478), поэтомуexpect(totalPages).toBe(478)проходит при любой ветке тернарникаprogress ? progress.total : totalPages.Fix: задать
progress.total = 500при DB-счёте 478 и проверитьtotalPages === 500.Code review — живой прогресс реиндексации embeddings
Вердикт: Request changes (запросить правки). Изменение качественное, аккуратное и хорошо покрыто тестами — серьёзных дефектов нет. Единственный must-fix формальный, но обязательный: смена вызова в
reindexWorkspaceоставила методgetIdsByWorkspaceбез вызывающих, а его docstring теперь противоречит коду (мёртвый код + рассинхрон «код vs документация», внесённые этим PR). Правка тривиальна.Ревью прогнано по 8 аспектам (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture); ключевые утверждения перепроверены по исходникам PR-головы
d2a6fcc7.Must fix before merge
[conventions/documentation] Удалить осиротевший
getIdsByWorkspace(или хотя бы исправить его docstring) —apps/server/src/database/repos/page/page.repo.ts:271-283PR переключил единственного потребителя метода —
reindexWorkspace— на новыйgetEmbeddablePageIds(embedding-indexer.service.ts:225). На PR-голове уgetIdsByWorkspaceне осталось ни одного вызова, при этом его docstring всё ещё гласит «Used by the RAG bulk reindex to (re)build embeddings for every existing page» — что прямо противоречит коду, а соседнийgetEmbeddablePageIdsтоже заявляет «The bulk reindex iterates THIS set». Два docstring'а претендуют быть источником страниц для реиндекса, но истинен только один.Fix: удалить метод
getIdsByWorkspaceцеликом; если он намеренно сохраняется как публичное API репозитория — убрать из docstring ложное упоминание про bulk reindex.[warning][stability] Очищать только что засеянную запись прогресса, если
aiQueue.add()бросает исключение —apps/server/src/integrations/ai/ai-settings.service.ts(блок seed перед enqueue вreindex())reindex()сидит запись прогресса (start(workspaceId, total)) доaiQueue.add(), а очищается она только вfinallyворкераreindexWorkspace(). Еслиadd()упадёт после успешногоstart()(реалистичный транзиент: моргнул Redis, закрылась очередь при shutdown), воркер не запустится,clear()не выполнится, и засеянная запись проживёт весь TTL (1 ч): всё это времяgetMasked()отдаётreindexing:trueи «0 of N», кнопка Reindex висит вloading. Само-восстанавливается через TTL, данные не портятся — поэтому non-blocking.start()/clear()глотают ошибки Redis сами, так что единственный бросающий вызов здесь —add().Fix: обернуть seed+enqueue так, чтобы при ошибке
add()вызватьawait this.reindexProgress.clear(workspaceId)перед ре-throw (чистя только если seed сделал именно этот вызов, чтобы не затереть параллельный активный прогон).Test coverage
Юнит-логика покрыта основательно:
EmbeddingReindexProgressService(get валидный/пустой/частичный/нечисловой/ошибка, start/increment/clear + глотание ошибок),reindex()seed (порядок seed-до-enqueue и отсутствие ре-seed при активном прогоне),getMasked()(живой прогресс vs DB-fallback),reindexWorkspace()(start/increment×N/clear, очистка при fatal-abort и при unconfigured-return), клиентские чистые функцииnextReindexPollInterval/isReindexComplete(все ветки).Один реальный пробел:
getEmbeddablePageIdsи его «lockstep» сcountEmbeddablePages—apps/server/src/database/repos/page/page.repo.ts:297-320Этот запрос — смысловой стержень фикса: его WHERE (
p.text_content ~ '[^[:space:]]'OR EXISTS не-удалённой строкиpageEmbeddings, плюсdeletedAt is null) обязан возвращать ровно то множество, которое считаетcountEmbeddablePages— иначе живой счётчик и стационарный знаменатель снова разойдутся (тот самый баг «478 of 478»). Но во всех тестах метод замокан (getEmbeddablePageIds: jest.fn().mockResolvedValue([...])), так что ни регистр пробелов, ни ветка OR-EXISTS (страница без текста, но со старыми embeddings), ни сам инвариант равенства мощностей не проверяются — дрейф предикатов пройдёт сборку «зелёным». В проекте уже есть дешёвый паттерн (*.int-spec.tsна реальном Postgres черезgetTestDb()).Fix: int-тест, засевающий воркспейс (страница с текстом; с пустым/пробельным текстом; без текста, но с живой строкой embeddings; удалённая; только с soft-deleted embeddings) и проверяющий точный набор id плюс инвариант
getEmbeddablePageIds(ws).length === countEmbeddablePages(ws).Architecture & design
Реестр прогресса: свой Redis-store vs существующий паттерн
fileTasks/ нативный прогресс BullMQ. Это уже третий механизм отдачи состояния фоновой задачи: BullMQ-воркер (ноjob.updateProgressнигде не используется), DB-таблицаfileTasksс опросным контроллером (ZIP-импорт, PDF-экспорт), и новый bespoke Redis-хэш. Для данного случая bespoke-store оправдан (читатель — другой процесс, чтение поworkspaceId, данные косметические/эфемерные, TTL сам чистит).Дублирование SQL-предиката
getEmbeddablePageIds↔countEmbeddablePages(page.repo.ts). Оба метода держат байт-в-байт одинаковый WHERE, синхронизируемый только комментарием «MUST stay in lockstep». Сейчас идентичны (проверено), но правка одного предиката в будущем молча вернёт баг рассинхрона знаменателясделать общий билдер запроса* (s): извлечь приватный
embeddablePagesQuery(workspaceId), из которого один метод делает SELECT id, другой COUNT — инвариант становится структурным.d2a6fcc752tobf09eec4e1F4: extract the reindex button `loading` predicate into a pure, unit-tested `isReindexButtonLoading({ mutationPending, deadline, status })` next to the other reindex helpers, replacing the inline JSX expression. Covers the load-bearing post-cap case (deadline nulled, reindexing stale-true -> not loading) plus mutationPending, active-run, and finished cases. F5: rewrite the `useAiSettingsQuery` poll comment to match the actual `nextReindexPollInterval` stop condition (continues while reindexing===true OR within deadline and not fully indexed; stops only when reindexing===false && indexed>=total, or the deadline cap) instead of the stale "until indexed===total". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>F10 [suggestion]
apps/server/src/integrations/ai/ai-settings.service.ts:117-128— узкая гонка seed/dedup может оставить залипшее «reindexing: 0 of N» на весь TTL (1ч). Сид прогресса гейтится поreindexProgress.get()===null, а дедуп запуска — по существованию BullMQ-джобы (jobId). Это два разных источника истины, расходящиеся на хвосте предыдущего прогона: воркерский finally вызывает clear() ДО того, как removeOnComplete удалит джобу. Если triggerReindex попадает в это окно (двойной клик / второй админ-вкладка / авто-триггер при включении AI Search, совпавший с завершением), то get() уже null → seeded=true (ставится 0 of N), но aiQueue.add дедупится против завершающейся джобы → новый воркер НЕ стартует → сид никто не инкрементит и не чистит → статус отдаёт reindexing:true, indexed:0 до истечения TTL (1ч) или следующего ручного клика. Косметика, самоустраняется, потому suggestion/low.Fix: сделать пред-сид (в triggerReindex, до старта воркера) короткоживущим — добавить параметр ttlSeconds в start() и вызывать из triggerReindex с коротким TTL (30-60с); воркерский start() в начале реального прогона перезапишет запись и поднимет TTL до полного. Если воркер не стартовал (дедуп), фантомная запись истечёт за секунды. Альтернатива — сидить только ПОСЛЕ подтверждённого add() новой джобы.
Ревью
bdc033e68— переревью ПОЛНЫМИ 8 аспектами (отдельный субагент на каждый). Вердикт: CHANGES.Раскладка: security / test-coverage / conventions / architecture — LGTM. stability — новая F10. regressions — F6 (открыта). documentation — F7 (открыта). simplification — F8, F9 (открыты).
Открыто:
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.