Skip to content

feat(web-shell): expose transcript event changes - #5193

Merged
ytahdn merged 2 commits into
QwenLM:mainfrom
chiga0:feat/web-shell-support-onevent
Jun 16, 2026
Merged

feat(web-shell): expose transcript event changes#5193
ytahdn merged 2 commits into
QwenLM:mainfrom
chiga0:feat/web-shell-support-onevent

Conversation

@ytahdn

@ytahdn ytahdn commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR adds a web-shell callback that lets host applications observe transcript block changes as the shell processes user, assistant, and tool events. It also tightens prompt-status recovery during daemon replay so replayed non-user events do not incorrectly mark a fresh prompt as waiting.

Why it's needed

Consumers embedding web-shell need a direct way to react to the transcript/event stream without reimplementing the shell's internal transcript reducer. Separately, replay recovery should only infer an in-progress prompt when replay contains a user message without a terminal event; otherwise a new session or non-turn replay can briefly look like a prompt is still running.

Reviewer Test Plan

How to verify

Use web-shell with an onEventChange callback and confirm it receives the full transcript blocks array whenever the displayed transcript changes after user, assistant, or tool activity. Resume or create a session whose replay snapshot contains only non-user session updates and confirm the prompt status stays idle instead of showing a waiting/in-progress state.

Evidence (Before & After)

N/A for screenshots. Local verification passed for the focused daemon session test, the webui build, the web-shell build, and ESLint over the changed packages.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

cd packages/webui && npx vitest run src/daemon/session/DaemonSessionProvider.test.tsx

npm run build --workspace=@qwen-code/webui

npm run build --workspace=@qwen-code/web-shell

npx eslint packages/webui packages/web-shell --ext .ts,.tsx --max-warnings 0

Risk & Scope

  • Main risk or tradeoff: onEventChange receives the full transcript blocks array on every transcript change, so consumers should treat the value as read-only and avoid expensive synchronous work in the callback.
  • Not validated / out of scope: full browser matrix and end-to-end host integration outside the local package checks.
  • Breaking changes / migration notes: none. Existing web-shell users are unaffected unless they pass the new callback.

Linked Issues

N/A

中文说明

What this PR does

这个 PR 为 web-shell 增加了一个回调,让宿主应用可以在 shell 处理 user、assistant、tool 等事件并更新 transcript blocks 时得到通知。同时收紧了 daemon replay 阶段的 prompt 状态恢复逻辑,避免只有非用户事件的 replay 把新会话误判成 waiting。

Why it's needed

嵌入 web-shell 的消费方需要一种直接观察 transcript/event 变化的方式,而不需要重新实现 shell 内部的 transcript reducer。另一方面,replay 恢复只应该在 replay 中存在用户消息且没有终止事件时推断 prompt 仍在进行;否则新会话或非 turn replay 会短暂表现得像仍有 prompt 在运行。

Reviewer Test Plan

How to verify

在 web-shell 中传入 onEventChange 回调,确认用户、助手或工具活动导致展示内容变化时,它会收到完整的 transcript blocks 数组。恢复或创建一个 replay snapshot 只包含非用户 session update 的会话,确认 prompt status 保持 idle,而不是显示 waiting 或进行中状态。

Evidence (Before & After)

截图不适用。本地已通过聚焦的 daemon session 测试、webui build、web-shell build,以及对变更 package 的 ESLint 检查。

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

cd packages/webui && npx vitest run src/daemon/session/DaemonSessionProvider.test.tsx

npm run build --workspace=@qwen-code/webui

npm run build --workspace=@qwen-code/web-shell

npx eslint packages/webui packages/web-shell --ext .ts,.tsx --max-warnings 0

Risk & Scope

  • Main risk or tradeoff: onEventChange 会在每次 transcript 变化时收到完整的 transcript blocks 数组,因此消费方应把它当作只读数据,并避免在回调中执行昂贵的同步逻辑。
  • Not validated / out of scope: 未覆盖完整浏览器矩阵,也未做本地 package 检查之外的宿主集成 E2E。
  • Breaking changes / migration notes: 无。现有 web-shell 使用方不传新回调时不受影响。

Linked Issues

N/A

@ytahdn
ytahdn requested a review from qwen-code-ci-bot June 16, 2026 10:27

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Stale comment at DaemonSessionProvider.tsx:636 — the comment reads "If replay has events but no terminal signal..." but the code now checks hasReplayUserMessage (only user_message_chunk), not replayEvents.length > 0. Consider updating to "If replay has a user message but no terminal signal..." to match the narrowed condition.

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/App.tsx Outdated
Comment thread packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx
Comment thread packages/web-shell/client/App.tsx Outdated
@ytahdn
ytahdn requested a review from chiga0 June 16, 2026 13:29

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overview

Final Verdict: Approve — Focused, well-tested PR that adds a transcript change callback and tightens replay prompt-status recovery. The second commit (048ccd0a) addresses all suggestions from the earlier automated review.

Cross-Validation

# Finding Reviewer My Assessment
C1 Stale comment at line 636 ("If replay has events…") qwen-code-ci-bot Fixed at HEAD — commit 048ccd0a updated to "If replay has a user message but no terminal signal"
C2 Naming: onEventChangeonTranscriptChange qwen-code-ci-bot Fixed at HEAD — prop is onTranscriptChange
C3 Missing positive-branch test for replay user_message_chunk qwen-code-ci-bot Fixed at HEAD — test "marks replayed user turns without terminal events as waiting" present
C4 JSDoc missing debounce/throttle warning qwen-code-ci-bot Fixed at HEAD — JSDoc includes "fires on every streaming delta during active generation, so consumers should debounce or throttle expensive work"

No unique findings from my independent review.

Additional Audit Coverage

Areas I independently checked beyond existing findings:

  • useEffect correctness: verified [blocks, onTranscriptChange] deps are complete; effect fires onTranscriptChange?.(blocks) safely via optional chaining when prop is omitted
  • isUserMessageEvent type narrowing: as cast with null | undefined union + optional chaining is defensive and consistent with codebase patterns; discriminator field sessionUpdate is the canonical one used elsewhere
  • Replay recovery semantics: the change from replayEvents.length > 0 to hasReplayUserMessage correctly prevents false "waiting" state when replay contains only assistant messages without a terminal event — the old condition would have triggered a loading indicator flicker for completed conversations
  • Callback frequency design: confirmed blocks (raw from useTranscriptBlocks()) is intentionally passed, not messageBlocks (animation-frame throttled) — consumers get full-fidelity data for logging/sync, with the JSDoc warning them to debounce
  • Test quality: negative test uses agent_message_chunk (the most likely false-positive trigger), positive test correctly asserts promptStatus === 'waiting' — together they bracket the condition inversion regression
  • Consistency with sibling callbacks: onTranscriptChange follows the same pattern as onStreamingStateChange and onConnectionChange (prop declaration, destructure, useEffect dispatch)

@ytahdn
ytahdn requested a review from yiliang114 June 16, 2026 14:26

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No review findings. Downgraded from Approve to Comment: CI still running.

The two changes are well-executed:

  • Replay status fix: Correctly narrows the prompt-status seeding from "any replay events" to "replay contains a user message". The isUserMessageEvent helper follows established patterns in the file. Both branches (negative: agent-only replay stays idle, positive: user-message replay becomes waiting) are covered by the new tests.

  • onTranscriptChange callback: Follows the identical useEffect pattern as the sibling onStreamingStateChange and onConnectionChange callbacks. Dependencies are correct. The JSDoc appropriately warns consumers about streaming-frequency firing.

Builds (webui, web-shell) and all 75 tests in DaemonSessionProvider.test.tsx pass. ESLint and tsc clean on changed files. — qwen3.7-max via Qwen Code /review

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the transcript callback and replay-status changes, including resolved review threads and CI status. The scope is focused and the tests cover the relevant replay branches; no blocking issues found.

@ytahdn
ytahdn merged commit 259c933 into QwenLM:main Jun 16, 2026
33 checks passed
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.

4 participants