Skip to content

feat(web-shell): paginate restored session history - #7064

Merged
wenshao merged 8 commits into
QwenLM:mainfrom
chiga0:codex/web-shell-history-pagination
Jul 17, 2026
Merged

feat(web-shell): paginate restored session history#7064
wenshao merged 8 commits into
QwenLM:mainfrom
chiga0:codex/web-shell-history-pagination

Conversation

@ytahdn

@ytahdn ytahdn commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR changes restored Web Shell sessions to paint a bounded recent history page first and fetch complete older user turns as the transcript scrolls to the top. Backward pages use stable persisted-record boundaries and opaque cursors, preserve tool execution sequences within a turn, maintain the scroll anchor while prepending content, and stop cleanly at the browser transcript capacity limit.

The daemon advertises the new contract through the session_transcript_pagination capability. Web Shell opts into pagination only when that feature is present, so older servers continue using their existing full-history load behavior.

Why it's needed

Loading every persisted record before rendering makes long sessions slow and can produce oversized replay responses and browser transcript state. A bounded initial page improves restore latency and memory use while automatic top-loading keeps older history accessible without adding a manual control.

Capability negotiation is required because a newer Web Shell may connect to an older daemon that does not understand the pagination request or backward transcript contract.

Reviewer Test Plan

How to verify

  1. Restore a long persisted session through Web Shell and confirm the latest history renders first.
  2. Scroll to the top and confirm a loading status appears, the previous complete user turn is prepended automatically, and the visible scroll position remains anchored.
  3. Use a turn containing multiple tool updates and confirm no page begins with only the tail of that turn.
  4. Connect with capabilities that omit session_transcript_pagination and confirm session load omits the page-size option and no older-page request is made.
  5. Connect with the capability present and confirm the initial load uses a 100-record target and older pages are requested as needed.
  6. Request a workspace-qualified backward page whose complete turn exceeds the source-byte limit and confirm the server returns HTTP 413 with transcript_page_too_large instead of a partial turn.

Evidence (Before & After)

Before: restored sessions replayed all persisted history before rendering, and Web Shell had no automatic path to retrieve history omitted by replay compaction.

After: restored sessions render the recent bounded page first, automatically prepend complete older turns at the top, and retain the legacy full-load path when the server does not advertise pagination.

Tested on

OS Status
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

Node.js 22, local daemon bundle and Web Shell build.

Risk & Scope

  • Main risk or tradeoff: backward pages align to complete user turns, so a single long turn may exceed the requested record count; workspace-qualified reads still enforce their source-byte limit and return an explicit 413 when the complete turn is too large.
  • Not validated / out of scope: live-session block eviction and a full cross-browser E2E matrix are not changed by this PR.
  • Breaking changes / migration notes: none. Pagination is capability-gated and existing clients or servers retain the legacy load behavior.

Linked Issues

N/A

Pictures

image
中文说明

此 PR 的改动

此 PR 将 Web Shell 恢复会话改为优先渲染有上限的最近历史页,并在记录滚动到顶部时按完整用户轮次获取更早内容。向后分页使用稳定的持久化记录边界和不透明游标,保证同一轮中的工具执行序列不会被拆开,在前插内容时保持滚动锚点,并在达到浏览器记录容量上限时停止加载。

daemon 通过 session_transcript_pagination capability 声明新协议。只有该 feature 存在时 Web Shell 才启用分页,因此旧 server 会继续使用原来的全量历史 load 行为。

为什么需要

恢复时先加载全部持久化记录,会让长会话变慢,并可能产生过大的 replay 响应和浏览器 transcript 状态。限制初始页可以改善恢复延迟和内存占用,同时顶部自动加载让用户无需手动操作仍可访问更早历史。

能力协商是必要的,因为新版 Web Shell 可能连接到不理解分页请求或向后 transcript 协议的旧 daemon。

Reviewer 测试计划

如何验证

  1. 在 Web Shell 中恢复一个较长的持久化会话,确认先显示最近历史。
  2. 滚动到顶部,确认出现加载状态、自动前插上一个完整用户轮次,并保持当前可见位置不跳动。
  3. 使用包含多次工具更新的轮次,确认任何分页都不会只显示该轮次的尾部。
  4. 使用不包含 session_transcript_pagination 的 capabilities,确认 session load 不发送页长参数,也不会请求更早页面。
  5. 使用包含该 capability 的 server,确认初始 load 使用 100 条记录作为分页目标,并按需请求更早页面。
  6. 请求一个完整轮次超过源字节限制的 workspace 向后分页,确认 server 返回 HTTP 413 和 transcript_page_too_large,而不是返回部分轮次。

前后对比证据

改动前:恢复会话会在渲染前 replay 全部持久化历史,Web Shell 也没有自动获取被 replay 压缩丢弃历史的路径。

改动后:恢复会话先渲染有上限的最近页,在顶部自动前插完整的更早轮次;当 server 未声明分页能力时,仍保留旧的全量 load 路径。

测试平台

OS 状态
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

环境(可选)

Node.js 22,本地 daemon bundle 和 Web Shell build。

风险与范围

  • 主要风险或取舍:向后分页会按完整用户轮次对齐,因此单个很长的轮次可能超过请求的记录数;workspace 路由仍执行源字节限制,当完整轮次过大时明确返回 413。
  • 未验证或超出范围:此 PR 不改变实时会话 block 淘汰策略,也未执行完整的跨浏览器 E2E 矩阵。
  • 破坏性变更或迁移说明:无。分页受 capability 控制,现有客户端或 server 会保留旧的 load 行为。

关联 Issue

N/A

@ytahdn

ytahdn commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

E2E / verification report

Tested locally on macOS with Node.js 22 after rebasing onto the current main.

  • Real bundled daemon: GET /capabilities returned HTTP 200 and advertised session_transcript_pagination.
  • Real bundled daemon: a workspace backward page containing a complete ~5 MiB turn returned HTTP 413 with transcript_page_too_large and the 4 MiB source limit.
  • Capability compatibility harness: without the feature, load omitted historyPageSize, history paging stayed disabled, and no transcript-page request was made; with the feature, load sent the configured page size and the older page was prepended.
  • Replay compaction harness: persisted events sharing one record ID were retained or evicted together, and a compacted paged load reported that older history remained.
  • Web Shell DOM tests: 52 message-list tests passed, including automatic top loading, underfilled transcript loading, loading status, and capacity status.
  • Split View tests: 36 passed.
  • WebUI session provider tests: 159 passed.
  • Serve capability/load/transcript focused tests: 12 passed.
  • npm run build: passed.
  • npm run typecheck: passed.

Expected stderr from mocked detach failures and the intentional Split View error-boundary case appeared during tests; all corresponding tests passed.

@ytahdn
ytahdn requested review from doudouOUC, qwen-code-ci-bot and wenshao and removed request for qwen-code-ci-bot and wenshao July 17, 2026 02:03
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Re-run at 73ddfdebc — head moved since the previous pass (fa98b2ed4). Two new commits: merge conflict resolution and a malformed-event skip fix.

Template good ✓ — all required sections present with bilingual content.

Problem: Real, observed performance issue. Loading all persisted records before rendering makes long Web Shell sessions slow and produces oversized replay responses. The before/after is clear — restored sessions now render the bounded recent page first.

Direction: Aligned with Web Shell performance and UX goals. Paginated session restore with capability negotiation is a well-established pattern. The session_transcript_pagination feature gate ensures backward compatibility with older daemons.

Size: 1661 production logic lines (additions + deletions) across 30 production files, 1214 test lines across 9 test files, 155 design-doc lines. This PR touches core paths (packages/core/src/services/, packages/cli/src/serve/, packages/cli/src/acp-integration/) well past the 500-line and 1000-line thresholds — maintainer awareness flagged. The maintainer (@wenshao) has been actively engaged across multiple review rounds.

Approach: The scope matches the feature — pagination threads through core reader, serve routes, ACP bridge, SDK types, and Web Shell UI. Each layer does its part and no more. The latest commit (73ddfdebc) adds defensive try/catch for malformed transcript page events in loadMoreTranscript, emitting a recoverable notice instead of crashing — good hardening. No simpler path visible; the capability negotiation and turn-boundary alignment requirements drive the complexity.

Moving on to code review. 🔍

中文说明

Re-run at 73ddfdebc — 自上次审查以来 head 有变动。两个新提交:合并冲突解决和 malformed event 跳过修复。

模板完整 ✓

问题:已观测到的性能问题。恢复时全量加载持久化记录导致长会话变慢。

方向:与 Web Shell 性能目标一致。capability 协商确保向后兼容。

规模:1661 行生产逻辑代码(30 个生产文件),1214 行测试代码(9 个测试文件),155 行设计文档。触及核心路径(packages/core/src/services/packages/cli/src/serve/packages/cli/src/acp-integration/),超过 500 行和 1000 行阈值——已标记维护者关注。维护者 @wenshao 已在多轮审查中积极参与。

方案:范围与功能匹配——分页贯穿核心 reader、serve 路由、ACP bridge、SDK 类型和 Web Shell UI。最新提交为 malformed transcript page event 添加了防御性 try/catch。未发现更简路径。

进入代码审查 🔍

Qwen Code · qwen3.7-max

Reviewed at 73ddfdebc11eb8d1ee22bb09b3bfd43e233599a8 · re-run with @qwen-code /triage

chiga0
chiga0 previously approved these changes Jul 17, 2026

@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.

Code Review Overview (AI Generated)

PR: #7064 — feat(web-shell): paginate restored session history
Type: New Feature
Change size: +1764/-56 across 34 files
HEAD: c5fe3bc5

Findings Summary

  • Critical/Major: 0
  • Minor: 4
  • Nit: 3

Review

Well-architected paginated history feature with strong layering, careful turn-boundary handling, and clean backward compatibility via capability negotiation. The design correctly separates concerns: core reader handles page selection with turn alignment, compaction engine respects record boundaries, ACP agent forwards pagination parameters, and Web Shell manages scroll-to-top UX with proper anchor preservation.

Key Strengths Verified

  • Turn boundary preservation: selectBackwardPageUuids uses a multi-pass algorithm (limit-based start → forward turn scan → backward turn scan → byte limiting → re-alignment) to ensure no page begins with only the tail of a multi-update turn. If the aligned turn exceeds maxBytes, it throws SessionTranscriptPageTooLargeError rather than returning a partial turn.
  • Capability negotiation: session_transcript_pagination is correctly gated. When absent, the client sends no page size, receives no hasMore, and never calls getSessionTranscriptPage. Degradation is clean.
  • Scroll anchor preservation: Standard "capture height/position, adjust after prepend" pattern with requestAnimationFrame. followPausedByUserRef prevents auto-follow from fighting the anchor.
  • Race condition prevention: Three-layer protection — component ref guard, provider ref guard, and session change guard. Dual guards make duplicate triggers impossible even under rapid scroll events.
  • Capacity limit: Correctly handled for both initial page (oversized replay kept intact with capacityReached: true) and subsequent pages (prepend rejected when total blocks exceed maxBlocks).
  • Test coverage: 30+ new/updated tests across 8 files covering backward pagination, turn alignment, long-turn completeness, maxBytes rejection, capability gating, scroll-to-top trigger, underfill auto-load, resize auto-load, and capacity indicator.

Minor Findings

  1. Turn-start predicate duplicated: isHistoryTurnStart in acpAgent.ts and isReplayTurnStart in session-transcript-reader.ts encode the same logic (type === 'user' && subtype !== 'mid_turn_user_message') independently. Drift risk if the definition changes. Consider extracting a shared predicate in core.

  2. No DOM test validating scroll position math: The scroll anchor preservation logic (scrollTop = previousTop + (newScrollHeight - previousHeight)) is not explicitly tested. Existing DOM tests verify that loadMore is called but don't assert the resulting scrollTop value.

  3. Error path permanently stops paging: On error, hasMore is set to false and a warning notice is emitted. The user cannot retry without switching sessions. Consider exposing a retry action or allowing loadMore to be retried after a cooldown (UX follow-up).

  4. Missing edge-case tests: No test for empty-session pagination, beforeRecordId pointing to the first record, or the stale-session guard during an in-flight older-page load.

Nits

  1. selectBackwardPageUuids performs five passes — more inline comments explaining each pass's purpose would aid future maintainers.
  2. ResizeObserverStub assignment changed from ??= to = in test setup — intentional but could mask cross-test pollution if isolation is weak.
  3. getPersistedReplayRecordId wraps isRecord checks in try/catch — the type guards should be sufficient; the catch is belt-and-suspenders.

Final Verdict

LGTM. The design is sound, turn boundaries are correctly preserved, capability negotiation degrades cleanly, and test coverage is broad. All findings are Minor/Nit and non-blocking.


This review was generated by QoderWork AI

@ytahdn

ytahdn commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

CI failure follow-up:

The workspace-sidebar visual test still expected the Primary badge, but that badge was removed by #7035 while the visual scenario added in #7041 retained the old assertion. The failure reproduces on the current upstream/main code path and is unrelated to transcript pagination.

I updated the visual assertion to wait for both workspace headings and the loaded primary session row, then verified the focused dark/light Playwright captures (2 passed) and the sidebar workspace-removal unit suite (20 passed). Commit: fe42aaf.

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Re-run at 73ddfdebc. New commits since the previous pass: merge conflict resolution (9959ec009) and fix(webui): skip malformed transcript page events (73ddfdebc).

New in this pass: The loadMoreTranscript callback now wraps per-event processing in a try/catch — malformed history events emit a recoverable daemon.replay_event_malformed notice and are skipped instead of aborting the entire page load. This is correct defensive handling for the backward-pagination path where older events may have inconsistent shapes.

Core reader (session-transcript-reader.ts): Backward paging mode (selectBackwardPageUuids) correctly aligns pages to replay turn boundaries, enforces the 4 MiB SESSION_TRANSCRIPT_MAX_PAGE_BYTES budget, and throws SessionTranscriptPageTooLargeError when a complete turn exceeds the budget. The direction: 'backward' field flows through cursor state with proper validation in decodeCursorState.

ACP bridge + CLI routes: historyPageSize flows through session/load as private ACP metadata. The backward transcript route enforces mutual exclusivity of cursor and beforeRecordId. The shared SESSION_TRANSCRIPT_MAX_PAGE_BYTES constant replaced the local WORKSPACE_TRANSCRIPT_PAGE_SOURCE_MAX_BYTES — good consolidation.

Compaction engine: seedReplayEvents now groups events by record ID before creating replay segments, ensuring multi-event records stay in a single segment. This is necessary for the backward paging path to correctly identify turn boundaries.

WebUI provider (DaemonSessionProvider.tsx): prependTranscriptHistory correctly checks total block count against maxBlocks before prepending, and sets capacityReached when the combined count would exceed the limit. The loadMoreTranscript callback properly guards against session switches mid-request. Retry logic correctly distinguishes terminal failures (partial pages, 4xx except 408/429) from retryable ones (5xx, 408, 429).

Web Shell scroll anchoring (MessageList.tsx): Before loading older history, captures scrollHeight and scrollTop. After prepending, a layout effect restores the visual position. loadOlderHistoryIfUnderfilled handles the viewport-underfill case with a guard against infinite loops (checks totalVirtualSize stability).

No blockers found. Implementation is thorough, well-tested, and follows project conventions.

CI

All checks green:

  • Test (ubuntu-latest, Node 22.x): pass
  • Web-shell E2E Smoke: pass
  • review-pr: pass
中文说明

代码审查在 73ddfdebc 重新运行。新提交修复了 malformed transcript page event 处理——在 loadMoreTranscript 中为每个事件添加了 try/catch,跳过格式异常的事件而不是中断整页加载。

核心 reader、ACP bridge、CLI 路由、compaction engine、WebUI provider 和 Web Shell scroll anchoring 的实现均正确。无阻塞问题。

CI 全部通过。

Qwen Code · qwen3.7-max

Reviewed at 73ddfdebc11eb8d1ee22bb09b3bfd43e233599a8 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — Clean across every stage. Real performance problem, well-architected paginated solution with capability negotiation, thorough implementation, all CI green, maintainer actively engaged through multiple review rounds.

This PR solves a genuine problem — restoring long sessions was slow and could produce oversized replay responses. The solution is well-layered: backward paging in the core reader with turn-boundary alignment, capability negotiation through session_transcript_pagination, defensive error handling for malformed events, and proper scroll anchoring in the Web Shell.

The implementation has been through several review cycles with @wenshao, and each round of feedback has been addressed promptly. The latest commit adds try/catch for malformed transcript page events — a defensive fix that prevents a single bad event from aborting the entire backward page load.

The PR is large (1661 production lines across 30 files) but the scope is driven by the feature's requirements — pagination threads through core, CLI, bridge, SDK, and web-shell. No part of the diff feels gratuitous.

LGTM, ready to ship. ✅

中文说明

信心:5/5 — 各阶段均通过。真实的性能问题,带有 capability 协商的分页方案架构良好,实现完整,CI 全绿,维护者在多轮审查中积极参与。

PR 较大(30 个文件中 1661 行生产代码),但范围由功能需求驱动——分页贯穿 core、CLI、bridge、SDK 和 web-shell。diff 中没有多余的改动。

可以合入。✅

Qwen Code · qwen3.7-max

Reviewed at 73ddfdebc11eb8d1ee22bb09b3bfd43e233599a8 · re-run with @qwen-code /triage

@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.

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 73ddfde. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

No screenshot changes against the PR base.

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head 73ddfde, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

capabilities

field PR base (before) this PR (after)
features[] "session_transcript_pagination"

Qwen Code · serve A/B

@wenshao

wenshao commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Review — head fe42aaf48

Overview

This PR makes restored Web Shell sessions paint a bounded recent page first (historyPageSize on session/load, default 100 via WEB_SHELL_HISTORY_PAGE_SIZE) and back-fill complete older user turns on scroll-to-top, via a new backward mode on the transcript pager (beforeRecordId + direction: 'backward' cursors). It is capability-gated behind session_transcript_pagination, spans core reader → ACP agent → bridge → serve routes → TS SDK → webui provider → Web Shell UI, and ships a design doc. Overall this is carefully engineered: boundaries are exclusive persisted-record UUIDs rather than timestamps, backward cursors freeze the snapshot, and the failure modes (partial replay, capacity, invalid boundary, snapshot loss) all fail closed without corrupting the displayed transcript.

What I verified (holds up under tracing)

  • Backward page selection (selectBackwardPageUuids): pages tile [0, position) with no gap/overlap; every page starts at a normal user-turn boundary; a byte-capped cut re-aligns forward (tail deferred to the next page) or grows to the complete turn and throws SessionTranscriptPageTooLargeError (mapped to HTTP 413 in error-response.ts). The empty-window edge (beforeRecordId = first record) terminates with hasMore: false.
  • Turn-start predicate: treating every user record except mid_turn_user_message as a boundary is chronologically correct — notification/cron records start fresh turns in the record stream (core client.ts starts a new Turn for them); the stricter exclusion in rebuildTurnBoundaries is about rewind targets, not record order.
  • Initial-page/backward-page continuity: the client derives beforeRecordId from the first replay event carrying _meta.recordId, so records evicted by the bridge byte budget (now record-granular in seedReplayEvents; addReplaySegment no-ops on empty flushes) are fetched by the first backward page — no duplication, no loss. restoreHistoryHasMore covers both the agent-side suffix cut and bridge-side truncation.
  • Prepend safety in the store: store.reset() rebuilds blockIndexById from the seeded array; toolBlockByCallId maps to block id strings, not indices; ordinals are identity-only (blocks render in insertion order), so older blocks with higher ordinals are fine; active streaming pointers come from ...current only. Capacity is checked atomically before the prepend, so a trimmed live store can never produce a gap.
  • Reconnect: a same-session reconnect with a non-empty snapshot resets the store to the fresh initial page and re-derives the boundary, so in-flight/older pages can't duplicate (guarded further by the transcriptHistoryRef identity check).
  • Validation defense-in-depth: cursor/beforeRecordId mutual exclusion is enforced at the route, the ACP agent, and the reader; page-size range enforced at route (400) and agent (RequestError.invalidParams); repeated query params fall out as non-strings → 400.

Findings

1. Bound-route backward pages are unbounded (medium)

GET /session/:id/transcript (bound workspace) forwards to the bridge → ACP agent, which calls reader.readPage with no maxBytes (packages/cli/src/acp-integration/acpAgent.ts:6305), and the route res.json(result)s the bridge result with no response-size guard. With backward paging, limit is now a soft target: a single long agentic turn (hundreds of tool records — common in real sessions) is returned complete in one page, serialized through the ACP wire and Express. Pre-PR this route was bounded by limit × record size; now a pathological turn is unbounded regardless of limit. The workspace-qualified route enforces WORKSPACE_TRANSCRIPT_PAGE_SOURCE_MAX_BYTES and fails closed with 413 — and the UI already handles that cleanly (stops paging, keeps transcript intact). Suggest passing a (generous) maxBytes on the ACP read path too, at least for backward requests, so single-workspace qwen serve deployments get the same fail-closed behavior the PR's test plan step 6 demonstrates for workspace routes.

2. Dead branch in readPage (low)

packages/core/src/services/session-transcript-reader.ts (~line 1094): if (!cursor && direction === 'backward' && options.beforeRecordId === undefined) position = activeUuids.length is unreachable — without a cursor, direction is 'backward' only when beforeRecordId is defined. Either drop it or actually support "backward from the end" (no boundary) as an entry point; as written it reads like supported surface but has no caller and no test.

3. Scroll-anchor restore relies on commit-before-rAF timing (low)

MessageList.loadOlderHistory (packages/web-shell/client/components/MessageList.tsx:2921) captures scrollHeight before await onLoadOlderHistory() and adjusts scrollTop in a requestAnimationFrame after it resolves. That works only if React has committed the prepended blocks before the rAF fires — true today for typical React 18 flush timing, but not guaranteed (long tasks, future concurrent rendering), and when it misses, the viewport silently jumps. Also, the in-flight flag is cleared in finally before the rAF runs, so a fast follow-up load can capture a stale previousHeight. A deterministic alternative: record a pending anchor and apply it in a useLayoutEffect keyed on the first block id changing. Not a blocker — worst case is a one-frame jump.

4. _meta.recordId is unnamespaced (low, contract)

Every other private key this feature adds is namespaced (qwen.session.loadReplayPageSize, qwen.session.loadReplay), but the per-update record identity is plain recordId (history-replay-page.ts replayContext → consumed by getPersistedReplayRecordId in the provider and by seedReplayEvents grouping in the compaction engine). Any other producer stamping _meta.recordId on session updates would be misinterpreted as a persisted-record boundary. Renaming after clients ship is much harder than now — consider qwen.session.recordId (or documenting the reservation in the design doc).

Notes (no action required)

  • Paged-in older history is discarded on SSE reconnect (store reset to the fresh initial page) — the user re-scrolls to fetch it. Consistent with the snapshot model; just a UX tradeoff worth knowing.
  • When the bridge byte budget evicts the head of the initial page, the snapshot can start mid-turn (eviction is record-granular, not turn-granular). The first backward page heals the display; same shape as pre-PR truncation behavior.
  • handleScroll now checks the top threshold before the scrollCooldown early-return, so programmatic smooth scrolls that pass near the top can trigger a load. Guarded and idempotent, so at worst one extra page fetch.

Test coverage

Good breadth: reader backward paging (boundary, soft limit, 413, invalid boundary), agent suffix selection (short/long turn), bridge hasMore/page-size plumbing, record-granular eviction, provider capability-off / prepend / partial-page / capacity paths, MessageList top-trigger dedupe, underfill, resize re-check. Gaps worth closing:

  • No route-level test for the cursor+beforeRecordId mutual-exclusion 400 or for beforeRecordId validation on the workspace-qualified route (only the bound-route forwarding test was added).
  • Provider tests only exercise the first page (beforeRecordId); the cursor-continuation second page (and nextCursor adoption) is untested at that layer.
  • Finding 1's scenario (bound route, complete turn ≫ limit, no byte cap) has no test — adding one would surface the asymmetry.

Security / compat

Cursor HMAC signing is preserved; the new direction field is validated on decode; backward cursors omit replay state so they stay small (and pass the workspace cursor-size cap). Capability negotiation is correct in both directions: old server + new client (no historyPageSize, no paging), new server + old client (fields optional, forward contract byte-for-byte unchanged). beforeRecordId is length-capped at the route and only ever used as an exact-match lookup.

Verdict

Solid design and implementation; the invariants I traced (tiling, turn alignment, dedup across the load/page boundary, fail-closed error paths) all hold. Finding 1 is the one I'd like addressed (or explicitly accepted with rationale) before merge; 2–4 are polish. CI (Test / Serve A/B / web-shell visuals) was still pending at review time.

中文版审查意见

审查 — head fe42aaf48

概述

此 PR 让 Web Shell 恢复会话先渲染有上限的最近页(session/loadhistoryPageSize,Web Shell 默认 100),滚动到顶部时按完整用户轮次向后补齐更早历史(transcript pager 新增 beforeRecordId + direction: 'backward' 游标)。通过 session_transcript_pagination capability 协商,改动贯穿 core reader → ACP agent → bridge → serve 路由 → TS SDK → webui provider → Web Shell UI,并附设计文档。整体工程质量很高:边界用持久化记录 UUID 而非时间戳,向后游标冻结快照,各失败路径(partial replay、容量、非法边界、快照丢失)都 fail-closed,不污染已显示内容。

已验证成立的部分

  • 向后分页选择selectBackwardPageUuids):页面无缝平铺 [0, position),每页起点都是正常用户轮次边界;字节截断会向前重新对齐(尾部留给下一页)或扩展为完整轮次并抛 SessionTranscriptPageTooLargeError(映射为 HTTP 413)。空窗口边界情形正确终止。
  • 轮次起点判定:除 mid_turn_user_message 外的 user 记录都视为边界,在时间序上是正确的 —— notification/cron 在记录流中确实开启新轮次(core client.ts 为其创建新 Turn);rebuildTurnBoundaries 更严格的排除是 rewind 目标语义,与记录顺序无关。
  • 初始页与向后页的衔接:客户端从首个带 _meta.recordId 的 replay 事件推导 beforeRecordId,被 bridge 字节预算淘汰的记录(seedReplayEvents 现在按记录整体淘汰)会被第一次向后分页取回 —— 不重复、不丢失。
  • 前插安全性store.reset() 会从新数组重建 blockIndexByIdtoolBlockByCallId 映射到 block id 字符串而非下标;ordinal 仅作身份用(渲染按插入顺序),活跃流指针只从 ...current 继承;容量在前插前原子校验,不会产生显示空洞。
  • 重连:同会话重连若带非空快照会重置 store 并重新推导边界,配合 transcriptHistoryRef 身份校验,进行中的分页结果不会污染新状态。
  • 校验纵深cursor/beforeRecordId 互斥在路由、ACP agent、reader 三层都强制;页长范围在路由(400)和 agent 双重校验。

发现的问题

1. bound 路由向后分页无字节上限(中)

GET /session/:id/transcript(绑定 workspace)经 bridge → ACP agent 调 reader.readPage没有传 maxBytesacpAgent.ts:6305),路由也无响应大小限制。向后分页使 limit 变为软目标:一个很长的 agentic 轮次(数百条工具记录,实际会话中常见)会作为单页完整返回。改动前该路由受 limit × 单条记录大小 约束;现在与 limit 无关。workspace 路由有 WORKSPACE_TRANSCRIPT_PAGE_SOURCE_MAX_BYTES 并以 413 fail-closed,UI 已能优雅处理。建议 ACP 读取路径(至少向后请求)也传一个宽松的 maxBytes,让单 workspace qwen serve 部署获得与测试计划第 6 步相同的行为。

2. readPage 中的死分支(低)

session-transcript-reader.ts(约 1094 行):!cursor && direction === 'backward' && beforeRecordId === undefined 不可达 —— 无游标时只有 beforeRecordId 存在才会是 backward。建议删除,或真正支持"从末尾向后"并补测试。

3. 滚动锚点恢复依赖 commit 早于 rAF 的时序(低)

MessageList.loadOlderHistoryawait 前记录 scrollHeight,resolve 后在 rAF 中调整 scrollTop,前提是 React 已在 rAF 前提交前插的 DOM。当前 React 18 时序下通常成立,但无保证;失败时视口静默跳动。且 in-flight 标志在 finally 中先于 rAF 清除,快速连续加载可能取到过期高度。更确定的做法:记录 pending anchor,在以首个 block id 变化为键的 useLayoutEffect 中应用。非阻塞项。

4. _meta.recordId 未加命名空间(低,契约)

本特性其他私有键都有命名空间(qwen.session.*),但记录身份用了裸 recordId。其他生产者若在 update 上写 _meta.recordId 会被误判为持久化记录边界。客户端依赖形成后再改名代价大,建议现在改为 qwen.session.recordId 或在设计文档中声明保留。

备注(无需处理)

  • SSE 重连会丢弃已翻入的旧历史(store 重置为新初始页),需重新滚动获取 —— 与快照模型一致的取舍。
  • bridge 字节预算淘汰初始页头部时,快照可能从轮次中间开始(按记录而非轮次粒度淘汰);第一次向后分页即可修复显示,与改动前的截断行为同形。
  • handleScroll 的顶部阈值检查现在在 scrollCooldown 早退之前,程序化平滑滚动经过顶部可能触发一次加载;有幂等保护,最多多取一页。

测试覆盖

覆盖面不错(reader 向后分页各边界、agent 后缀选择、bridge 透传、按记录淘汰、provider 各路径、MessageList 触发/去重/underfill/resize)。建议补充:workspace 路由的 beforeRecordId 校验与互斥 400 的路由级测试;provider 层第二页(cursor 续页)路径;发现 1 场景的测试。

安全 / 兼容

游标 HMAC 签名保留,direction 解码时校验;向后游标不带 replay 状态、体积小;capability 双向协商正确;beforeRecordId 路由层限长且只做精确匹配。

结论

设计与实现扎实,我追踪的不变量(平铺、轮次对齐、加载/分页边界去重、fail-closed)全部成立。建议合并前处理(或给出理由明确接受)发现 1;2–4 为打磨项。审查时 CI(Test / Serve A/B / web-shell visuals)仍在运行。

@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.

Reviewed. Suggestions are inline. Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built. Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/services/session-transcript-reader.ts Outdated
@ytahdn

ytahdn commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the completed review and CI feedback together in 8c1c31140:

  • bounded the bound-session transcript path with the shared 4 MiB page budget and preserved the structured HTTP 413 transcript_page_too_large response through ACP;
  • fixed the Web Shell full-module test mocks that caused the Ubuntu CI failure;
  • removed the unreachable backward-pagination branch;
  • moved prepend scroll-anchor restoration to the layout phase and added an explicit scroll-position regression test;
  • namespaced persisted replay metadata as qwen.session.recordId across producer and consumers.

Verification after the final changes:

  • focused ACP transcript-page error test: passed;
  • focused serve HTTP 413 mapping test: passed;
  • Web Shell App, ChatPane, and MessageList DOM suites: 201 passed;
  • ACP agent and history replay suites: 259 passed;
  • core transcript reader: 44 passed;
  • ACP bridge suites: 458 passed;
  • WebUI session provider: 159 passed;
  • npm run build: passed;
  • npm run typecheck: passed.

The focused affected suites were also independently reproduced and verified. npm run verify:pr was intentionally not run.

qwen-code-ci-bot pushed a commit that referenced this pull request Jul 17, 2026
Comment thread packages/webui/src/daemon/session/DaemonSessionProvider.tsx Outdated
Comment thread packages/web-shell/client/components/MessageList.tsx Outdated
Comment thread packages/cli/src/acp-integration/session/history-replay-page.ts
@ytahdn

ytahdn commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the completed review and the Ubuntu CI failure together in bd6540c01:

  • transient transcript-page failures now preserve hasMore, surface a recoverable notice, and reject to the MessageList so failed loads do not apply a scroll anchor;
  • terminal partial replay and non-retryable 4xx failures still stop pagination;
  • MessageList prevents an underfilled transcript from entering an immediate retry loop, while another upward wheel/touch/keyboard intent at the top retries automatically (including the no-scrollbar case);
  • added backward replay coverage for empty pending tool state, dangling finalization, and cursor replay-state omission;
  • updated the real qwen serve capabilities integration baseline with session_transcript_pagination, which was the only Ubuntu CI failure.

Verification:

  • WebUI DaemonSessionProvider: 160 passed;
  • Web Shell MessageList.dom: 54 passed;
  • ACP history replay page: 4 passed;
  • qwen-serve-routes integration file: 32 passed;
  • npm run build: passed;
  • npm run typecheck: passed.

The findings and CI failure were independently reproduced before the fix. npm run verify:pr was not run.

qwen-code-ci-bot pushed a commit that referenced this pull request Jul 17, 2026
@ytahdn

ytahdn commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up on the wenshao and chiga0 reviews:

  • All four findings in the wenshao head review are addressed: the session transcript route now enforces the shared 4 MiB page budget and returns 413 for oversized pages; the unreachable capacity branch was removed; prepended-history scroll anchoring now runs in the layout phase with a DOM scroll-math regression test; and the persisted cursor metadata is namespaced as qwen.session.recordId.
  • chiga0 minor findings about missing scroll-anchor coverage and permanently disabling pagination after a transient load failure are addressed. Transient failures remain retryable, terminal partial/non-retryable responses stop pagination, failed loads do not apply a stale scroll anchor, and backward replay direction has regression coverage.
  • The duplicate turn-start predicate was intentionally not extracted: the two call sites operate on different representations (ChatRecord and the transcript index), so a shared abstraction would add indirection without establishing a stronger invariant.
  • The remaining edge-test and style items in the AI overview were explicitly non-blocking suggestions. Defensive parsing of daemon event metadata and deterministic ResizeObserver test setup are intentionally retained. Broader stale-session test expansion is deferred to avoid widening this already cross-package PR.

The fixes are in 8c1c31140 and bd6540c01. All fixed inline review threads have now been resolved.

@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.

Reviewed. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output. Not reviewed: PR #7064 adds paginated history loading for restored Web ... — pointed at diff lines it never opened: it made tool calls, but none of them read the diff. Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 10 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 10 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 10 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run. Not reviewed: verification — its prompt was built, but no agent was launched with it that opened its brief, so the posted findings were not verified.

— qwen3.7-max via Qwen Code /review

Comment thread packages/cli/src/acp-integration/session/history-replay-page.ts
Comment thread packages/cli/src/acp-integration/acpAgent.ts
Comment thread packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx
@ytahdn
ytahdn requested a review from wenshao July 17, 2026 06:47
@wenshao

wenshao commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Re-verification — head fa98b2ed4 (local build + live E2E)

Follow-up to my review at fe42aaf48 (#7064 (comment)). The branch gained 8c1c3114 (harden), bd6540c0 (retryable paging) and fa98b2ed (stop empty auto-load loops, merges main). I rebuilt the PR head and the merge-base 30984a2f5 in isolated worktrees and verified both against real daemons.

All 4 findings from the previous review are fixed

Previous finding Status at fa98b2ed4
[medium] bound GET /session/:id/transcript → ACP readPage had no byte cap (one long turn ⇒ unbounded page) Fixed. SESSION_TRANSCRIPT_MAX_PAGE_BYTES (4 MiB) is now shared by both routes; ACP maps SessionTranscriptPageTooLargeError-32012 → HTTP 413 transcript_page_too_large (verified live, see A/B below)
[low] dead branch in readPage (backward && beforeRecordId === undefined) Removed
[low] rAF-based scroll-anchor restore could race the commit Replaced with a useLayoutEffect state-anchored restore (synchronous with the prepend commit)
[low] un-namespaced _meta.recordId Now qwen.session.recordId end-to-end (replay page, compaction engine, provider)

The new commits also add: retryability classification in the provider (5xx/408/429 retryable; 4xx/partial-replay terminal), error-blocked older-history retry only on an explicit user gesture (wheel-up/touch/ArrowUp/PageUp/Home at top), and an underfill auto-load guard that stops empty-page loops — each covered by new unit tests.

What I ran (macOS, Node 22, npm ci builds)

1. Scoped suites at head — all green (1884 tests)

Suite Result
core session-transcript-reader.test.ts 44 ✅
acp-bridge bridge + compactionEngine 458 ✅
cli acpAgent + history-replay-page 261 ✅
cli serve/server.test.ts 752 ✅
webui DaemonSessionProvider 160 ✅
web-shell MessageList.dom + App + ChatPane 209 ✅

2. Live daemon API E2E — 15/15. Built dist qwen serve over a seeded workspace: a 180-record / 60-turn session and a session whose first turn contains a single >4 MiB record. Verified: pagination capability advertised; backward pages always start at a complete user turn; contiguous tiling [0,150) with exact prefix reconstruction in 4 pages; bound (bridge→ACP) backward page turn-aligned with qwen.session.recordId metas; 413 on both routes for the oversized turn (pageBytes=4718936 maxBytes=4194304); cursor+beforeRecordId → 400.

3. Baseline A/B at merge-base 30984a2f5 — identical seeds, identical invocation. Baseline does not advertise the capability, replay events carry no record-boundary metas, beforeRecordId is silently ignored, and the bound route returns the >4 MiB record unbounded in one 4.7 MB response — exactly the failure mode the PR fixes; the PR daemon returns 413 for the same call.

API + A/B evidence

4. Real-browser E2E against the PR daemon's Web Shell — 9/9 (Playwright/Chromium, real HTTP, no mocks).

Restore paints only the bounded recent page: turns 28–60 (99 records ≤ 100-record target), pinned to bottom; turn 1 is not in the DOM:

initial restore

One scroll-to-top fires exactly one request — GET /session/…/transcript?beforeRecordId=u-028&limit=100 → 200 — and turns 1–27 are prepended with the scroll anchor preserved (viewport stays on turn 28, no jump):

after backfill, anchored

Scrolling on up reaches turn 1; at history start no further transcript requests fire (no loop):

oldest turn visible

The >4 MiB-turn session fails safe: initial replay is byte-bounded and says so in-transcript ("History truncated: retained 4, dropped 2 (window 4194304 bytes)"), the older-page request gets 413 and the client stops with a visible notice — the huge text never enters the DOM and there is no retry loop:

giant turn 413 fail-safe

Notes (non-blocking)

  • fa98b2ed4 is a merge commit that also carries the empty-loop fix in its first-parent diff; the PR's net tree vs merge-base is clean (only the 40 pagination files), and a squash-merge makes the point moot.
  • An oversized complete turn is terminal for older history in the UI (hasMore=false + notice). That matches the PR description's documented trade-off, and the bounded initial page keeps the session itself usable.

Verdict: LGTM. All previous findings are fixed and re-verified end-to-end; CI is green at this head.

中文版本(Chinese version)

复验报告 — head fa98b2ed4(本地构建 + 真实 E2E)

继上次在 fe42aaf48 的评审(https://github.com/QwenLM/qwen-code/pull/7064#issuecomment-4998410110)之后,分支新增了 8c1c3114(加固)、bd6540c0(分页可重试)、fa98b2ed(停止空页自动加载循环,并合并了 main)。我在隔离 worktree 中分别构建了 PR head 和 merge-base 30984a2f5,并对两个真实 daemon 做了验证。

上次评审的 4 个发现全部已修复

上次发现 fa98b2ed4 的状态
[中] bound GET /session/:id/transcript → ACP readPage 无字节上限(一个超长 turn ⇒ 单页无界) 已修复。 两条路由现共享 SESSION_TRANSCRIPT_MAX_PAGE_BYTES(4 MiB);ACP 将 SessionTranscriptPageTooLargeError 映射为 -32012 → HTTP 413 transcript_page_too_large(已实测,见 A/B)
[低] readPage 中的死分支 已删除
[低] rAF 滚动锚点恢复存在时序竞争 改为 useLayoutEffect 状态锚定(与前插提交同步)
[低] _meta.recordId 未加命名空间 全链路改为 qwen.session.recordId

新提交还增加了:provider 的可重试分类(5xx/408/429 可重试;4xx/部分回放为终态)、失败后仅在用户显式手势(顶部滚轮上滑/触摸/ArrowUp/PageUp/Home)时重试、以及防止空页自动加载死循环的守卫——均有新单测覆盖。

验证内容(macOS,Node 22,npm ci 构建)

1. head 上范围化测试套件——全绿(共 1884 个):core reader 44 ✅、acp-bridge 458 ✅、cli acpAgent+replay-page 261 ✅、serve server.test 752 ✅、webui provider 160 ✅、web-shell 客户端 209 ✅。

2. 真实 daemon API E2E——15/15。 用构建产物 qwen serve 加载播种工作区:一个 180 条记录/60 轮的会话,以及一个首轮含单条 >4 MiB 记录的会话。验证了:capability 已声明;向后分页每页都从完整用户轮开始;[0,150) 连续平铺、4 页精确重建前缀;bound(bridge→ACP)向后分页轮对齐且带 qwen.session.recordId;超限轮在两条路由均返回 413pageBytes=4718936 maxBytes=4194304);cursor+beforeRecordId → 400。

3. merge-base 30984a2f5 基线 A/B——相同种子、相同启动参数。 基线不声明该 capability、回放事件无记录边界 meta、beforeRecordId 被静默忽略,且 bound 路由把 >4 MiB 记录以 4.7 MB 单页无界返回——正是本 PR 修复的问题;PR daemon 对同一调用返回 413。(见上方 API/A-B 证据图)

4. 真实浏览器 E2E(Playwright/Chromium,真实 HTTP,无 mock)——9/9。

  • 恢复只渲染有界最近页:turn 28–60(99 条 ≤ 100 目标),钉在底部,turn 1 不在 DOM(截图 1);
  • 滚动到顶只发出一次请求 ?beforeRecordId=u-028&limit=100 → 200,turn 1–27 前插且滚动锚点保持(视口停留在 turn 28,不跳动)(截图 2);
  • 继续上滚可见 turn 1;到达历史起点后不再发请求(无循环)(截图 3);
  • 4 MiB 轮会话失败安全:初始回放按字节预算截断并在记录内明示("History truncated: retained 4, dropped 2 (window 4194304 bytes)"),旧页请求得到 413,客户端停止并显示提示——巨型文本从未进入 DOM,也没有重试循环(截图 4)。

备注(不阻塞)

  • fa98b2ed4 是一个在 first-parent diff 中携带修复内容的 merge commit;PR 相对 merge-base 的净 diff 干净(仅 40 个分页相关文件),squash 合并后无影响。
  • 超限完整轮在 UI 中对更早历史是终态(hasMore=false + 提示),与 PR 描述中声明的取舍一致;初始页有界,会话本身仍可用。

结论:LGTM。 上次全部发现已修复并经端到端复验;该 head 的 CI 全绿。

@wenshao

wenshao commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@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.

LGTM, looks ready to ship. ✅

wenshao
wenshao previously approved these changes Jul 17, 2026
@wenshao

wenshao commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts and pushed the branch update.

Merge Conflict Resolution Summary

PR: #7064 — feat(web-shell): paginate restored session history
Base branch: main
Commit: 9959ec009merge: resolve conflicts with origin/main (SSE restart + history pagination)

Conflicts and Resolutions

Three files had content conflicts between the PR branch (history pagination feature) and origin/main (SSE event-stream restart feature). Both features are independent and additive, so every conflict was resolved by keeping both sides.

1. packages/web-shell/client/components/SplitView.tsx

Conflict: The PR branch added suppressOwnUserEcho to <DaemonSessionProvider>, while origin/main added restartEventStreamOnPrompt={restartSseOnPrompt}.

Resolution: Kept both props — they are independent features that coexist on the same component.

2. packages/web-shell/client/components/WorkspaceSessionProvider.tsx

Conflict: Same pattern as SplitView — PR branch added suppressOwnUserEcho, origin/main added restartEventStreamOnPrompt={restartSseOnPrompt}.

Resolution: Kept both props.

3. packages/webui/src/daemon/session/DaemonSessionProvider.tsx

Conflict: The PR branch added transcriptHistoryRef and transcriptHistoryState/setTranscriptHistoryState (for paginated history loading), while origin/main added eventStreamRef (for SSE restart-on-prompt). Both are ref declarations used extensively in non-conflicted code below.

Resolution: Kept both blocks — the transcript history refs (from PR) followed by the event stream ref (from main). Both are referenced throughout the rest of the file in non-conflicted sections that auto-merged correctly.

Files Modified

Only the three conflicted files were touched. No unrelated edits.

@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.

Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run.

— qwen3.7-max via Qwen Code /review

Comment thread packages/webui/src/daemon/session/DaemonSessionProvider.tsx

@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.

Reviewed. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built.

— qwen3.7-max via Qwen Code /review

@ytahdn

ytahdn commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@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.

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 17, 2026
Merged via the queue into QwenLM:main with commit de44c74 Jul 17, 2026
73 checks passed

@doudouOUC doudouOUC 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. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output. Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 11 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 10 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run.

— qwen3.7-max via Qwen Code /review

Comment on lines +2225 to +2230
const retryable =
!terminalFailure &&
(!(error instanceof DaemonHttpError) ||
error.status >= 500 ||
error.status === 408 ||
error.status === 429);

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] Missing test for non-retryable DaemonHttpError in error classification — Failure scenario: the daemon returns 404 because the session was deleted between the load and the transcript page request. The provider should set hasMore = false and stop paging. If someone changes error.status >= 500 to error.status >= 400, a 404 would be treated as retryable, causing the UI to repeatedly attempt loading history from a nonexistent session. Existing tests cover transient errors (generic Error) and terminal failures (partial page), but no test passes a DaemonHttpError with a 4xx status (e.g., 404).

Suggested fix: add a test where getSessionTranscriptPage rejects with a DaemonHttpError (status 404) and assert that history.hasMore becomes false and no further calls are made.

— qwen3.7-max via Qwen Code /review

Comment on lines +168 to +170
if (history.blocks.length + current.blocks.length > maxBlocks) {
return false;
}

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] Capacity check uses original maxBlocks prop instead of the store's possibly-expanded capacity — Failure scenario: a restored session produces exactly 200 replay blocks with maxBlocks = 200. The replayExceededCapacity guard at line ~1040 does NOT fire (since 200 > 200 is false). The user sees "Load more" available. Clicking it loads a history page; prependTranscriptHistory checks 200 + N > 200 which is always true for N >= 1, and returns false. The UI flips to capacityReached = true after the first failed click — a button that appears available but always fails on first use.

Suggested change
if (history.blocks.length + current.blocks.length > maxBlocks) {
return false;
}
if (history.blocks.length + current.blocks.length > current.maxBlocks) {
return false;
}

— qwen3.7-max via Qwen Code /review

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants