diff --git a/docs/code_review_tui.md b/docs/code_review_tui.md index 9cc2b07..1a71d5d 100644 --- a/docs/code_review_tui.md +++ b/docs/code_review_tui.md @@ -13,6 +13,66 @@ --- +## Статус исполнения + +Ревью выполнено, все приоритетные находки исправлены. Сводка по коммитам (ветка +`feature/navi-code`, коммиты без Co-Authored-By): + +| Этап | Коммит | Находки | Тестов | +|---|---|---|---| +| 1 — crash/injection/reconnect + drop artifact | `3bae766` | 4.B1, 6.B1, 3.B1, 4.B2, 5.B2, 1.B1, 2.B3, 2.B1 | +9 / −4 | +| 2 — throttle markdown + height cache | `6581963` | 2.P1, 3.P1, 2.P2 (3.P2 пропущен) | +2 | +| 3 — async REST api | `85104aa` | 5.M1, 6.P1, 4.P1, 5.M2 | (моки → async) | +| 4 — theme repaцвет + bounded history + tail purge | `d2bdf4c` | 5.B1, 4.P2, 3.P3, 1.P1 | +5 | +| 5 — minor correctness/perf | `efda6b4` | 5.B3, 6.B2, 3.B3, 2.B2, 5.P1, 2.P3 | +7 | + +**Полный pytest:** 932 → **953 passed, 1 skipped** (+21 чистый тестов). + +### Сделано (✅) + +- **Этап 1:** 4.B1 palette IndexError guard · 6.B1 `/profile` session_id · 3.B1 + todo markup escape · 4.B2 `/help` plain text · 5.B2 submit-always-enqueue · + 1.B1 stream_start resets thinking · 2.B3 todo validation robust · 2.B1 + ArtifactRenderer удалён (артефакты не нужны в TUI — файлы видны средствами ОС). +- **Этап 2:** 2.P1+3.P1 Markdown-троттл ~150 мс + кэш `get_content_height` (O(N²)→O(N), + live-форматирование сохранено, flush на stream_end) · 2.P2 `ThemedMarkdown` + yield без materialize. (3.P2 — `_sync` O(visible) пропущен: преждевременная, + microsecond-level.) +- **Этап 3:** 5.M1+6.P1 `api.py` → 7 async-функций через shared `httpx.AsyncClient` + (event loop не блокируется, connection pooling) · 5.M1 все callers `await` + (tui_app, builtin, cli) · 4.P1 `sessions_picker` load в worker · 5.M2 убрана + мёртвая `iscoroutine`-проверка. +- **Этап 4:** 5.B1 `ChatPanel.refresh_content()` + `apply_theme` (bubbles + пере-красятся) · 4.P2 `theme_picker` debounce live-preview + cancel-on-escape · + 3.P3 `ChatModel.items` bounded (cap = visible × 3) · 1.P1 `stream_end` purge + хвостом (O(tail) вместо full-copy). +- **Этап 5:** 5.B3 `session_id` `.get` guard · 6.B2 `/switch` surface original + error · 3.B3 bare-slash no-auto-pick · 2.B2 grep regex-highlight · 5.P1 + file_refs filter-before-sort · 2.P3 generic tool result truncation (200 строк). + +### Не сделано (по решению) + +- **1.B2** (failed tool resume) — ⏸️ **отложено пользователем**. `load_history` + хардкодит `success:True`, т.к. persisted `Message` не хранит `success`. Требует + правки backend persistence; в работу не брали. +- **1.P2** (bounded input-queue) — пропущен: минор, риск (drop oldest теряет + сообщения) > пользы. +- **3.B2** (`_denoise` edge при `│` в контенте) — косметика, известное + ограничение; selection в целом протестировано. +- **Maintainability** (6.M2 единый session-resolution хелпер, 1.M1 WsBridge + public-хуки, 2.M1 TodoResultRenderer структурированный meta, 1.M3/2.M2 + fallback dump, 3.M1 `ChatModel.reset`, 3.M2 `_item_msg` meta-знание, 5.M3 + double-stop, 6.M1 deprecated `get_event_loop`, 6.M3 `app._mouse_enabled`, + 4.M3 `match("")` поведение, 1.M2/1.M5/2.M3/2.M4 мелкие) — оставлены: рабочее, + без пользовательской пользы, чистота/будущестойкость. +- **3.M4** (ElapsedTimer `format_duration(None)`) — снято: `format_duration` + обрабатывает `None` безопасно (non-issue). + +Подробности по каждой находке — в этапах 1–6 ниже; статус исполнения этапов +работы — в «План исправления». + +--- + ## Этап 1 — Транспорт и модель данных (TUI) Файлы: `ws_bridge.py` (206), `chat_model.py` (280), `events.py` (39), `context.py` (37) + контракт @@ -704,6 +764,8 @@ ## Этап 1 — Критичные crash/injection + потеря данных (точечные фиксы) +**Статус:** ✅ выполнено — коммит `3bae766` (все 8 пунктов, +9/−4 теста). + **Цель:** убрать все найденные crash-edges и injection-баги + потерю сообщения и склейку thinking. Каждый фикс локальный, низкорисковый, с тестом. Быстрая победа, видимый прирост надёжности. @@ -725,6 +787,9 @@ ## Этап 2 — Перформанс стриминга markdown (тема A, ключевая) +**Статус:** ✅ выполнено — коммит `6581963` (2.P1+3.P1 троттл, 2.P2 yield; 3.P2 пропущен как +преждевременная). +2 теста. + **Цель:** убрать O(N²) по длине ответа при стриминге, **сохранив live-markdown-форматирование** (решение пользователя: троттл, не «Text-в-стриме»). @@ -748,6 +813,9 @@ ## Этап 3 — Блокирующий I/O + транспорт (тема D, системная) +**Статус:** ✅ выполнено — коммит `85104aa` (5.M1+6.P1 async api + shared AsyncClient, 5.M1 +callers await, 4.P1 sessions_picker worker, 5.M2 убрана iscoroutine). Моки в 6 тест-файлах → async. + **Цель:** TUI перестаёт «замерзать» при сетевых операциях. | id | Фикс | Файл | @@ -770,6 +838,9 @@ ## Этап 4 — UI-состояние: theme + bounded items + артефакты +**Статус:** ✅ выполнено — коммит `d2bdf4c` (5.B1 refresh_content, 4.P2 debounce, 3.P3 bounded +items, 1.P1 tail purge). 2.B1 уже удалён в Этапе 1; 1.P2 пропущен (минор). +5 тестов. + **Цель:** корректное пере-красение при theme switch, bounded-память длинных сессий, решение по артефактам. @@ -788,20 +859,24 @@ ## Этап 5 — Минорные correctness/perf + maintainability (по желанию) +**Статус:** ✅ частично — коммит `efda6b4`. Выбраны все некосметические пункты: +2.B2, 5.P1, 2.P3, 3.B3, 5.B3, 6.B2 (6 фиксов, +7 тестов). 3.B2 и maintainability +оставлены (косметика). 1.B2 отложено. + **Цель:** остаточные минорные находки. Брать выборочно по желанию пользователя; многие — косметика. **Correctness/perf (LOW):** -- 2.B2 — grep regex-highlight (компилировать pattern как regex при `args.regex=True`) -- 5.P1 — `file_refs._collect_files`: фильтровать sensitive до sort -- 2.P3 — generic `ToolResultRenderer`/`PlainRenderer` truncation большого result -- 3.B2 — `_denoise` edge при `│` в контенте (известное ограничение) -- 3.B3 — `_complete_command` на пустом `/` -- 5.B3 — `session["session_id"]` → `.get` (KeyError edge в `_resolve_session`) -- 6.B2 — `/switch` re-raise теряет оригинальную ошибку +- ✅ 2.B2 — grep regex-highlight (компилировать pattern как regex при `args.regex=True`) +- ✅ 5.P1 — `file_refs._collect_files`: фильтровать sensitive до sort +- ✅ 2.P3 — generic `ToolResultRenderer`/`PlainRenderer` truncation большого result +- ⏺ 3.B2 — `_denoise` edge при `│` в контенте (оставлено: известное ограничение) +- ✅ 3.B3 — `_complete_command` на пустом `/` +- ✅ 5.B3 — `session["session_id"]` → `.get` (KeyError edge в `_resolve_session`) +- ✅ 6.B2 — `/switch` re-raise теряет оригинальную ошибку - ~~1.B2~~ — **отложено** (см. «Решения»); в работу не берётся -**Maintainability (LOW):** +**Maintainability (LOW):** — ⏺ **оставлены** (рабочее, без пользовательской пользы): - 6.M2 — единый хелпер session-resolution (3 копии → 1) - 1.M1 — `WsBridge` public-хуки транспорта (`recv()`, `wait_connected()`), убрать приватный доступ - 2.M1 — `TodoResultRenderer`: структурированный `{done,total}` в meta вместо парсинга строки @@ -810,7 +885,7 @@ - 3.M2 — `_item_msg` meta-знание (принять как цену разделения) - 5.M3 — `WsBridge.stop` `if not None` + обнуление (double-stop cleanup) - 6.M1 — `asyncio.to_thread` вместо deprecated `get_event_loop()` -- 6.M3 / 5.M2 — приватные доступы `app._mouse_enabled`; `iscoroutine` (уже в Этапе 3) +- 6.M3 / 5.M2 — приватные доступы `app._mouse_enabled`; `iscoroutine` (5.M2 ✅ в Этапе 3) - 4.M3 — `registry.match("")` поведение (решение по 3.B3) - 1.M2 / 1.M5 / 2.M3 / 2.M4 / 3.M4(снято) — мелкие @@ -842,11 +917,17 @@ ## Порядок работы и контрольные точки -1. Этап 1 (7 точечных фиксов + тесты) → полный pytest зелёный → коммит. -2. Этап 2 (перфоманс стриминга) → pytest + визуальная проверка → коммит. -3. Этап 3 (async I/O) → pytest + визуальная проверка → коммит. -4. Этап 4 (theme + bounded + артефакты) → pytest + визуальная проверка → коммит. -5. Этап 5 (минорные, выборочно) → pytest → коммит(ы). +**Все 5 этапов выполнены** (ветка `feature/navi-code`). Каждый — с подтверждения +подхода, полным зелёным pytest, отдельным коммитом без Co-Authored-By. + +1. ✅ Этап 1 (8 точечных фиксов + тесты) → `3bae766`. +2. ✅ Этап 2 (перфоманс стриминга) → `6581963`. +3. ✅ Этап 3 (async I/O) → `85104aa`. +4. ✅ Этап 4 (theme + bounded + purge) → `d2bdf4c`. +5. ✅ Этап 5 (минорные, некосметические) → `efda6b4`. + +Итог: 932 → 953 passed (1 skipped), +21 чистый тест. Не сделано по решению: 1.B2 +(отложено, backend), косметика/maintainability (оставлены). Перед каждым этапом — подтверждение подхода (правило «обсуди до реализации»); находки из секции «Решения» — отдельно до начала соответствующего этапа. \ No newline at end of file