Skip to content

Полное код-ревью проекта: 15 приоритетных багов + подтверждённые находки (xhigh recall) #1234

Description

@axisrow

Полное код-ревью всего проекта (xhigh recall)

Процесс: 10 независимых углов поиска → 62 кандидата → дедупликация → 11 верификаторов (каждый кандидат получил вердикт CONFIRMED / PLAUSIBLE / REFUTED) → контрольный sweep по слабо покрытым зонам. Выжило 52 находки + 4 из sweep; 7 кандидатов опровергнуты верификацией.

Ветка на момент ревью: ao/tcf-orchestrator. Номера строк соответствуют состоянию рабочего дерева на дату ревью.


Топ-15 находок (по убыванию серьёзности)

1. src/web/runtime_shims.py:156 — no-op синхронизации scheduler-задач в веб-режиме

Суть: sync_search_query_jobs/sync_pipeline_jobs — no-op с ложным комментарием «worker подхватит на следующем цикле»: у воркера нет периодического re-sync, а веб-обработчики мутаций search-query/pipeline не ставят команду scheduler.reconcile.

Сценарий отказа: Дефолтный serve с работающим планировщиком: пользователь добавляет/правит/включает search-query или pipeline в веб-UI → APScheduler-задача не регистрируется (и не удаляется) до рестарта воркера или случайного reconcile от несвязанного действия (start/stop/set-interval). Статистика нового запроса молча не собирается, включённый pipeline молча не генерирует. Подтверждено: reconcile ставится только в 4 местах web/scheduler/handlers.py, в worker-цикле только read-only публикация снапшотов.

2. src/search/ai_search.py:99 — AI-поиск не работает в 100% вызовов

Суть: AISearchEngine вызывает self._agent.run(), но deepagents.create_deep_agent возвращает langgraph CompiledStateGraph без метода .run (только .invoke) — AI-поиск падает AttributeError в 100% вызовов.

Сценарий отказа: llm.enabled=true + api_key → initialize() успешен, любой AI-запрос в /search → AttributeError, перехватывается и деградирует в «AI search error: ... Showing local results». Собственный бэкенд проекта (src/agent/backends/deepagents.py:538) для этого же типа использует hasattr-guard и .invoke. Тесты маскируют баг: create_deep_agent замокан MagicMock, у которого .run есть.

3. src/agent/tools/accounts.py:360 — bool-строка обходит защиту от перезаписи сессии

Суть: import_session проверяет деструктивный флаг сырой истинностью if not args.get("force") вместо arg_bool — JSON-строка "false" истинна и обходит защиту от перезаписи.

Сценарий отказа: Бэкенд, сериализующий аргументы строками (deepagents/ReAct — коэрции нет ни на одном пути), передаёт force="false" (то есть НЕ перезаписывать) для существующего phone → отказ «аккаунт уже существует» пропускается → db.add_account делает UPSERT: рабочая сессия аккаунта перезаписана, is_premium сброшен. Хелпер arg_bool/is_affirmative существует в _registry.py и его докстринг описывает ровно этот класс бага (#1115 — bool-строка в agent-tool, аудит #837).

4. src/services/publish_service.py:186 — таймаут после фактической отправки → дубль поста

Суть: asyncio.wait_for(timeout=60) вокруг отправки в Telegram: таймаут отменяет только локальное ожидание, а уже принятый сервером запрос доставляется — цель помечается неотправленной, и ретрай публикует пост второй раз.

Сценарий отказа: Медленная загрузка медиа/сети: publish_files превышает 60с после того как MTProto-запрос ушёл на сервер → пост появляется в канале, но success=False('Timeout'), цель не записывается в metadata.published_targets → run остаётся completed+approved (retry-eligible), задача FAILED → оператор жмёт publish повторно (естественная реакция на «Timeout») → дедуп по published_targets цель не находит → дубль поста в канале.

5. src/services/notification_matcher.py:44 — ReDoS от пользовательского regex замораживает event loop

Суть: Пользовательский regex из search-query исполняется re.search по каждому сообщению без защиты от катастрофического бэктрекинга (ловится только re.error) — один паттерн замораживает event loop воркера.

Сценарий отказа: search-query add '(a+)+$' --regex + любое собранное сообщение с длинной серией 'a' не в конце строки (текст канала контролируется третьей стороной, до 4096 символов) → экспоненциальный бэктрекинг в синхронном C-вызове → heartbeat, очередь сбора и снапшоты заморожены; asyncio-таймаут не сработает — сам loop заблокирован. Усилители: 24-часовой backlog переподаётся матчеру каждый проход, dry-run гоняет всё окно.

6. src/telegram/pool_lifecycle.py:547 — exception-путь force_native выбрасывает здоровый клиент из пула

Суть: Exception-путь _acquire_from_lease при force_native=True выбрасывает здоровый пуловый клиент: direct_session принудительно None (строка 474), и except делает self.clients.pop(phone) без disconnect.

Сценарий отказа: get_native_client_by_phone (auth-флоу, agent-tools, messages read --live) при транзиентной ошибке подключения эфемерного native-клиента → из пула удаляется здоровая подключённая сессия (утечка её _send_loop/_recv_loop), телефон исчезает из _connected_phones → все последующие get_client_by_phone/get_available_client для аккаунта возвращают None до рестарта/re-add. Гарантия наличия клиента в пуле на этом пути доказана строкой 451.

7. src/runtime/worker.py:349 — DatabaseBusyError убивает standalone-воркер

Суть: Snapshot-цикл standalone-воркера ловит только CancelledError/TimeoutErrorDatabaseBusyError (2 ретрая по 0.05/0.2с, busy_timeout не выставлен) убивает весь процесс воркера.

Сценарий отказа: Split-деплой (serve --no-worker + worker): контеншн записи от веб-процесса → upsert_snapshot бросает DatabaseBusyError → исключение выходит из while, finally останавливает контейнер, процесс завершается — сбор и диспетчеризация стоят до ручного рестарта. Embedded-двойник (embedded_worker.py:182-190) guard'ит и DatabaseBusyError, и Exception — standalone-цикл скопирован без этих guard'ов.

8. src/telegram/collector_mixins/collection.py:410 — недоступность одного аккаунта останавливает всю очередь

Суть: Недоступность закреплённого за каналом аккаунта (flood/disconnect) при свободных остальных аккаунтах классифицируется как pool-wide NoActiveCollectionClientsError — очередь дренируется и воркеры останавливаются.

Сценарий отказа: Приватный канал с preferred_phone=A; A во flood-wait, B свободен → get_client_by_phone(A)=None, но availability='available' → ветка all_flooded не срабатывает → NoActiveCollectionClientsError('No active connected clients')_handle_collection_exception: задача +120с, _drain_memory_queue(), stop_workers → каналы, собираемые через B, выкидываются из очереди. DB pull loop переподхватывает через ~3с, поэтому это не вечная заморозка, а постоянный churn дренажа с ложной диагностикой каждый раз, когда канал A попадает в очередь.

9. src/cli/commands/pipeline.py:235pipeline add --json-file теряет цели и не активирует pipeline

Суть: target_refs передаются словарями, а import_json принимает только строки "phone|id" — все цели молча отбрасываются; тот же путь (строка 385) не применяет активацию, и pipeline остаётся is_active=False вопреки отсутствию --inactive.

Сценарий отказа: pipeline add NAME --json-file graph.json --target "+7900...|123" → «Added pipeline id=N», но целей 0 (фильтр isinstance(ref,str) and '|' in ref молча пропускает словари) и pipeline неактивен (import_json хардкодит is_active=False; веб-мастер компенсирует это через toggle, CLI-ветка — нет, флаг --inactive принимается и игнорируется) → генерация никогда не запускается, publish отдаёт 'No targets configured'. Ветка --node и веб не затронуты.

10. src/web/settings/handlers.py:663 — смена интервала сбора со страницы Settings не доезжает до планировщика

Суть: Сохранение интервала сбора на странице Settings пишет настройку и вызывает scheduler.update_interval() — задокументированный no-op на шиме — но не ставит scheduler.reconcile, в отличие от параллельного маршрута /scheduler set-interval.

Сценарий отказа: Дефолтный serve: пользователь меняет collect_interval_minutes на странице Settings → флеш «scheduler_saved», но работающий IntervalTrigger collect_all сохраняет старую периодичность до рестарта воркера или несвязанного действия, ставящего reconcile. Живой SchedulerManager читает настройку только в load_settings()/start().

11. src/collection_queue.py:637 — QueueFull: PENDING перезаписывается в FAILED, ретрай теряется

Суть: _try_reconnect_and_requeue при QueueFull уже перевёл задачу в PENDING, но возвращает False — вызывающий код тут же перезаписывает её в FAILED, теряя ретрай и противореча собственному логу.

Сценарий отказа: ConnectionError при сборе + заполненная in-memory очередь (500 элементов): лог обещает «stays PENDING and will be picked up by the DB pull loop», но return False → update_collection_task(task_id, FAILED); pull loop берёт только PENDING → ретрай потерян навсегда. Guard в репозитории защищает только CANCELLED, PENDING→FAILED проходит.

12. src/collection_queue.py:450 — busy-ошибка после успешного сбора помечает задачу FAILED

Суть: _handle_collection_completion делает необёрнутые DB-чтения (get_collection_task:450, get_by_pk:473) после успешного сбора — транзиентный DatabaseBusyError уводит успешную задачу в FAILED.

Сценарий отказа: collect_single_channel вернул count>0 (сообщения сохранены, last_collected_id продвинут), затем busy-ошибка на пост-чтении → исключение попадает в общий except → generic-хвост _handle_collection_exception (586-589) помечает задачу FAILED с текстом lock-ошибки — UI показывает провал полностью успешного сбора. 20 строками выше (302-305) идентичный паттерн чтения обёрнут с комментарием ровно про этот класс ошибок.

13. src/services/production_limits_service.py:344 — дневной cost cap превышается конкурентными вызовами

Суть: Дневной cost cap — check-then-record без резервирования: _lock сериализует только проверку, а расход бронируется record_cost после завершения платного вызова (10-60с) — N конкурентных генераций все проходят проверку.

Сценарий отказа: Остаток бюджета = 1 изображение; N генераций стартуют конкурентно (pipeline-запуски + agent-tool + веб) → каждая проходит check_cost_cap по пре-спенд итогу → все N платных вызовов уходят → record_cost бронирует всё постфактум → дневной cap превышен на (N-1)×стоимость, «жёсткая» гарантия фичи (#814 — лимиты продакшена) не выполняется.

14. src/services/error_recovery_service.py:149 — CircuitBreaker открывается против здорового провайдера

Суть: CircuitBreaker считает кумулятивные, а не последовательные сбои: record_success сбрасывает failure_count только в half_open — восстановленные ретраем сбои копятся и открывают предохранитель против здорового провайдера.

Сценарий отказа: Процессный ErrorRecoveryService (EmbeddingService через SearchEngine, либо ABTestingService на 6 вариантах) видит 5 транзиентных сбоев за всё время жизни, каждый тут же успешно ретраится (record_failure инкрементирует, следующий record_success в состоянии closed НЕ сбрасывает) → на 5-м предохранитель открывается → «Circuit breaker is open» окнами по 60с при полностью работающем провайдере. Один вызов с 4 ретраями до успеха уже даёт 4 счёта; комментарий на строке 461 обещает «5 consecutive provider failures».

15. src/agent/codex_backend.py:154 — упавший ход Codex стримится как успешный пустой ответ

Суть: Codex-бэкенд трактует turn/completed как безусловный успех (bare break), но SDK доставляет провал хода ВНУТРИ turn/completed (Turn.status='failed' + Turn.error) — упавший ход стримится пользователю как успешный пустой ответ.

Сценарий отказа: Ход Codex падает на середине (истёкшая авторизация, ошибка модели, краш MCP-сервера): SDK шлёт turn/completed с payload.turn.status=failed (сверено с моделью openai_codex.generated.v2_all.Turn) → chat_stream ломает цикл без проверки status и эмитит {'done': True, 'full_text': ''} → в web/TUI «успешный» пустой ответ агента без индикации ошибки, сообщение персистится как нормальный ход. Бэкенд opt-in (dev-mode override), поэтому ниже в рейтинге.


За бортом топ-15 (подтверждены, но вытеснены по серьёзности)

Корректность:

  • src/agent/tools/filters.py:200 — agent-tool hard_delete_channels всегда падает: сервис собран без channel_service (RuntimeError); при наивной починке вскроется отсутствие dev-mode гейта, который есть в CLI и веб (чинить оба дефекта вместе).
  • src/agent/tools/filters.py:60 — та же bool-ловушка bool("false") в analyze_filters (quick): строка «false» включает семплированный quick-режим вместо полного анализа.
  • src/services/telegram_command_dispatcher.py:152 — гибель telegram-command-диспетчера от любого не-busy исключения (неретраемый sqlite3.OperationalError, busy на COMMIT, битый статус/дата в строке); команды висят PENDING, воркер выглядит живым (PLAUSIBLE — заявленный триггер битого JSON гасится _parse_json).
  • src/database/repositories/generation_runs.py:106set_moderation_status без state-machine, флип publishedapproved возможен, но дубль публикации блокируется дедупом по published_targets (PLAUSIBLE); agent bulk-инструменты пропускают existence-check (косметика).
  • src/web/runtime_shims.py:134 — колонка next-run на /scheduler всегда пустая (снапшот воркера не несёт времён следующего запуска).
  • src/telegram/pool_dialogs.py:390 — naive cached_at из окна коммитов 2026-03-11..03-16 может уронить дилоги TypeError'ом при вычитании из aware-now (PLAUSIBLE, узкое окно; репозиторий парсит через naive-preserving parse_datetime вместо parse_utc_datetime).
  • src/database/repositories/accounts.py:214 (+ 8 consumer-сайтов) — репозитории гидратируют naive datetime, каждый потребитель повторяет tz-«танец» if dt.tzinfo is None: replace(tzinfo=UTC); системный долг на границе row→model (прецедент: HTTP 500 в jobs_read_model), живой инстанс класса — находка про pool_dialogs.py:390.

Производительность:

  • src/web/filter/handlers.py:41get_channels_with_counts делает GROUP BY по всей таблице messages при каждой загрузке /filter, затем Python отбрасывает нефильтрованные каналы (класс инцидента «database locked» из /search); второй вызов на 135 вообще не использует counts.
  • src/telegram/collector_mixins/collection.py:1243_check_notification_queries перечитывает всю таблицу каналов раз на канал за цикл → O(N²) выборок + Pydantic-валидаций за проход.
  • src/services/notification_matcher.py:36parse_chat_filter + сплит exclude_patterns пересчитываются на каждую пару сообщение×запрос вместо однократной предобработки запроса.
  • src/web/routes/dashboard.py:52db.get_stats() с SELECT COUNT(*) FROM messages (полный проход по миллионам строк) при каждом открытии дашборда; Сделать lazyload #756 сделал только lazy-load.
  • src/web/scheduler/context.py:450len(await get_channels(...)) материализует+валидирует все строки только ради подсчёта; есть count_channels().
  • src/web/scheduler/handlers.py:325 — N+1 чтений scheduler_job_disabled:sq_<id> по одному get_setting на запрос; есть батч get_settings_by_prefix.
  • src/web/dialogs/handlers.py:148 — cache_status делает 2N+1 запросов (count_dialogs + get_cached_at в цикле по телефонам) вместо одного GROUP BY.
  • src/web/routes/import_channels.py:57 — синхронный openpyxl.load_workbook + итерация листа в async-маршруте блокируют event loop, разделяемый с embedded-воркером; лимит 5 МБ на сжатый zip → разжатие безгранично по времени. Cheaper: asyncio.to_thread.

Чистка / упрощение:

  • src/cli/commands/photo_loader.py:414 (+23 модуля) — ~744 строки мёртвых для продакшена argparse-адаптеров run(args) (elif-лестницы до AST-глубины 15); только тесты их зовут, могут звать *_impl напрямую.
  • src/web/pipelines/handlers.py:524 — весь lifecycle стрим-генерации (create_run, set_status, обработка test(ai-gen): SSE-стриминг, RAG-контекст, ошибки провайдера (fake+live) (TDD) (#1024) #1034/fix(564): add CLI equivalents for web-only operations #737) реализован inline в web SSE-обработчике и продублирован в CLI (pipeline.py:611-700, комментарий «Mirror the web handler»); уже дрейфует (CLI передаёт limit, web нет; web оборачивает начальный set_status в try/except, CLI нет). Глубокий фикс: сервисный run_streaming_generation().
  • src/agent/tools/moderation.py:157 — 10 почти идентичных approve/reject функций на 3 поверхностях (agent/CLI/web), различаются лишь литералом 'approved'/'rejected' и текстом.
  • src/models.py:870GenerationRun.status/moderation_status — сырые str, литералы lifecycle сравниваются на 15+ сайтах, хотя StrEnum — установленный паттерн (CollectionTaskStatus); одна опечатка = молча-непопадающая ветка.
  • src/database/repositories/messages.py:65 / :102 / src/search/local_search.py:104 — три реализации нормализации date_to; две в одном файле с одинаковым именем _normalize_date_to и перевёрнутым порядком кортежа; модульная версия без прод-вызовов.
  • src/services/telegram_actions.py:232 vs src/parsers.py — разошедшиеся грамматики парсинга инвайт/публичных Telegram-ссылок (dialogs-join принимает tg://join, www.t.me, telegram.me, что channel-import классифицирует как 'unknown').
  • src/utils/search_query_chat_filter.py:175_normalize_token — третья грамматика идентификаторов; валидация username расходится с parsers._BARE_USERNAME_RE.
  • src/database/repositories/telegram_commands.py:17 (+runtime_snapshots.py:16, collection_tasks.py:76) — клоны _parse_json дублируют orjson-backed safe_json_loads_dict (различие на NaN/Infinity); collection_tasks.py даже импортирует хелпер и инлайнит дубль.
  • src/agent/tools/analytics.py:24/:28 vs src/web/routes/analytics.py:39/:33_clamp_positive и _MAX_RATING_LIMIT дублированы дословно; ~15+ inline-копий max(1, min(limit, N)) по маршрутам (31 совпадение в src/).
  • src/telegram/resolve_guard.py:229 (+инлайн 40-45), src/database/repositories/channel_ratings.py:45 — клоны ISO-парсинга дублируют try_parse_utc_datetime.
  • src/database/facade.py:917 — ~74 чистых форвардера дублируют db.repos.* доступ (задокументированный паттерн); create_collection_task объявлен 4×; db.get_setting — 53 сайта.
  • src/cli/commands/dialogs.py:965, src/cli/commands/pipeline.py:1308 — Namespace-мост восстанавливает argparse.Namespace для _dispatch/getattr, хотя argparse убран (CLI argparse→Typer: Финал — удалить argparse-каркас + leaf-coverage на Typer + parity/manifest зелёные #1125); хендлеры могли бы брать типизированные kwargs.
  • src/database/repositories/messages.py:1569get_message_by_id — точный дубль get_by_id.
  • src/utils/json.py:23ensure_ascii — задокументированный no-op параметр (orjson не экранирует не-ASCII); 35 сайтов передают =False, 0 — True.
  • src/agent/tools/dialogs.py:179 (+~25 tools) — 4-шаговая gate-преамбула (require_live_runtime / resolve_phone / require_phone_permission + одинаковый except) повторяется дословно (~250 строк).
  • src/agent/tools/messaging_chat_state.py:103 — twins archive/unarchive на 4 слоях (отличие только folder_id 1|0 и глаголы); аналогично leave/delete в telegram_actions.py:742.
  • src/agent/tools/dialogs.py:112 — ad-hoc срез -100-префикса расходится с каноническим parsers.bare_channel_id (низкая серьёзность: правильная 13-значная форма режется верно, сбой — молчаливый read-only no-match для legacy коротких id).

Altitude / конвенции:

  • src/web/routes/channel_renames.py:52 — review переименований каналов (list/count/filter/keep) есть только в вебе → нарушает правило CLAUDE.md «CLI/Web parity: every web operation must have a CLI equivalent and vice versa».
  • src/web/routes/jobs.py:46 — унифицированный /jobs дашборд (JobsReadModel) есть только в вебе → то же нарушение паритета.
  • src/web/templates/scheduler.html:90dryRunNotifications() делает fetch POST → r.text()innerHTML (свап HTML-фрагмента), что политика CLAUDE.md резервирует за HTMX («reject fetch() where HTMX fits»).
  • src/web/panel_auth.py:13PANEL_USERNAME = os.environ.get("WEB_USER", "admin") + middleware принимает ЛЮБОЙ username при совпадении пароля, вопреки CLAUDE.md «username hardcoded as "admin"» (дрейф доки, плюс инвалидация cookie при смене WEB_USER).
  • pyproject.toml:153 — ruff select содержит B, DTZ сверх задокументированных в CLAUDE.md E/F/I/N/W (дрейф доки; локально «чистый по доке» код падает в CI).
  • src/services/account_availability.py обход — /accounts/flood-status (web) и account flood-status (CLI) выводят flood-статус inline мимо compute_account_availability (заявлен «единый источник истины» Agent must check Telegram account availability before reporting reconnect problems #529); CLI показывает resolve-backoff и «OK (expired)», web — нет.
  • src/web/search/handlers.py:55 — импорт приватного _is_worker_alive из src.web.routes.scheduler вместо сервисного evaluate_worker_heartbeat.
  • src/services/image_generation_service.py:252search_models() хардкодит if provider == "replicate"/"huggingface" внутри общего сервиса вместо spec-таблицы (в коде помечено как осознанное решение — низкий приоритет).

Опровергнуто верификацией (7)

Зафиксировано, чтобы не переоткрывать:

  1. pool_lifecycle.py:461 / account_lease_pool.py — «race двойной exclusive-аренды»: REFUTED — оба check-then-add участка синхронные (без await между проверкой и добавлением), на одном event loop интерливинг невозможен; окно fix(pool): two-phase release_client race (shared+exclusive на одной session) #1181 было на release-пути и уже закрыто.
  2. content_pipelines.py:339 / :353 — «обход write-lock в _replace_sources/targets_no_commit»: REFUTED как латентное — все текущие вызовы передают транзакционный conn; conn=None не используется. Хазард дизайна дефолтного аргумента, не активный баг.
  3. runtime/worker.py:195 — «снапшот коллектора хардкодит state='healthy'»: REFUTED — веб-панель считает all_flooded независимым путём из БД (_classify_health_state по flood_wait_until), retry-хинты тоже добираются из БД.
  4. channel_collection.py:186 — «дубль stats-команд из-за мёртвого is_stats_running»: REFUTEDTelegramCommandService.enqueue дедуплицирует по type+payload (PENDING/RUNNING) + сериальный диспетчер; теряется только парити текста ошибки.
  5. resolve_guard.py:229 — «'Z'-суффикс в backoff роняет парсинг»: REFUTED — Python 3.11 fromisoformat сам принимает 'Z', а писатели генерируют только +00:00.
  6. collector_mixins/stats.py:304 — «set_channel_active с channel.id=None молча no-op»: REFUTED — на всех реальных путях Channel грузится из БД, id = PRIMARY KEY, не бывает None.
  7. scheduler/service.py:310 — warm-task держится только локальной переменной: PLAUSIBLE, но не подтверждено — нарушение задокументированного контракта asyncio (RUF006-класс), но во всех await-точках задача укоренена внешне (таймеры loop), реальный mid-flight GC не продемонстрирован.

Отчёт сгенерирован автоматическим многоагентным код-ревью (10 углов поиска + 11 верификаторов + sweep). Каждая находка топ-15 и «за бортом» прошла независимую верификацию с конкретным триггером.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions