vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

fix(ai-chat): reset chat on New chat during the first turn's stream (#161) #162

Closed
Ghost wants to merge 3 commits from fix/ai-chat-new-chat-during-stream into develop
pull from: fix/ai-chat-new-chat-during-stream
merge into: vvzvlad:develop
Conversation 3 Commits 3 Files Changed 6 +260 -18
Ghost commented 2026-06-24 14:43:39 +03:00
Copy Link

Closes #161.

Проблема

«New chat» во время стрима первого хода ещё не усыновлённого нового чата лишь убирал бейдж роли — сам чат/сессия/стрим оставались. Перемонтирование треда управляется render-фазовым реконсилером в useChatSession, который срабатывает только при activeChatId !== thread.chatId. На новом (ещё не усыновлённом) чате оба null, поэтому setActiveChatId(null) в startNewChat — no-op, и remount не происходит.

Решение

  1. startFreshThread() в useChatSession — безусловно диспатчит reconcile на свежий mount-key, так что ChatThread перемонтируется даже когда activeChatId остаётся null. startNewChat теперь его вызывает. После диспатча thread.chatId === null === activeChatId, поэтому реконсилер не делает второй remount.
  2. Guard от «переусыновления» брошенного треда. useChat (ai@6) не прерывает запрос при unmount и проксирует колбэки, поэтому onFinish/onError покинутого треда срабатывают позже и через onTurnFinished усыновили бы только что покинутый чат, выдёргивая пользователя обратно. Теперь onTurnFinished(serverChatId?, finishingThreadKey?): если ключ завершившегося треда не совпадает с текущим смонтированным (threadKeyRef), делается только onInvalidateChatList() — без adoption, без арма fallback, без per-chat инвалидации сообщений. ChatThread прокидывает свой threadKey в обоих колбэках. Отсутствующий ключ (старые вызовы/тесты) трактуется как текущий тред — поведение #137 сохранено.

Тесты

3 новых кейса в use-chat-session.test.tsx: remount при activeChatId===null; поздний finish брошенного треда не усыновляет и не инвалидирует сообщения (но обновляет список); finish текущего треда (совпадающий ключ) усыновляет. Весь набор ai-chat зелёный (183), tsc чисто.

Границы

Как и в исходном дизайне issue, правка сознательно не прерывает фоновый стрим брошенного хода на сервере (консистентно с поведением selectChat). Реальный abort при unmount — отдельная задача.

🤖 Generated with Claude Code

Closes #161. ## Проблема «New chat» во время стрима первого хода ещё не усыновлённого нового чата лишь убирал бейдж роли — сам чат/сессия/стрим оставались. Перемонтирование треда управляется render-фазовым реконсилером в `useChatSession`, который срабатывает только при `activeChatId !== thread.chatId`. На новом (ещё не усыновлённом) чате оба `null`, поэтому `setActiveChatId(null)` в `startNewChat` — no-op, и remount не происходит. ## Решение 1. **`startFreshThread()`** в `useChatSession` — безусловно диспатчит `reconcile` на свежий mount-key, так что `ChatThread` перемонтируется даже когда `activeChatId` остаётся `null`. `startNewChat` теперь его вызывает. После диспатча `thread.chatId === null === activeChatId`, поэтому реконсилер не делает второй remount. 2. **Guard от «переусыновления» брошенного треда.** `useChat` (ai@6) не прерывает запрос при unmount и проксирует колбэки, поэтому `onFinish`/`onError` покинутого треда срабатывают позже и через `onTurnFinished` усыновили бы только что покинутый чат, выдёргивая пользователя обратно. Теперь `onTurnFinished(serverChatId?, finishingThreadKey?)`: если ключ завершившегося треда не совпадает с текущим смонтированным (`threadKeyRef`), делается только `onInvalidateChatList()` — без adoption, без арма fallback, без per-chat инвалидации сообщений. `ChatThread` прокидывает свой `threadKey` в обоих колбэках. Отсутствующий ключ (старые вызовы/тесты) трактуется как текущий тред — поведение #137 сохранено. ## Тесты 3 новых кейса в `use-chat-session.test.tsx`: remount при `activeChatId===null`; поздний finish брошенного треда не усыновляет и не инвалидирует сообщения (но обновляет список); finish текущего треда (совпадающий ключ) усыновляет. Весь набор ai-chat зелёный (183), `tsc` чисто. ## Границы Как и в исходном дизайне issue, правка сознательно **не** прерывает фоновый стрим брошенного хода на сервере (консистентно с поведением `selectChat`). Реальный abort при unmount — отдельная задача. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ghost added 1 commit 2026-06-24 14:43:40 +03:00
fix(ai-chat): reset chat on New chat during the first turn's stream (#161) cee231b136 
Pressing "New chat" while a brand-new chat's first turn was still streaming
only removed the role badge — the thread, session and stream were kept. The
thread remount is driven by a render-phase reconciler in useChatSession that
fires only when activeChatId !== thread.chatId; on a not-yet-adopted new chat
both are null, so startNewChat's setActiveChatId(null) was a no-op and nothing
remounted.

- useChatSession: add startFreshThread(), which unconditionally dispatches a
  reconcile to a fresh mount key so ChatThread remounts even when activeChatId
  stays null. startNewChat now calls it.
- Guard against the abandoned thread's late finish: ai@6's useChat does not
  abort on unmount and proxies its callbacks, so onFinish/onError of the chat
  the user just left still fire and would adopt it back. onTurnFinished now
  takes the finishing thread's key and, when it no longer matches the mounted
  thread, only refreshes the chat list — no adoption, no fallback arming, no
  per-chat message invalidation. ChatThread forwards its threadKey in both
  onFinish and onError. An omitted key (legacy callers/tests) counts as the
  current thread, preserving the #137 adoption behavior.

Tests: 3 new useChatSession cases (fresh-thread remount with activeChatId null,
abandoned-thread finish rejected, current-thread finish still adopts). Full
ai-chat suite green (183).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vvzvlad added the bug label 2026-06-24 20:48:45 +03:00
vvzvlad commented 2026-06-25 22:12:59 +03:00
Owner
Copy Link

Code review — PR #162: reset chat on «New chat» during the first turn's stream (#161)

Вердикт: Request changes — сама реализация корректна и хорошо покрыта тестами (6 из 7 аспектов LGTM), но PR не вливается в текущий develop и требует ребейза; при ребейзе нужно перепроверить стык с #174. Это единственный блокер; логика фикса — approve-уровня.

Ревью сделано против фактического HEAD ветки (cee231b). Ветка ответвилась от develop до фичи #174 (onServerChatId — ранняя адопция chatId на чанке start), поэтому в diff Gitea её не видно и MERGEABLE: false. Запускались 7 специализированных ревьюеров (security пропущен — в диффе нет trust boundary / секретов / парсинга недоверенного ввода, чистый клиентский React-стейт).

Must fix before merge

  • [integration] Сделать rebase ветки на текущий develop и переразрешить конфликты. Файлы ai-chat-window.tsx, chat-thread.tsx, hooks/use-chat-session.ts, hooks/use-chat-session.test.tsx изменены и в #174 (уже в develop), и в этом PR — поэтому PR сейчас не мержится. При ребейзе:

    • сохранить в возвращаемом объекте useChatSession и onServerChatId (из #174), и новый startFreshThread;
    • убедиться, что новый guard (threadKeyRef / finishingThreadKey) корректно сочетается с ранней адопцией #174.
    • Проверено заранее, чтобы снять тревогу: сочетается. adopt сохраняет тот же mount-key, поэтому threadKeyRef.current === thread.key остаётся валидным после ранней адопции, а startFreshThread по-прежнему нужен для окна до чанка start (пока activeChatId === null). Дополнительный guard на onServerChatId не требуется: он вызывается из useEffect([messages]), а эффекты размонтированного треда не срабатывают — late-fire ему не грозит (в отличие от onFinish/onError, которые useChat проксирует и после unmount).

  • [suggestion][simplification] Убрать ставший избыточным cancelPendingAdoption() в startNewChat — ai-chat-window.tsx. Идущий сразу следом startFreshThread() уже выполняет pendingNewChatRef.current = null, и между двумя вызовами ничего от disarm не зависит. Сам экспорт cancelPendingAdoption оставить — он по-прежнему нужен в selectChat. На корректность не влияет; можно оставить ради симметрии startNewChat/selectChat.

Test coverage

Все новые ветки покрыты осмысленными ассертами (прогон vitest из клиентского конфига: 13/13 зелёные):

  • startFreshThread() — remount при activeChatId === null (mount-key меняется);
  • abandoned-ветка onTurnFinished — нет adopt, нет per-chat инвалидации, но список инвалидируется;
  • current-ветка (совпадающий ключ) — adopt происходит.

Ветка finishingThreadKey === undefined (legacy/тесты) покрыта существующими тестами без отдельного именованного кейса — достаточно. Компонентных тестов на ai-chat-window/chat-thread нет, но их в проекте и не было: хук — установленный unit-seam, это не пробел. Регресс-замок на #161 присутствует (тесты падали бы на до-фиксовом коде).

Architecture & design (forward-looking, non-blocking)

Консолидировать разрозненные guard'ы «этот late-callback — от смонтированного треда?» в один seam. Модуль накапливает ad-hoc защиты от late-callback'ов размонтированных тредов: pendingNewChatRef + cancelPendingAdoption, зеркало activeChatIdRef, и теперь threadKeyRef + параметр finishingThreadKey. Корень один — useChat (ai@6) не abort'ит запрос на unmount и проксирует onFinish/onError.

один предикат isCurrentThread(key), через который проходят все мутирующие identity-колбэки.* Плюсы: корень назван один раз, новые колбэки получают unmount-safety бесплатно, меньше концептуальная поверхность самого баговатого места модуля. Минусы: рефактор со своими тестами; делать после ребейза. Усилия: medium.

Aspects: stability ✅, regressions ✅, conventions ✅, documentation ✅, test-coverage ✅, simplification (1 nit), architecture (1 proposal). Security — N/A.

## Code review — PR #162: reset chat on «New chat» during the first turn's stream (#161) **Вердикт: Request changes** — сама реализация корректна и хорошо покрыта тестами (6 из 7 аспектов LGTM), но PR **не вливается в текущий `develop`** и требует ребейза; при ребейзе нужно перепроверить стык с #174. Это единственный блокер; логика фикса — approve-уровня. > Ревью сделано против фактического HEAD ветки (`cee231b`). Ветка ответвилась от `develop` **до** фичи #174 (`onServerChatId` — ранняя адопция `chatId` на чанке `start`), поэтому в diff Gitea её не видно и `MERGEABLE: false`. Запускались 7 специализированных ревьюеров (security пропущен — в диффе нет trust boundary / секретов / парсинга недоверенного ввода, чистый клиентский React-стейт). ### Must fix before merge - **[integration] Сделать rebase ветки на текущий `develop` и переразрешить конфликты.** Файлы `ai-chat-window.tsx`, `chat-thread.tsx`, `hooks/use-chat-session.ts`, `hooks/use-chat-session.test.tsx` изменены **и** в #174 (уже в `develop`), **и** в этом PR — поэтому PR сейчас не мержится. При ребейзе: - сохранить в возвращаемом объекте `useChatSession` и `onServerChatId` (из #174), и новый `startFreshThread`; - убедиться, что новый guard (`threadKeyRef` / `finishingThreadKey`) корректно сочетается с ранней адопцией #174. - *Проверено заранее, чтобы снять тревогу:* сочетается. `adopt` сохраняет тот же mount-key, поэтому `threadKeyRef.current === thread.key` остаётся валидным после ранней адопции, а `startFreshThread` по-прежнему нужен для окна **до** чанка `start` (пока `activeChatId === null`). Дополнительный guard на `onServerChatId` **не** требуется: он вызывается из `useEffect([messages])`, а эффекты размонтированного треда не срабатывают — late-fire ему не грозит (в отличие от `onFinish`/`onError`, которые `useChat` проксирует и после unmount). - **[suggestion][simplification] Убрать ставший избыточным `cancelPendingAdoption()` в `startNewChat`** — `ai-chat-window.tsx`. Идущий сразу следом `startFreshThread()` уже выполняет `pendingNewChatRef.current = null`, и между двумя вызовами ничего от disarm не зависит. Сам экспорт `cancelPendingAdoption` оставить — он по-прежнему нужен в `selectChat`. На корректность не влияет; можно оставить ради симметрии `startNewChat`/`selectChat`. ### Test coverage Все новые ветки покрыты осмысленными ассертами (прогон vitest из клиентского конфига: **13/13 зелёные**): - `startFreshThread()` — remount при `activeChatId === null` (mount-key меняется); - abandoned-ветка `onTurnFinished` — нет adopt, нет per-chat инвалидации, но список инвалидируется; - current-ветка (совпадающий ключ) — adopt происходит. Ветка `finishingThreadKey === undefined` (legacy/тесты) покрыта существующими тестами без отдельного именованного кейса — достаточно. Компонентных тестов на `ai-chat-window`/`chat-thread` нет, но их в проекте и не было: хук — установленный unit-seam, это не пробел. Регресс-замок на #161 присутствует (тесты падали бы на до-фиксовом коде). ### Architecture & design (forward-looking, non-blocking) **Консолидировать разрозненные guard'ы «этот late-callback — от смонтированного треда?» в один seam.** Модуль накапливает ad-hoc защиты от late-callback'ов размонтированных тредов: `pendingNewChatRef` + `cancelPendingAdoption`, зеркало `activeChatIdRef`, и теперь `threadKeyRef` + параметр `finishingThreadKey`. Корень один — `useChat` (ai@6) не abort'ит запрос на unmount и проксирует `onFinish`/`onError`. один предикат `isCurrentThread(key)`, через который проходят все мутирующие identity-колбэки.* Плюсы: корень назван один раз, новые колбэки получают unmount-safety бесплатно, меньше концептуальная поверхность самого баговатого места модуля. Минусы: рефактор со своими тестами; делать **после** ребейза. Усилия: medium. --- *Aspects: stability ✅, regressions ✅, conventions ✅, documentation ✅, test-coverage ✅, simplification (1 nit), architecture (1 proposal). Security — N/A.*
vvzvlad added 1 commit 2026-06-25 22:39:27 +03:00
Merge branch 'develop' into fix/ai-chat-new-chat-during-stream (#162) 4d69518037 
Resolve the PR #162 review blocker: the branch no longer merged into
develop after #174 (onServerChatId early adoption) and #183
(server-sourced Copy/export) landed, which touched the same files.

Conflict resolution (ai-chat-window.tsx, chat-thread.tsx,
use-chat-session.ts):
- Keep develop's onServerChatId wiring (#174) intact in all three files.
- Keep develop's removal of the old liveStateRef / liveThreadRef /
  MutableRefObject live-export mechanism (deleted by #174/#183); it was
  only inherited by this branch and is not part of #161 — not resurrected.
- Graft #162's actual fix on top: startFreshThread() (unconditional
  remount even when activeChatId stays null) and the abandoned-thread
  guard (threadKeyRef + finishingThreadKey on onTurnFinished, forwarded
  as ChatThread's threadKey through onFinish/onError).

Also apply the review's simplification: drop the now-redundant
cancelPendingAdoption() call inside startNewChat (startFreshThread()
already nulls pendingNewChatRef); the export stays, still used by
selectChat.

ai-chat client suite green (176 passed), no conflict markers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vvzvlad commented 2026-06-25 22:39:59 +03:00
Owner
Copy Link

Обновлено: блокер ревью устранён, ветка влита с актуальным develop

Запушен merge-коммит 4d695180 (cee231b1..4d695180). develop теперь предок ветки — PR вливается без конфликтов.

Что сделано

  • Слияние с develop с разрешением конфликтов в ai-chat-window.tsx, chat-thread.tsx, use-chat-session.ts (конфликт был с уже влитыми #174 и #183).
  • Сохранён onServerChatId (#174) во всех трёх файлах + ранняя адопция chatId.
  • Не воскрешён удалённый в #174/#183 механизм liveStateRef/liveThreadRef/MutableRefObject: он жил в merge-base и был лишь унаследован этой веткой, к #161 отношения не имеет. За базу взята версия develop, поверх привит только реальный вклад #162 (startFreshThread, guard брошенного треда threadKeyRef + finishingThreadKey, проброс threadKey из ChatThread).
  • Применено упрощение из ревью: убран ставший избыточным cancelPendingAdoption() в startNewChat (его роль выполняет startFreshThread(), который обнуляет pendingNewChatRef); экспорт cancelPendingAdoption сохранён — он по-прежнему нужен в selectChat.

Проверка

  • git diff develop по 4 файлам = ровно вклад #162 + упрощение, ничего лишнего.
  • Клиентский набор ai-chat зелёный (176 passed, включая 3 теста #161:).
  • Маркеров конфликта нет; повторное ревью — без замечаний (approve).

Архитектурное предложение из ревью (консолидация guard'ов late-callback в один isCurrentThread(key) seam) осознанно оставлено на отдельную задачу — оно non-blocking.

## Обновлено: блокер ревью устранён, ветка влита с актуальным `develop` Запушен merge-коммит `4d695180` (`cee231b1..4d695180`). `develop` теперь предок ветки — **PR вливается без конфликтов**. ### Что сделано - **Слияние с `develop`** с разрешением конфликтов в `ai-chat-window.tsx`, `chat-thread.tsx`, `use-chat-session.ts` (конфликт был с уже влитыми #174 и #183). - **Сохранён `onServerChatId` (#174)** во всех трёх файлах + ранняя адопция chatId. - **Не воскрешён** удалённый в #174/#183 механизм `liveStateRef`/`liveThreadRef`/`MutableRefObject`: он жил в merge-base и был лишь унаследован этой веткой, к #161 отношения не имеет. За базу взята версия `develop`, поверх привит только реальный вклад #162 (`startFreshThread`, guard брошенного треда `threadKeyRef` + `finishingThreadKey`, проброс `threadKey` из `ChatThread`). - **Применено упрощение из ревью:** убран ставший избыточным `cancelPendingAdoption()` в `startNewChat` (его роль выполняет `startFreshThread()`, который обнуляет `pendingNewChatRef`); экспорт `cancelPendingAdoption` сохранён — он по-прежнему нужен в `selectChat`. ### Проверка - `git diff develop` по 4 файлам = ровно вклад #162 + упрощение, ничего лишнего. - Клиентский набор ai-chat зелёный (**176 passed**, включая 3 теста `#161:`). - Маркеров конфликта нет; повторное ревью — без замечаний (approve). Архитектурное предложение из ревью (консолидация guard'ов late-callback в один `isCurrentThread(key)` seam) осознанно оставлено на отдельную задачу — оно non-blocking.
vvzvlad commented 2026-06-25 23:48:51 +03:00
Owner
Copy Link

Код-ревью — PR #162: сброс чата по «New chat» во время стрима первого хода (#161)

Вердикт: Request changes (запросить изменения). Сам код корректен — все «тяжёлые» аспекты (стабильность, регрессии, безопасность, конвенции, упрощение) дали LGTM, ключевая логика проверена на интерливингах (см. ниже). Слияние блокируют только два мелких пункта дисциплины: нет записи в CHANGELOG и устаревший заголовок/комментарий ранее существовавшего теста, который после правки описывает уже несуществующее поведение startNewChat.

Объём ревью: диф develop..4d695180 по 4 файлам (+127/−13). Ветка слита с актуальным develop, конфликтов нет. Запущено 8 аспектных ревьюеров параллельно; находки сведены и перепроверены по исходникам.

Must fix before merge

  • [documentation] Добавить запись в CHANGELOG [Unreleased] › Fixed про #161 — CHANGELOG.md (секция [Unreleased]). Фикс пользовательский: нажатие «New chat» во время стрима первого хода ещё не усыновлённого чата оставляло живой стримящийся тред, а поздний refetch / поздний onFinish мог выдернуть пользователя обратно в покинутый чат. По принятой в проекте конвенции такие фиксы фиксируются в ### Fixed с номером ишью (рядом в файле — (#163), (#146, #147)); #161 в CHANGELOG.md отсутствует. Fix: добавить буллет под ## [Unreleased] › ### Fixed со ссылкой (#161) в стиле соседних записей.

  • [documentation] Поправить устаревший заголовок/комментарий теста про startNewChat → cancelPendingAdoption — apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx:123-139. До правки startNewChat действительно вызывал cancelPendingAdoption(); теперь он вызывает startFreshThread(). Заголовок теста («startNewChat while already in a new chat: cancelPendingAdoption stops…») и комментарий («only the explicit cancelPendingAdoption() disarms») теперь противоречат изменённому коду — это doc-vs-code контрадикция, внесённая именно этой правкой (сам тест по-прежнему валиден: cancelPendingAdoption остался и используется в selectChat). Fix: переименовать тест/комментарий, отвязав сценарий от startNewChat (например, привязав к selectChat / прямому вызову cancelPendingAdoption), и добавить парный тест на разоружение через startFreshThread (см. ниже).

  • [warning][test coverage] Добавить тест: startFreshThread() разоружает уже взведённый error-path fallback — use-chat-session.test.tsx (рядом с :230-238). Обоснование удаления cancelPendingAdoption() из startNewChat целиком держится на том, что startFreshThread обнуляет pendingNewChatRef (use-chat-session.ts:308). Это поведение нигде не проверяется: три новых теста проверяют смену mount-key, отклонение брошенного finish и адопцию при совпадающем ключе, но ни один не делает «взвести fallback → startFreshThread() → поздний refetch → нет адопции». Если startFreshThread когда-нибудь перестанет разоружать, поздний refetch снова выдернет пользователя в только что упавший чат, а сьют останется зелёным. Fix: тест по образцу :123-139, но через startFreshThread() вместо cancelPendingAdoption().

  • [suggestion][test coverage] Покрыть проброс threadKey из ChatThread по ветке onError — chat-thread.tsx:264-285. У chat-thread.tsx нет компонентных тестов; проброс threadKey и в onFinish (onTurnFinished(extractServerChatId(message), threadKey)), и в onError (onTurnFinished(undefined, threadKey)) ничем не зафиксирован. Откат threadKey → undefined на ветке onError молча переоткрыл бы баг #161 для ошибочного хода. Fix: лёгкий render-тест ChatThread (мок useChat), дёргающий onError и проверяющий вызов пропа onTurnFinished(undefined, threadKey).

Test coverage

Новая логика (startFreshThread, guard брошенного треда в onTurnFinished, проброс threadKey) покрыта тремя юнит-тестами хука на уровне поведения. Незакрытые места: (1) разоружение fallback через startFreshThread — load-bearing для удаления cancelPendingAdoption из startNewChat, не проверено; (2) проброс threadKey из ChatThread (ветки onFinish/onError) — компонентных тестов нет. Back-compat путь finishingThreadKey === undefined покрыт существующими одноаргументными вызовами onTurnFinished.

Architecture & design (forward-looking, non-blocking)

  • Множество guard'ов против поздних колбэков брошенного треда. Правка добавляет 6-й/7-й механизм (threadKeyRef + проброс finishingThreadKey) к хуку, где уже есть несколько guard'ов (activeChatIdRef от двойного срабатывания, pendingNewChatRef error-path fallback, render-phase reconciler, history-latch) — все компенсируют одну причину: ai@6 useChat не прерывает запрос при unmount и продолжает проксировать onFinish/onError после размонтирования. Сам автор отметил будущий seam isCurrentThread(key). Дефекта нет, вопрос — понятность и поверхность будущих багов.
    свести guard'ы к одному именованному seam isCurrentThread(key)* без смены поведения. Effort: small. Pros: низкий риск, реализует идею автора, даёт следующему edge-case явное место. Cons: косметика — число механизмов не уменьшает.

  • Полномочия на mount-key разделены между startFreshThread и render-phase reconciler. Оба могут диспатчить reconcile → chatId:null. В случае activeChatId === null (баг #161) reconciler — no-op, ремонтит только startFreshThread; доказательство отсутствия двойного ремонта живёт в комментарии, а не в структуре. Корректно, но «два источника правды для ключа» — ровно тот запах, который убирал thread-identity reducer. Рекомендация: оставить как есть — null → null это не-переход, который divergence-based reconciler структурно не видит, поэтому императивный nudge честен; вводить nonce ради элегантности не стоит. Surfaced лишь чтобы зафиксировать разделение полномочий — при выборе

## Код-ревью — PR #162: сброс чата по «New chat» во время стрима первого хода (#161) **Вердикт: Request changes (запросить изменения).** Сам код корректен — все «тяжёлые» аспекты (стабильность, регрессии, безопасность, конвенции, упрощение) дали LGTM, ключевая логика проверена на интерливингах (см. ниже). Слияние блокируют только два мелких пункта дисциплины: нет записи в `CHANGELOG` и устаревший заголовок/комментарий ранее существовавшего теста, который после правки описывает уже несуществующее поведение `startNewChat`. _Объём ревью: диф `develop..4d695180` по 4 файлам (+127/−13). Ветка слита с актуальным `develop`, конфликтов нет. Запущено 8 аспектных ревьюеров параллельно; находки сведены и перепроверены по исходникам._ ### Must fix before merge - **[documentation] Добавить запись в `CHANGELOG` `[Unreleased] › Fixed` про #161** — `CHANGELOG.md` (секция `[Unreleased]`). Фикс пользовательский: нажатие «New chat» во время стрима первого хода ещё не усыновлённого чата оставляло живой стримящийся тред, а поздний refetch / поздний `onFinish` мог выдернуть пользователя обратно в покинутый чат. По принятой в проекте конвенции такие фиксы фиксируются в `### Fixed` с номером ишью (рядом в файле — `(#163)`, `(#146, #147)`); `#161` в `CHANGELOG.md` отсутствует. Fix: добавить буллет под `## [Unreleased] › ### Fixed` со ссылкой `(#161)` в стиле соседних записей. - **[documentation] Поправить устаревший заголовок/комментарий теста про `startNewChat → cancelPendingAdoption`** — `apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx:123-139`. До правки `startNewChat` действительно вызывал `cancelPendingAdoption()`; теперь он вызывает `startFreshThread()`. Заголовок теста («`startNewChat while already in a new chat: cancelPendingAdoption stops…`») и комментарий («`only the explicit cancelPendingAdoption() disarms`») теперь противоречат изменённому коду — это doc-vs-code контрадикция, внесённая именно этой правкой (сам тест по-прежнему валиден: `cancelPendingAdoption` остался и используется в `selectChat`). Fix: переименовать тест/комментарий, отвязав сценарий от `startNewChat` (например, привязав к `selectChat` / прямому вызову `cancelPendingAdoption`), и добавить парный тест на разоружение через `startFreshThread` (см. ниже). - **[warning][test coverage] Добавить тест: `startFreshThread()` разоружает уже взведённый error-path fallback** — `use-chat-session.test.tsx` (рядом с `:230-238`). Обоснование удаления `cancelPendingAdoption()` из `startNewChat` целиком держится на том, что `startFreshThread` обнуляет `pendingNewChatRef` (`use-chat-session.ts:308`). Это поведение нигде не проверяется: три новых теста проверяют смену mount-key, отклонение брошенного finish и адопцию при совпадающем ключе, но ни один не делает «взвести fallback → `startFreshThread()` → поздний refetch → нет адопции». Если `startFreshThread` когда-нибудь перестанет разоружать, поздний refetch снова выдернет пользователя в только что упавший чат, а сьют останется зелёным. Fix: тест по образцу `:123-139`, но через `startFreshThread()` вместо `cancelPendingAdoption()`. - **[suggestion][test coverage] Покрыть проброс `threadKey` из `ChatThread` по ветке `onError`** — `chat-thread.tsx:264-285`. У `chat-thread.tsx` нет компонентных тестов; проброс `threadKey` и в `onFinish` (`onTurnFinished(extractServerChatId(message), threadKey)`), и в `onError` (`onTurnFinished(undefined, threadKey)`) ничем не зафиксирован. Откат `threadKey → undefined` на ветке `onError` молча переоткрыл бы баг #161 для ошибочного хода. Fix: лёгкий render-тест `ChatThread` (мок `useChat`), дёргающий `onError` и проверяющий вызов пропа `onTurnFinished(undefined, threadKey)`. ### Test coverage Новая логика (`startFreshThread`, guard брошенного треда в `onTurnFinished`, проброс `threadKey`) покрыта тремя юнит-тестами хука на уровне поведения. Незакрытые места: (1) разоружение fallback через `startFreshThread` — load-bearing для удаления `cancelPendingAdoption` из `startNewChat`, не проверено; (2) проброс `threadKey` из `ChatThread` (ветки `onFinish`/`onError`) — компонентных тестов нет. Back-compat путь `finishingThreadKey === undefined` покрыт существующими одноаргументными вызовами `onTurnFinished`. ### Architecture & design (forward-looking, non-blocking) - **Множество guard'ов против поздних колбэков брошенного треда.** Правка добавляет 6-й/7-й механизм (`threadKeyRef` + проброс `finishingThreadKey`) к хуку, где уже есть несколько guard'ов (`activeChatIdRef` от двойного срабатывания, `pendingNewChatRef` error-path fallback, render-phase reconciler, history-latch) — все компенсируют одну причину: `ai@6 useChat` не прерывает запрос при unmount и продолжает проксировать `onFinish`/`onError` после размонтирования. Сам автор отметил будущий seam `isCurrentThread(key)`. Дефекта нет, вопрос — понятность и поверхность будущих багов. свести guard'ы к одному именованному seam `isCurrentThread(key)`* без смены поведения. Effort: small. Pros: низкий риск, реализует идею автора, даёт следующему edge-case явное место. Cons: косметика — число механизмов не уменьшает. - **Полномочия на mount-key разделены между `startFreshThread` и render-phase reconciler.** Оба могут диспатчить `reconcile → chatId:null`. В случае `activeChatId === null` (баг #161) reconciler — no-op, ремонтит только `startFreshThread`; доказательство отсутствия двойного ремонта живёт в комментарии, а не в структуре. Корректно, но «два источника правды для ключа» — ровно тот запах, который убирал thread-identity reducer. **Рекомендация:** оставить как есть — `null → null` это не-переход, который divergence-based reconciler структурно не видит, поэтому императивный nudge честен; вводить nonce ради элегантности не стоит. Surfaced лишь чтобы зафиксировать разделение полномочий — при выборе
Ghost added 1 commit 2026-06-26 16:59:27 +03:00
test(ai-chat): cover the #161 New-chat-during-stream reset + changelog 8f9d7fe0bd 
Address PR #162 review must-fixes:

- CHANGELOG: add an [Unreleased] > Fixed bullet for #161 (New chat during
  the first turn's stream now resets the thread; a late refetch/onFinish
  from the abandoned thread no longer pulls the user back in).
- Re-anchor the stale startNewChat/cancelPendingAdoption test to selectChat:
  startNewChat now calls startFreshThread, but cancelPendingAdoption still
  backs selectChat, so the disarm guard is decoupled and kept valid.
- Add a test pinning that startFreshThread() disarms the armed error-path
  fallback (the justification for dropping cancelPendingAdoption from
  startNewChat).
- Add a ChatThread render test (mocked useChat) asserting the onError branch
  forwards onTurnFinished(undefined, threadKey).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ghost closed this pull request 2026-06-26 17:10:31 +03:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 epic
Large multi-phase effort spanning many changes
 feature
New functionality request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 idea
Idea / proposal for discussion
 invalid
This doesn't seem right
 needs-human
эскалация: нужно решение человека
 question
Further information is requested
 refactor
Code cleanup / refactoring
 review/approved
в последнем ревью нет открытых blocking-находок
 review/changes-requested
последнее ревью оставило открытые blocking-находки
 review/needs
head не ревьюился (head != reviewed_head)
 security
Security / hardening issue
 status/blocked
ждёт зависимость blocked_by
 status/done
закрыто и проверено
 status/in-progress
в активной работе (мягкая заявка)
 status/ready
специфицировано, не заблокировано, ждёт исполнителя
 test
Test coverage / test infrastructure
 wontfix
This will not be worked on
No Label bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent_coder agent_reviewer vvzvlad
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#162
Delete Branch "fix/ai-chat-new-chat-during-stream"

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?