镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

MAINT Extract ChatWindow send workflow into a reducer (#2652) - #4162

Open
Julio César Suástegui (juliosuas) wants to merge 6 commits into
microsoft:mainfrom
juliosuas:maint/2652-send-reducer
Open

Julio César Suástegui (juliosuas) wants to merge 6 commits into
microsoft:mainfrom
juliosuas:maint/2652-send-reducer

Conversation

@juliosuas

Copy link
Copy Markdown
Contributor

Description

Related to #2652. Final step: the send workflow as an explicit reducer plus the ChatWindow composition cleanup, as scoped in the issue thread. Loading landed in #3310, score/objective/export in #3369, and copy/branch in #3812.

  • frontend/src/utils/chatSendState.ts: reducer with typed actions for the send workflow's rendered state: per-conversation send and scoring locks, send issues, failed-send recoveries, repeated-send progress and the undoable last send.
  • frontend/src/hooks/useChatSend.ts: useChatSendState owns that reducer plus the synchronous locks the conversation loader reads; useChatSend owns the commands (optimistic messages, attachment and conversion capture, lazy attack creation, progress tracking, refresh after unknown delivery, preparation-draft restore). Its input is grouped by seam (tracking, route, readiness, request, composer, loader, callbacks) with typed, documented interfaces, following useConversationCopy.
  • ChatWindow.tsx: composes the two hooks instead of inlining the send workflow (3,045 to 2,208 lines). The conversation loader result is passed through unchanged.

No prop, UI, accessibility, request payload or navigation changes.

Open questions for review:

  • The send state is split into two hooks because the loader needs the send locks before useChatSend can consume the loader. Is that boundary acceptable, or would you prefer a different split?
  • Import handoff, editor save/exit and apply-scorer still live in ChatWindow. I left import handoff alone because it overlaps FEAT GUI: Import dataset prompts and conversations into chat (#2751) #3670. Happy to extract the others here or in a follow-up if you want them in scope.

Tests and Documentation

  • New chatSendState.test.ts (reducer transitions) and useChatSend.test.ts (duplicate sends, two conversations sending independently, optimistic prompts on return to an in-flight conversation, first-send conversation creation, unknown delivery, pre-submission failures, converted pieces): 31/31.
  • Frontend jest: 4767/4767 (194 suites), including all 314 ChatWindow tests.
  • tsc --noEmit: 0 errors. Lint: 0 errors. npm run build passes.
  • Mock e2e: 198/200; the 2 failures in e2e/onboarding-tour.spec.ts fail identically on main (935afa8).
  • No documentation changes needed (internal refactor).

AI-assisted; reviewed and tested by me.

Final step of microsoft#2652, after microsoft#3310 (loading), microsoft#3369 (score, objective and
export) and microsoft#3812 (copy and branch).

The send workflow's rendered state (per-conversation send and scoring
locks, send issues, failed-send recoveries, repeated-send progress and
the undoable last send) now moves through an explicit reducer with typed
actions in utils/chatSendState.ts. useChatSendState owns that reducer
plus the synchronous locks the conversation loader reads; useChatSend
owns the commands: optimistic messages, attachment and conversion
capture, lazy attack creation, progress tracking, refresh after unknown
delivery and preparation-draft restore. Its input is grouped by seam
(route, readiness, request, composer, loader, callbacks), and ChatWindow
passes the conversation loader through unchanged.

No prop, UI, accessibility, request payload or navigation changes.
Reducer transitions and the hook's regression cases (duplicate sends,
two conversations sending independently, optimistic prompts on return to
an in-flight conversation, first-send conversation creation, unknown
delivery, pre-submission failures, converted pieces) have focused tests.

Refs microsoft#2652
Copilot AI balanced review requested due to automatic review settings November 10, 2026 15:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues were identified for the send-workflow extraction into a reducer and focused hooks.

No open issues 🎉

What changed in this PR

Continues #2652 by extracting ChatWindow’s send workflow into a reducer and focused hooks, building on the earlier loading, mutation/export, and copy/branch extractions.

Changes:

  • Centralizes rendered send state in a typed reducer, including locks, recovery, repeated-send progress, and undo.
  • Moves send orchestration into useChatSend and synchronous tracking into useChatSendState, leaving ChatWindow to compose them.
  • Adds focused reducer and hook tests for state transitions, independent sends, optimistic messages, recovery, and conversions.
File Description
frontend/​src/​utils/​chatSendState.ts Defines send state, actions, reducer, and recovery helpers.
frontend/​src/​utils/​chatSendState.test.ts Tests reducer transitions and persisted recovery handling.
frontend/​src/​hooks/​useChatSend.ts Owns send tracking, submission, progress, and recovery commands.
frontend/​src/​hooks/​useChatSend.test.ts Tests send orchestration and key failure paths.
frontend/​src/​components/​Chat/​ChatWindow.tsx Replaces inline send orchestration with hook composition.

Review effort: Balanced

Brings in microsoft#3663 (reload a sending conversation after another one has
loaded) and microsoft#3772. The ChatWindow conflict is resolved by keeping the
extracted send workflow; microsoft#3663's markConversationLoaded() on the created
conversation moves to the first-send branch of useChatSend, where the
created attack is now routed. Adds a useChatSend test for it.
Brings in microsoft#3506 (runtime generation configuration) and the rest of main
since microsoft#3663. ChatWindow keeps the extracted send workflow; microsoft#3506's send
path moves into useChatSend:

- the request carries the composer's generation draft, snapshotted on
  the pending send and parsed against the target when the send starts;
- the first piece carries request_generation_config instead of a
  temperature-rebuilt target binding (buildTemperatureTarget is gone,
  along with the now-unused targetResolutionStatus option);
- a first send reports its created attack so ChatWindow keeps the
  next-send draft across that route change.

ChatWindow keeps the draft state, generation settings UI and the
send-disabled reason. useChatSend tests cover the request metadata, the
send-time snapshot, the created-attack callback and out-of-range input.
# Conflicts:
#	frontend/src/components/Chat/ChatWindow.tsx
Adds a useChatSend test for a count > 1 send: one submission with the
repeat options, sibling conversation adoption with its own lock and
evidence read, lock release, and the repeat view surviving completion
until dismissed. Documents dispatch/locksRef on ChatSendTracking as the
seam driven by useChatSend only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants