Skip to content

docs(web-shell): add global turn navigation Phase 2 design - #11020

Merged
doudouOUC merged 6 commits into
mainfrom
docs/web-shell-turn-navigation-phase2
Sep 4, 2026
Merged

doudouOUC merged 6 commits into
mainfrom
docs/web-shell-turn-navigation-phase2

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

What this PR does

Adds the Phase 2 design document for session-wide turn navigation in Web Shell (#10750): the bounded client data layer that sits between the merged Phase 1 daemon/SDK protocol (#10751) and the Phase 3 virtualized rail UI.

The design covers: a client-side turn-index store that loads the newest metadata page first and pages older metadata independently of transcript content; a provider-owned page ledger that keeps the existing flat transcript store as the render source while adding immutable page boundaries, explicit gaps, and whole-page eviction; an anchored-open action that lands a random-access transcript page at a selected persisted turn without disturbing the live tail; canonical identity plumbing that carries persisted record IDs onto rendered messages and reconciles provisional live entries by exact prompt or record identity rather than by label or timestamp; and the capability-gated fallback that preserves the current loaded-message rail against older daemons and oversized transcripts.

It also records the verified current state of the Web Shell client data layer (store topology, pagination, retention, identity flow, rail derivation), the consumed Phase 1 contract with its full error-code mapping, a five-step migration path whose first three steps are behavior-preserving, the provider/store test plan, and the open questions that need measurement before defaults freeze.

Why it's needed

Phase 1 shipped the server contract (sparse turn index, signed snapshots, anchored reads), but the client cannot consume it yet: the transcript store is a single prepend-only array, eviction is a one-way trim with no gap model, and the rail derives from loaded messages only. Implementing the client data layer directly against code would mix several separable decisions — store topology, identity plumbing, eviction policy, capability fallback — into one large change. This document pins those decisions and their verification story before implementation starts, so the Phase 2 PRs can be reviewed against an agreed contract.

Reviewer Test Plan

How to verify

This is a design-only change; there is no runtime behavior to exercise. Suggested review focus:

  1. Fidelity to the consumed Phase 1 contract — the SDK methods, page/entry shapes, and the error-code table can be checked against the merged Phase 1 implementation.
  2. Consistency with the two upstream designs it builds on: web-shell-global-turn-navigation.md (parent design, three-phase delivery) and web-shell-bounded-transcript-and-subagent-details.md (page-table model).
  3. Coverage of the Phase 2 checklist in feat(web-shell): add session-wide turn navigation #10750, including the fallback and the "never merge non-contiguous ranges" invariant.
  4. The key structural decision — a provider-owned page ledger over the existing flat transcript store instead of a cross-package store rewrite — and whether its rationale (reusing isolated-page materialization, atomic admission, and the trim/re-anchor machinery) holds.
  5. The open questions at the end, which are the decisions deliberately deferred to implementation-time measurement.

Evidence (Before & After)

N/A (documentation only).

Tested on

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

Environment (optional)

N/A — documentation only.

Risk & Scope

  • Main risk or tradeoff: the design commits to a provider-owned page ledger over the existing flat transcript store rather than a store rewrite; the tradeoff and the rejected alternative are documented in the design itself.
  • Not validated / out of scope: no code changes in this PR. Phase 3 rail UI, real-browser E2E, memory-budget measurement, and the two known non-blocking Phase 1 follow-ups (ACP-path anchor length parity; a two-record expansion pinning test) are all out of scope here.
  • Breaking changes / migration notes: none.

Linked Issues

Part of #10750

中文说明

本 PR 做了什么

为 Web Shell 全会话 turn 导航(#10750)补充 Phase 2 设计文档:位于已合并的 Phase 1 daemon/SDK 协议(#10751)与 Phase 3 虚拟化导航条 UI 之间的有界客户端数据层。

设计内容覆盖:一个客户端 turn 索引存储——先加载最新元数据页,旧元数据独立于 transcript 内容分页;一个 provider 持有的页台账——保留现有扁平 transcript store 作为渲染源,同时加入不可变页边界、显式空洞与整页驱逐;一个锚定打开动作——在不干扰 live tail 的前提下,把随机访问的 transcript 页落地到选定的持久化 turn;规范身份管道——把持久化记录 ID 带到渲染出的消息上,并按精确 prompt 或记录身份(绝不按标签或时间戳)对账临时 live 条目;以及能力门控回退——在面对旧 daemon 与超大 transcript 时保持现有的已加载消息导航条。

文档同时记录了 Web Shell 客户端数据层经核实的现状(store 拓扑、分页、保留策略、身份流、导航条推导)、所消费的 Phase 1 契约及其完整错误码映射、前五步零行为变化的迁移路径、provider/store 测试计划,以及在默认值冻结前需要实测的开放问题。

为什么需要

Phase 1 交付了服务端契约(稀疏 turn 索引、签名快照、锚定读取),但客户端尚无法消费:transcript store 是单一只能前插的数组,驱逐是没有空洞模型的单向裁剪,导航条只能从已加载消息推导。直接对着代码实现客户端数据层,会把若干可分离的决策——store 拓扑、身份管道、驱逐策略、能力回退——混进一个大改动。本文档在实现开始前固定这些决策及其验证方案,使后续 Phase 2 的 PR 可以对照一份已对齐的契约评审。

Reviewer 测试计划

如何验证

这是纯设计变更,没有可运行的行为。建议评审重点:

  1. 对所消费 Phase 1 契约的保真度——SDK 方法、页/条目结构、错误码表,均可对照已合并的 Phase 1 实现核查。
  2. 与其依赖的两份上游设计的一致性:web-shell-global-turn-navigation.md(父设计,三阶段交付)与 web-shell-bounded-transcript-and-subagent-details.md(页表模型)。
  3. feat(web-shell): add session-wide turn navigation #10750 中 Phase 2 清单的覆盖度,包括回退路径与「绝不合并非连续区间」这一不变量。
  4. 关键结构决策——在现有扁平 transcript store 之上做 provider 页台账,而非跨包重写 store——及其理由(复用隔离页物化、原子 admission 与 trim/重锚定机制)是否成立。
  5. 文末的开放问题,即被刻意推迟到实现期实测的决策。

证据(Before & After)

N/A(纯文档)。

测试平台

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

环境(可选)

N/A——纯文档。

风险与范围

  • 主要风险或权衡:设计选择在现有扁平 transcript store 之上做 provider 页台账,而非重写 store;该权衡与被否决的替代方案已在设计文档内说明。
  • 未验证/范围外:本 PR 无代码改动。Phase 3 导航条 UI、真实浏览器 E2E、内存预算实测,以及两个已知非阻塞的 Phase 1 遗留项(ACP 路径锚点长度对齐;跨两条记录的扩展钉测)均不在此范围。
  • 破坏性变更/迁移说明:无。

关联 Issue

#10750 的一部分

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR — this is a re-run at f32bcbb5, so I've re-gated it and folded the earlier findings in below.

Template looks good ✓ — every required heading is present, all three Risk & Scope bullets are filled in, and the Chinese translation is complete rather than abridged.

Problem: not a bug report, and it doesn't need to be — this is the design doc AGENTS.md asks for before non-trivial work. I re-checked the gap independently at today's main rather than taking the description's word for it: there is still not one reference to getTurnIndexPage or session_turn_navigation anywhere under packages/web-shell/, and client/adapters/ has zero occurrences of promptId or sourceRecordIds. Phase 1 (#10751) really did merge this morning at 06:34Z as 8a0a9c6, and #10750 is still open with all eight Phase 2 boxes unticked. Evidence-backed, not theoretical.

Direction: aligned. docs/design/ is exactly where this belongs, and it sits between two committed designs and an open tracking issue. One thing I want to call out as good practice: the doc pins the commit its facts were verified against (80497a74d0). I confirmed that SHA is a real main commit three ahead of the Phase 1 merge with nothing behind it, so "includes the merged Phase 1" is literally true — and it's why the citations still land even as main moves underneath them. No direct reference in the claude-code CHANGELOG for turn/session navigation, but the area is plainly ours: Web Shell is a first-party surface and today's rail is coupled to transcript retention.

Size: not applicable. Two markdown files, +676/−5 — zero production, test, or generated lines, and no core paths, so the Stage 0 two-tier gate never engages.

Approach: the scope feels right and the central call survived hard pushing — keep the SDK store flat and add a provider-owned page ledger instead of a cross-package store rewrite. My own instinct would still be a page-table store, but the doc's version is cheaper and it now states the reuse boundary honestly instead of overclaiming: it names that applyTranscriptHistory is prepend-only (verified — blocks: [...history.blocks, ...current.blocks] is hardcoded at DaemonSessionProvider.tsx:502) and that mid-window insertion therefore needs a new admission variant, rather than implying the existing machinery already covers it.

The prior pass deferred at 3/5 on three items — the 413 error row, the two-outcome vs three-outcome admission mismatch, and the phase-boundary contradiction with the parent design. All three are now resolved; I verified each against the code in Stage 2 rather than accepting the round summaries. The parent-design fix went the right way too: #10750's own checklist puts the whole data layer in Phase 2 and leaves Phase 3 rail-only, so moving those four items up aligns the parent with the issue that authorises it.

One note for whoever picks this up next, not a request: this PR has now been through five review rounds, so per AGENTS.md only correctness-level findings should land here from this point — the remaining nits I list in Stage 2 belong in the first Phase 2 implementation PR, not in another round on this one.

Risk: no elevated risk signals. Stage 1e matched nothing — both changed paths are under docs/design/web-shell/, and none of the revert-correlated files are touched.

Moving on to code review. 🔍

中文说明

感谢贡献 —— 这是在 f32bcbb5 上的一次 re-run,我重新过了 gate,并把之前的发现合并到下面。

模板完整 ✓ —— 所有必需小节都在,Risk & Scope 三条都填了,中文翻译是完整的而不是节略版。

问题: 这不是 bug 报告,也不需要是 —— 这正是 AGENTS.md 要求在做非平凡工作前先写的设计文档。我没有只采信 PR 描述,而是在今天的 main 上重新核实了它所说的缺口:packages/web-shell/ 下依然没有任何一处引用 getTurnIndexPagesession_turn_navigationclient/adapters/ 目录里 promptIdsourceRecordIds 均为 0 次出现。Phase 1(#10751)确实今早 06:34Z 以 8a0a9c6 合并,而 #10750 仍处于 open 状态、Phase 2 的八个方框一个都没打勾。是有证据支撑的,不是理论性加固。

方向: 对齐。docs/design/ 正是它该在的位置,并且它衔接两份已提交的设计与一个 open 的跟踪 issue。有一点我想单独指出为好实践:文档固定了它核实事实所依据的 commit(80497a74d0)。我确认该 SHA 是真实的 main commit,领先 Phase 1 合并三个提交、落后为零,所以"包含已合并的 Phase 1"字面成立 —— 这也正是即使 main 在其下移动、引用依然能落到位的原因。claude-code 的 CHANGELOG 里没有 turn/session 导航的直接对应条目,但这个方向显然是我们自己的:Web Shell 是第一方面板,而今天的导航条与 transcript 保留策略是耦合的。

规模: 不适用。两个 markdown 文件,+676/−5 —— 没有生产代码、测试或生成代码行数,也不触及核心路径,所以 Stage 0 的两级 gate 根本不会启动。

方案: 范围合理,而且最关键的那个判断经受住了用力推敲 —— 保持 SDK store 扁平、在其之上加一个 provider 持有的页台账,而不是跨包重写 store。我自己的直觉仍然会是做页表 store,但文档这个版本更省,并且它现在诚实地写明了复用边界,而不是过度声称:它点名 applyTranscriptHistory 只能前插(已核实 —— blocks: [...history.blocks, ...current.blocks] 硬编码在 DaemonSessionProvider.tsx:502),因此中段插入需要一个全新的 admission 变体,而不是暗示现有机制已经覆盖。

上一轮 triage 以 3/5 延期,理由是三条 —— 413 错误行、两结果与三结果 admission 不一致、以及与父设计的阶段边界矛盾。三条现在都已解决;我是在 Stage 2 里逐条对着代码核实的,没有采信各轮的处置摘要。父设计那一处的修法方向也是对的:#10750 自己的清单就把完整数据层放在 Phase 2、Phase 3 只留导航条 UI,所以把这四项上移是让父设计与授权它的 issue 对齐。

给接下来接手的人一个提示,不是要求:这个 PR 已经过了五轮 review,所以按 AGENTS.md,此后只应收 correctness 级的发现 —— 我在 Stage 2 列出的剩余小项应该进 Phase 2 的首个实现 PR,而不是在本 PR 上再开一轮。

风险: 无升级风险信号。Stage 1e 没有命中任何一项 —— 两个改动路径都在 docs/design/web-shell/ 下,没有触及任何与 revert 相关的高风险文件。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Code review

Re-run at f32bcbb5. Before reading the diff I wrote down again what I'd expect a Phase 2 client design to contain: a turn-index cache keyed by snapshot with an explicit staleness rule, some way to land a mid-history page without corrupting the live tail, identity carried from persisted records onto rendered messages, and a hard fallback when the daemon is older than the capability. The doc delivers all four, and its structural choice is still better than my baseline — I would have reached for a page-table store and forked the SDK's indexing/batching/trim machinery, whereas the ledger-over-flat-store shape reuses three mechanisms I re-confirmed exist: isolated-page materialization (materializeTranscriptHistory, DaemonSessionProvider.tsx:310), reset-based atomic admission (store.reset(applyTranscriptHistory(...)), :4228-4231), and prefix trim with re-anchoring.

The point of this pass was the three items that held the last one at 3/5. All three are genuinely fixed, not reworded — I checked each against the code rather than against the round summaries.

1. The 413 row is now correct, and the matrix no longer inherits the error. The table describes transcript_page_too_large as the serialized-response cap and says outright that it is thrown by both route families and is not anchor-specific. That matches: WORKSPACE_TRANSCRIPT_RESPONSE_MAX_BYTES = 2 * SESSION_TRANSCRIPT_MAX_EXPANDED_PAGE_BYTES = 32 MiB (routes/session.ts:276-277, the 16 MiB constant at session-transcript-reader.ts:100), thrown from the single serializeWorkspaceTranscriptResponse guard (session.ts:672), which has four call sites including both turn-index routes (:5167 and :5272, each immediately after getSessionTurnIndexPage). The new footnote is the part that actually closes the old finding, and its arithmetic holds: entries are capped at 500 per page (SESSION_TRANSCRIPT_MAX_LIMIT, session-transcript-reader.ts:86) with previews hard-truncated to 160 code points for the label (:1130) and 240 for the detail (:1153), so an index page lands near ~250 KB — two orders below the cap, and the conclusion survives even if that estimate is off several-fold. Both consequences the old matrix had are gone: the index store explicitly has no 413 branch of its own and treats a 413 from an index fetch as a generic transient failure, and the "turn too large" notice is now scoped in the matrix to an anchored transcript fetch only, so it can no longer fire on a cursor page containing no turn. The other four rows are exact too — 400/400/409/413/413 for invalid_transcript_cursor, invalid_turn_anchor, transcript_snapshot_unavailable, transcript_page_too_large, transcript_too_large matches error-response.ts:428-469 code for code.

2. Anchored admission now mirrors the real three-outcome contract. openTranscriptAtTurn returns page_too_large / window_full / window_impossible as distinct reasons, each mapped to the code path it comes from: the server's 413, the retryable capacityReached + rejectedPage latch, and the terminal impossible branch. All three citations land — impossible is computed at DaemonSessionProvider.tsx:410-411, the terminal throw is at :4194-4206, and the footprint latch at :4209-4216 carries a code comment saying precisely what the doc says it does (re-offer only once enough capacity exists for this page, or streaming trims churn fetch/reject/re-render). The doc also now flags window_impossible as the branch an anchored random jump is most likely to hit, since a single aggregate record can outrun any window, and requires it to stay an explicit tested outcome — that was the substance of the old finding, and it's in the verification plan as its own bullet.

3. The phase-boundary contradiction is resolved on both sides, and in the right direction. The parent design moves the turn-index store, tail refresh, provisional reconciliation and canonical locator map up into Phase 2 as item 4, renumbers Phase 3 to the rail UI, and adds a dated re-alignment note that points at this document as the detailed design; this doc carries a matching note. I checked which document should have moved rather than assuming: #10750's own Phase 2 checklist has eight items covering the complete data layer, and its Phase 3 is rail rendering, selection UX, keyboard/a11y and browser E2E only. So the parent was the one out of line with the issue that authorises it, and the edit aligns it. Coverage is 1:1 — each of the eight checklist items maps onto a Goal, including the "never merge non-contiguous ranges" invariant (the ledger's non-overlap/exact-cover invariant plus the gap model) and the capability/ceiling fallback.

Beyond those three, I spot-checked the claims added in the later rounds, since nothing before this pass had reviewed them. They hold:

  • Anchor combination rules (routes/session.ts:4801-4813) match the doc exactly — cursor must be sent alone, beforeRecordId may carry a snapshot, a bare snapshot is rejected, and atRecordId requires one. The signed cursor really does self-bind (cursorPayload at session-transcript-reader.ts:654-672 carries file identity, snapshot size, position, leaf), which is why "sent alone" is safe.
  • Anchored reads continue forward: hasMore at :3477-3480 is nextPosition < index.replayUuids.length unless direction === 'backward', and the minted cursor omits direction in that case. The doc's forward-tail claim is right.
  • The boundary == 0 no-request guard is correct, and correctly reasoned: the seed start is options.start ?? Math.max(0, totalTurns - Math.min(limit, totalTurns)) (:2565), so any session with at most limit durable turns seeds at ordinal 0, and the clamped limit = boundary - start would then be 0 — which parseTranscriptLimit rejects with 400 invalid_transcript_limit (session.ts:505-512). Suppressing the request is the only right answer, and the doc says why.
  • The three gap resolvers are necessary, not decorative. I grepped for afterRecordId across every package: zero hits. There is genuinely no "page after record X" operation, so the newer side of a gap really does have to be resolved by a stored forward cursor, a re-anchor, or a newer-neighbour beforeRecordId backfill.
  • Identity gap is real: client/adapters/ has zero occurrences of promptId or sourceRecordIds, so migration step 3 is plumbing both fields, exactly as the doc now states.
  • Capability registered at capabilities.ts:144; WEB_SHELL_HISTORY_PAGE_SIZE = 200 at client/constants/sessions.ts:22; the two-place feature-constant split the doc proposes mirrors the existing SESSION_TRANSCRIPT_PAGINATION_FEATURE pattern (provider-local at DaemonSessionProvider.tsx:203-206, App-facing in constants/sessions.ts:15).

On citation discipline, one observation worth recording because it changes how the rest of this doc should be read: the round-1 drift complaints are structurally fixed, not just patched. The doc pins 80497a74d0 and its line numbers are accurate at that SHA. I verified the case that looked most suspicious — transcript.ts:2075-2076 for nextOrdinal being a monotone id counter rather than a position. On today's main that code sits at :2156-2157; at the pinned SHA it is allocateBlockId at exactly 2075-2076. The ~100-line difference is main moving, not the doc being wrong.

Three non-blocking nits, all of which belong in the first implementation PR rather than another round here:

  • DaemonHttpError { status, body.code } is runtime-accurate but glosses the typing: body is readonly body: unknown (DaemonHttpError.ts:18) and the SDK's convention is a narrow per-contract guard built on getErrorBodyRecord (isSessionLevelNotFound and friends in the same file), so five new codes means five guards. One line in the matrix would save an implementer the surprise.
  • The historyPageSize gate is summarised as capability-only. The send site is cited correctly now (DaemonSessionProvider.tsx:1751-1756), but it also requires (!restoreSessionId || restoreMode === 'load'), replaySource !== 'memory', and a defined historyPageSizeRef.current. Worth deciding during implementation whether those three gates apply to index seeding too, since the seeding rule mirrors the simpler form.
  • The Phase 1 follow-up "a pinning test for the two-record anchored-expansion case" still reads as missing coverage; session-transcript-reader.test.ts already pins an exactly-two-record anchored expansion. If the real gap is narrower, naming it would keep the follow-up from inviting duplicate coverage. (The other follow-up — ACP-path atRecordId length parity with the route's 200-char cap — is accurate.)

Testing

This is an unattended run, so no PR-derived code was built or executed; the evidence below is the PR's own CI on the reviewed commit, read through the API. Real-scenario tmux testing is N/A — a markdown-only diff has no runtime surface to drive. There is also no behavioural claim for a sandboxed lane to settle, which the verification lane independently reached on its own (/verify recorded ⚠️ not run / n/a on this PR: nothing to execute).

Both pull_request workflow runs completed green with nothing pending. The 61 skipped checks are path-filtered — for a docs-only diff the macOS and Windows unit legs, the CLI integration leg and the web-shell E2E smoke are all expected to skip, while the ubuntu leg carries lint, format, typecheck and the unit suite. The two review-pr checks still in progress are pull_request_target bot orchestration, not PR CI, and do not gate this.

Not verified: the ~239 B/turn wire figure in the performance model, which the doc attributes to Phase 1 maintainer measurement on #10751 (300 turns, a limit=500 page serialising to ~72 KB). Re-measuring it would mean running code, which this review does not do, and no lane would settle it from a markdown diff — the doc already labels it an order-of-magnitude figure rather than a guarantee, which is the honest framing.

Check Conclusion
Test (ubuntu-latest, Node 22.x) success
Lint & Static (ubuntu-latest, Node 22.x) success
Integration Tests (no-AK, No Sandbox) success
Desktop Shell (ubuntu-22.04) success
Desktop Shell (windows-2022) success
Dependency CVE audit success
Secret scan (TruffleHog) success
Classify PR success
Test (macos-latest, Node 22.x) skipped
Test (windows-latest, Node 22.x) skipped
Integration Tests (CLI, No Sandbox) skipped
web-shell E2E Smoke (ubuntu-latest, Node 22.x) skipped
review-pr (bot orchestration, not PR CI) in_progress
中文说明

代码审查

f32bcbb5 上的 re-run。看 diff 之前我又写下了自己对 Phase 2 客户端设计的预期:一个按 snapshot 归键、带明确失效规则的 turn 索引缓存;一种能把历史中段页落地而不破坏 live tail 的机制;把持久化记录身份带到渲染消息上的管道;以及 daemon 老于 capability 时的硬回退。四点文档都交付了,而且它的结构性选择仍然比我的基线更好 —— 我原本会去做页表 store,从而分叉 SDK 的索引/批处理/裁剪机制;而"扁平 store 之上加台账"复用了三个我重新确认存在的机制:隔离页物化(materializeTranscriptHistoryDaemonSessionProvider.tsx:310)、基于 reset 的原子 admission(store.reset(applyTranscriptHistory(...)):4228-4231)、以及带重锚定的前缀裁剪。

本轮的重点是上次把结论压在 3/5 的那三条。三条都是真正修好了,不是改了措辞 —— 我是对着代码核实的,没有采信各轮的处置摘要。

1. 413 那一行现在是对的,错误矩阵也不再继承这个错误。 表格把 transcript_page_too_large 描述为序列化响应上限,并直接写明它由两个路由族共同抛出、不是锚定专属。这与代码一致:WORKSPACE_TRANSCRIPT_RESPONSE_MAX_BYTES = 2 * SESSION_TRANSCRIPT_MAX_EXPANDED_PAGE_BYTES = 32 MiB(routes/session.ts:276-277,16 MiB 常量在 session-transcript-reader.ts:100),由唯一的 serializeWorkspaceTranscriptResponse 守卫抛出(session.ts:672),该函数有四个调用点,其中包含两个 turn-index 路由(:5167:5272,都紧跟在 getSessionTurnIndexPage 之后)。真正闭合旧发现的是新增的脚注,而它的算术成立:每页条目上限 500(SESSION_TRANSCRIPT_MAX_LIMITsession-transcript-reader.ts:86),预览被硬截断为 label 160 码点(:1130)、detail 240 码点(:1153),因此一个索引页约 ~250 KB —— 比上限低两个数量级,而且即使这个估算偏差数倍,结论依然成立。旧矩阵的两个后果都消失了:索引存储明确没有自己的 413 分支,索引请求返回的 413 按普通瞬时失败处理;而"turn too large"提示在矩阵里已被限定为仅锚定 transcript 请求,因此不会再在一个不含 turn 的 cursor 页上触发。另外四行也精确 —— 五个码的 400/400/409/413/413 与 error-response.ts:428-469 逐一对应。

2. 锚定 admission 现在与真实的三结果契约一致。 openTranscriptAtTurnpage_too_large / window_full / window_impossible 作为三个不同的 reason 返回,每个都映射到它的来源代码路径:服务端 413、可重试的 capacityReached + rejectedPage latch、以及终态 impossible 分支。三处引用都落到位 —— impossibleDaemonSessionProvider.tsx:410-411 计算,终态 throw 在 :4194-4206,footprint latch 在 :4209-4216,而且那里的代码注释说的正是文档所说的(只有当容量足够容纳这一页时才重新尝试 admission,否则流式裁剪会反复 fetch/reject/re-render)。文档还点明 window_impossible 恰恰是锚定随机跳转最可能撞上的分支(单条聚合记录可以超出任何窗口),并要求它保持为显式、被测试的结果 —— 这正是旧发现的实质,如今已进入验证计划成为独立一条。

3. 阶段边界矛盾在两侧都解决了,而且方向是对的。 父设计把 turn 索引存储、tail 刷新、临时条目对账与规范 locator map 上移为 Phase 2 第 4 项,把 Phase 3 重编号为导航条 UI,并加了一条带日期的重对齐说明指向本文档作为详细设计;本文档也带了对应的说明。我核实了应该改哪一份,而不是默认:#10750 自己的 Phase 2 清单有八项、覆盖完整数据层,而它的 Phase 3 只有导航条渲染、选择 UX、键盘/无障碍与浏览器 E2E。所以与授权 issue 不一致的是父设计,这次编辑让它对齐了。覆盖是 1:1 的 —— 八个清单项各自对应一个 Goal,包括"绝不合并非连续区间"不变量(台账的非重叠/精确覆盖不变量加空洞模型)与 capability/上限回退。

除这三条之外,我还抽查了后几轮新增的陈述,因为此前没有任何一轮审过它们。都成立:锚定组合规则(routes/session.ts:4801-4813)与文档逐条一致,签名 cursor 确实自绑定(cursorPayloadsession-transcript-reader.ts:654-672),锚定读取确实向前续页(:3477-3480hasMore),boundary == 0 不发请求的守卫推理正确(种子 start 公式在 :2565,clamp 后的 limit 会是 0,被 parseTranscriptLimit 以 400 invalid_transcript_limit 拒绝,session.ts:505-512)。三个空洞解析器不是装饰:我在所有 package 里 grep afterRecordId,零命中,协议确实没有"X 之后的页"这个操作。身份缺口是真实的:client/adapters/promptIdsourceRecordIds 均为零次出现。capability 注册在 capabilities.ts:144WEB_SHELL_HISTORY_PAGE_SIZE = 200constants/sessions.ts:22;文档提议的两处常量声明确实复用了现有 SESSION_TRANSCRIPT_PAGINATION_FEATURE 的模式。

关于引用规范,有一点值得记下来,因为它改变了这份文档其余部分该怎么读:第一轮的行号漂移抱怨是被结构性地修好了,而不只是补了一下。文档固定了 80497a74d0,其行号在该 SHA 上是准确的。我核实了看起来最可疑的一处 —— transcript.ts:2075-2076nextOrdinal 是单调 id 计数器而非位置)。在今天的 main 上这段代码位于 :2156-2157;而在固定的那个 SHA 上,2075-2076 正好是 allocateBlockId。约 100 行的差异来自 main 移动,不是文档写错。

三条非阻塞小项,都应该进首个实现 PR,而不是在这里再开一轮:DaemonHttpError { status, body.code } 在运行时准确但略过了类型(bodyunknown,SDK 惯例是基于 getErrorBodyRecord 的窄守卫,五个码就是五个守卫);historyPageSize 的门被概括为仅 capability,而发送点还要求另外三个条件,值得在实现时决定索引 seeding 是否同样适用;以及"两条记录锚定展开的 pinning 测试"这个 Phase 1 后续项读起来仍像是缺覆盖,而 session-transcript-reader.test.ts 已经固定了恰好两条记录的锚定展开 —— 如果真实缺口更窄,点名它可以避免重复覆盖。

测试

这是一次无人值守的运行,因此没有构建或执行任何来自 PR 的代码;下面的证据是 PR 自己在被审 commit 上的 CI,通过 API 读取。真实场景 tmux 测试为 N/A —— 纯 markdown diff 没有可驱动的运行时表面。也没有任何行为性主张需要沙箱通道来判定,验证通道自己也独立得出了同样结论(本 PR 上 /verify 记录为 ⚠️ 未运行 / 不适用:没有可执行对象)。

两个 pull_request workflow run 均已绿色完成,无 pending。61 个 skipped 检查是路径过滤的结果 —— 对纯文档 diff,macOS 与 Windows 单测腿、CLI 集成腿与 web-shell E2E smoke 都应跳过,而 ubuntu 腿承载 lint、format、typecheck 与单测套件。仍在进行的两个 review-prpull_request_target 的 bot 编排,不是 PR CI,不构成 gate。

未核实:性能模型里 ~239 B/turn 的线上数字,文档将其归于 #10751 上的 Phase 1 维护者实测(300 个 turn,limit=500 的页序列化约 72 KB)。重新测量意味着运行代码,本次审查不做;而对一个 markdown diff,也没有通道能判定它 —— 文档已把它标为数量级参考而非保证,这是诚实的写法。

Qwen Code · qwen3.8-max-2026-09-02

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — the three things that held the last pass at 3/5 are genuinely fixed and I verified each against the code; what's left is three non-blocking nits that belong in the first implementation PR, not another round here.

Stepping back, the reason this PR exists is still the right one, and it has not weakened since the first pass: Phase 1 landed a complete server contract this morning and the client cannot consume a byte of it. I re-confirmed that at today's main instead of trusting the earlier finding — zero references to getTurnIndexPage or session_turn_navigation under packages/web-shell/, zero promptId/sourceRecordIds in the adapters directory, #10750 open with all eight Phase 2 boxes unticked. Writing the client layer straight against code would have mixed store topology, identity plumbing, eviction policy and capability fallback into one unreviewable diff, and AGENTS.md asks for exactly this artifact instead.

My independent baseline was a page-table store, which is what the parent design literally said. The ledger-over-flat-store shape is cheaper and I still think it's the better call, mainly because the doc now states its reuse boundary honestly: it names that applyTranscriptHistory is prepend-only and that mid-window insertion needs a new admission variant, instead of implying existing machinery already covers it. That kind of sentence is what separates a design doc from a sales pitch.

On the re-run risk — a fifth-round re-trigger is exactly where a gate rubber-stamps, and the author's disposition table is a very readable reason to. So I deliberately did not read the fixes through it. I re-derived all three from source: the 32 MiB cap and its four call sites including both turn-index routes, the 500-entry/160/240-code-point bounds behind the new footnote, the impossible / capacityReached+rejectedPage / admitted three-way split at DaemonSessionProvider.tsx:410-411, :4194-4206 and :4209-4216, and which of the two designs was actually out of line with #10750. Then I went after the material no earlier pass had seen — everything added in rounds two through five — because that is where a fresh defect would be hiding. The anchor-combination rules, the cursor's self-binding, forward continuation on anchored reads, the boundary == 0 guard's reasoning, and the absence of any afterRecordId operation all held up. I was trying to find a reason to hold this at 3/5 again and could not.

If I were maintaining this in six months I would thank whoever wrote it. The "Current client state (verified)" section is what makes the rest reviewable, and pinning the baseline commit turned the round-1 line-drift complaints from a recurring chore into a non-issue: I checked the citation that looked most wrong (transcript.ts:2075-2076) and it is exact at the pinned SHA — today's main simply moved ~100 lines underneath it. Every change in the diff is necessary too; the parent-design edit is not drive-by, it is the half of the fix that stops two committed designs contradicting each other.

Why 4 and not 5: DaemonHttpError.body is unknown, so the doc's { status, body.code } shorthand hides five narrow guards an implementer will have to write; the historyPageSize gate is summarised as capability-only when the send site also requires three other conditions, and the index-seeding rule mirrors the simpler form; and the "two-record anchored-expansion pinning test" follow-up still reads as missing coverage that session-transcript-reader.test.ts already has. None of those change what the design decides, which is the bar at round five — but they are real, so I'm naming them rather than calling it clean.

For the record on volume: this author has two open PRs, both design docs from today. I evaluated this one on its own evidence and the other gets its own pass.

CI is settled green on this commit with nothing pending, so I'm approving pinned to f32bcbb5 rather than deferring to the finalize job.

中文说明

Confidence: 4/5 —— 上次把结论压在 3/5 的三条都真正修好了,我逐条对着代码核实过;剩下的是三条非阻塞小项,应该进首个实现 PR,而不是在这里再开一轮。

退一步看,这个 PR 存在的理由依然成立,而且比第一轮时没有变弱:Phase 1 今早合并了完整的服务端契约,而客户端一个字节都消费不了。这一点我在今天的 main 上重新确认过,没有沿用早先的结论 —— packages/web-shell/ 下没有任何 getTurnIndexPagesession_turn_navigation 引用,adapters 目录里没有任何 promptId/sourceRecordIds#10750 仍 open 且 Phase 2 八个方框全未打勾。直接对着代码写客户端层,会把 store 拓扑、身份管道、驱逐策略与 capability 回退混进一个无法评审的 diff,而 AGENTS.md 要求的正是这种设计文档。

我的独立基线是页表 store —— 那正是父设计字面上的要求。"扁平 store 之上加台账"这个形态更省,我仍然认为它是更好的判断,主要原因在于文档现在诚实地写明了复用边界:它点名 applyTranscriptHistory 只能前插、中段插入需要一个新的 admission 变体,而不是暗示现有机制已经覆盖。正是这种句子把设计文档和推销稿区分开。

关于 re-run 的风险 —— 第五轮的重新触发恰恰是 gate 最容易盖章放行的地方,而作者的处置表是一个非常好读的理由。所以我刻意没有透过它来读这些修复。我从源码重新推导了全部三条:32 MiB 上限及其四个调用点(含两个 turn-index 路由)、新脚注背后 500 条目 / 160 / 240 码点的界、DaemonSessionProvider.tsx:410-411:4194-4206:4209-4216 处的 impossible / capacityReached+rejectedPage / admitted 三分支,以及两份设计中究竟哪一份与 #10750 不一致。然后我去查了此前没有任何一轮审过的材料 —— 第二到第五轮新增的全部内容 —— 因为新的缺陷只可能藏在那里。锚定组合规则、cursor 的自绑定、锚定读取的向前续页、boundary == 0 守卫的推理、以及 afterRecordId 操作确实不存在,全部都站得住。我本来想找一个理由把它再压在 3/5,没找到。

如果六个月后由我来维护,我会感谢写它的人。"Current client state (verified)" 那一节是让其余部分变得可评审的原因,而固定基线 commit 把第一轮的行号漂移抱怨从一件反复出现的杂务变成了非问题:我核实了看起来最错的那处引用(transcript.ts:2075-2076),在固定的 SHA 上它是精确的 —— 只是今天的 main 在其下移动了约 100 行。diff 里每一处改动也都是必要的;父设计那次编辑不是顺手夹带,它是让两份已提交设计不再互相矛盾的那一半修复。

为什么是 4 而不是 5:DaemonHttpError.bodyunknown,所以文档 { status, body.code } 的简写掩盖了实现者必须写的五个窄守卫;historyPageSize 的门被概括为仅 capability,而发送点还要求另外三个条件,且索引 seeding 规则照抄了较简形式;以及"两条记录锚定展开的 pinning 测试"这个后续项读起来仍像是缺覆盖,而 session-transcript-reader.test.ts 已经有了。这些都不改变设计所做的决定 —— 那才是第五轮的标准 —— 但它们是真实存在的,所以我点名,而不是称它干净。

关于数量的记录:这位作者有两个 open PR,都是今天的设计文档。我按本 PR 自己的证据评审,另一个会有它自己的一轮。

本 commit 上的 CI 已稳定为绿、无 pending,所以我把批准固定到 f32bcbb5,而不是交给 finalize 任务延后处理。

Qwen Code · qwen3.8-max-2026-09-02

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

📄 Docs-only change — the automatic review ran at --effort medium (verified findings, no reverse audit; medium posts no inline comments). Outcome:

Review complete: pr-11020 — Comment, not posted (0 Critical, 1 Suggestion)

Reviewed head: f32bcbb5c9d18260ed51bcfd789ab20a3be607b3. Full report in the workflow run. For a full high-effort review with inline comments, comment @qwen-code /review.

中文说明

📄 纯文档变更 —— 自动评审以 --effort medium 运行(发现已验证、无反向审计;medium 不发布行内评论),结果见上方引用行。评审的 head:f32bcbb5c9d18260ed51bcfd789ab20a3be607b3。完整报告见 workflow 运行;如需带行内评论的完整高强度(high-effort)评审,请评论 @qwen-code /review

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Phase 2 设计文档 review

对照文档自称的核实基线 origin/main (80497a74d0) 逐条校验了 file:line 声明与协议契约。该 SHA 确实等于当前 origin/main HEAD 且是本 PR 的 merge-base,所以所有引用都可精确验证。

总体:设计方向站得住,Phase 1 契约引用绝大部分精确,#10750 的 Phase 2 清单 8/8 覆盖。以下 7 处实质问题建议合并前处理,其中前两条会直接把实现带偏。

已验证正确的承重事实(避免后续重复核查):

  • 协议契约:entry/page 形状、start 需要 snapshotsession.ts:5197-5203)、首调返回最新页(reader.ts:2564-2565)、targetRecordId/hasOlderreader.ts:3528-3529)、200 字符 anchor 上限(session.ts:614)、256 MiB ceiling(reader.ts:89)、limit 200 ≤ max 500(reader.ts:82)、getTurnIndexPageDaemonSessionClient.ts:988)、capability(capabilities.ts:144)——五个错误码及 HTTP 状态逐一对得上
  • 现状调研:web-shell 零 turn-index 引用、无页/gap 模型、adapter 确实丢弃 sourceRecordIds、测试 fixture 确实没有 targetRecordId/hasOlder、500 块 + 15 s idle reload(MessageList.tsx:3921-3975 行号精确)、1160 px 门槛、split pane 传 hideSessionTimeline
  • 精确命中的引用:materializeTranscriptHistory 310、DaemonTranscriptHistory 162、onTruncation 874、createDaemonTranscriptStore store.ts:24reduceDaemonTranscriptEvents transcript.ts:194DaemonTranscriptState types.ts:1124、block sourceRecordIds types.ts:912transcriptBlocksToDaemonMessages 378、getSessionTimelineEntries 1194、SessionTimeline 2537。

所有问题都不涉及代码改动,改完即可合。

Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

All review findings addressed in e2388de5 (docs-only, prettier clean). 全部评审意见已在 e2388de5 处理完毕。

Finding / 发现 Disposition / 处置
Phase-boundary contradiction with the parent design(阶段边界矛盾) Both artifacts synced: this doc gains a phase-boundary note in Status, and the parent design's delivery plan is updated in this same PR — turn-index store / tail refresh / reconciliation / locator map moved to Phase 2, matching the #10750 checklist. 两份文档已同步。
Snapshot request shape(cursor+snapshot → 400;锚定 nextCursor 为前向) Contract bullet rewritten: beforeRecordId+snapshot legal; cursor must be sent alone (frozen state lives inside the signed cursor); anchored continuation is forward. Cites session.ts:4801-4813, reader.ts:654-672, 3477-3495.
transcript_page_too_large 是 32 MiB 整体响应上限且两个路由族都会抛 Contract table corrected; degradation matrix now splits an index-fetch 413 (store error + backoff, no turn notice) from an anchored-fetch 413 (jump rejected with notice).
Admission 需保留三结果契约 openTranscriptAtTurn now distinguishes page_too_large (server) / window_full (retryable; carries capacityReached+rejectedPage latch semantics) / window_impossible (terminal, the admission.impossible branch — called out as the branch random jumps are most likely to hit).
复用论证夸大(admission 只能头插) Reuse scoped to isolated materialization + atomic budget admission + prefix trim/re-anchor; the doc now requires a new insert-at-ledger-position admission variant, and notes nextOrdinal is a counter so mid-window insertion keeps id uniqueness.
Snapshot 不变量与 append-only 保留冲突 Per-page snapshot is now the read authority; the store-level snapshot only seeds/fetches older pages; multi-snapshot retention documented as protocol-legal.
页网格未定义(重叠 → 重复 tick) Grid pinned non-overlapping: older fetches clamp limit against the smallest covered ordinal so pages butt exactly; ensurePage(ordinal) takes the largest non-overlapping slice of the uncovered interval.
行号漂移(8 处) All applied; the pagination anchor now documents the 3-level fallback chain.
"never by text" 与 boundary-echo 文本去重 Prompt-id matching subsumes the boundary-echo case when both sides carry identity; the text comparison stays only in the capability-off / legacy path.
promptId 也不存在于 message 层 Current-state and Files-affected now state both sourceRecordIds and promptId are new on Message.
能力常量实为两处声明 Documented: provider-local constant (provider does not import constants/sessions.ts) + the App-facing constant.
~239 B/turn 无出处 Attributed to the #10751 maintainer verification (300 real turns, limit=500 page ≈ 72 KB), marked order-of-magnitude.
缺 409 重播种测试用例 Added to the lifecycle group of the verification plan (stale-snapshot recovery).

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

第二轮 review(5f8f0c4e2388de5

上轮 12 条已逐一对照代码复验,全部修复,且多处比我提的要求更深入。特别记一笔:413 的机制是你查对了、我上轮说错了——真实来源是路由序列化器的 32 MiB 整响应上限(WORKSPACE_TRANSCRIPT_RESPONSE_MAX_BYTES = 2 * SESSION_TRANSCRIPT_MAX_EXPANDED_PAGE_BYTESroutes/session.ts:276-277;唯一抛出点 :680serializeWorkspaceTranscriptResponse 包了 4 个调用点,含两个 turn-index 路由 5167/5272),reader 自己从不抛。我原先引的 reader 4/16 MiB 选择预算是错的机制。

父设计的 delivery plan 同步更新得很干净(带日期与理由说明),两份文档不再互相矛盾。

本轮对全文做了开放式通读,新增 6 条。其中 N1 是唯一会在实现阶段立刻卡住的(「协议里没有这个 API」),N2 是常见路径上的语义空洞,建议这两条修完再合;N3-N6 可一并带上。

prettier --check 我对两个文件实跑过,均通过——所以 N4 那处内容损坏 CI 抓不到。

Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Round-2 findings addressed in 3c7838ca (docs-only, prettier clean). 第二轮 6 条全部处理完毕,每条先对照代码复核再改。

# Finding / 发现 Disposition / 处置
N1 TranscriptGap.newerafterRecordId 协议里不存在(全仓零匹配,已复核) TranscriptPageLedgerEntry 新增 nextCursor?: string(仅前向读且 hasMore 的页才携带,backward prepend 永不产生);TranscriptGap.newer 改为 { cursor: string } 并注明 cursor 自带冻结绑定、必须单发;同时写明通用兜底——gap 老侧页被驱逐(cursor 随之丢失)或纯 backward 留下的 gap,一律用 turn-index store 选 gap 内某个导航 turn 发 atRecordId + 该页 snapshot 重新锚定。
N2 append-only 刷新的部分重叠(350/450 算例)与「Pages never overlap」冲突 采用建议 (a) 两步刷新:① 无 snapshot 取最新页做 append-only 校验(按重叠 ordinal 比 turnId);② 校验通过后,用新 snapshot 发 clamped 填补请求(start = 已覆盖上界+1,limit = totalTurns - start,超一页则按页迭代)使新页精确落回网格;校验响应本身因必然重叠而不入库。「Pages never overlap」已限定为合并完成后的稳定态不变量。
N3 索引侧 413 不可达(limit ≤ 500 × label 160 / detail 240 截断 ≈ ≤250 KB ≪ 32 MiB,已复核 reader.ts:86/1130/1153 删除错误矩阵中的索引 413 行;机制说明保留在契约表,并新增脚注:两类路由共用同一序列化守卫,但索引页受协议约束永远撞不到上限,索引 fetch 的 413 按通用瞬时失败处理。
N4 prettier 把 + 改成 -,渲染成悬空子列表 改写为 "(the existing batcher and the structural snapshot gating)",运算符不再落行首;已复跑 prettier 确认稳定。
N5 两处跨行 code span 续行零缩进 entry 形状收成单行(prettier 不拆 code span);验证计划里的状态码移出 span 外,span 保持单行。
N6 行号口径两处 :501:502DaemonTranscriptHistoryContext 两处统一为 :655-657

另:上轮关于 413 机制的修正(路由序列化器 32 MiB 整响应上限,session.ts:276-277,4 个调用点含两个 turn-index 路由 5167/5272)已按你第二轮确认的口径保留在契约表与脚注中。

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

第三轮 review(e2388de53c7838ca

上轮 6 条全部已修,两处修得比我要求的更完整:TranscriptGap 现在把「older 邻居被驱逐、cursor 随之丢失」的回退也写进去了;两步 merge 把 validation 响应「永不 admit」这一条明确了,我拿算例走过——保留 0-149 / 150-349 加 fill 350-449,连续且无重叠。

我上轮有两处引用是错的,你改对了,记录在此避免后续被我的注释误导:

  1. SESSION_TRANSCRIPT_MAX_LIMIT 在审查基线 80497a74d0 上是 :86,不是我写的 :82。我那条 grep 打在了本地 worktree(HEAD 停在更早的 80b5dab3e0,该文件在那里确实是 82 行)而不是基线——正是我在上轮反复强调的规矩,自己犯了。文档现在的 :86 正确。
  2. 「code points」比我写的「chars」准确:compactPreviewText(text, maxCodePoints)Array.from(compacted) 按码点切(session-transcript-reader.ts:1092-1097)。

prettier --check 对两个文件实跑过,均通过。

本轮对全文再做一遍开放式通读,新增 3 条,都不大但确定:R1 会直接产生一个错误请求(一行判断即可修),R2 是行为退化(一句 LRU 规则),R3 是措辞精度。三条带上后我认为即可合。

Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Round-3 findings addressed in 701c0fd4 (docs-only, prettier clean). 第三轮 3 条全部处理,关键断言已对照代码复核(limit < 1 → 400 于 routes/session.ts:504-509start == totalTurns 合法返回空页;beforeRecordId+snapshot 为合法组合)。

# Finding / 发现 Disposition / 处置
1 两步刷新在无新增导航 turn 时 limit = totalTurns - start = 0 → 400,且落在 prompt terminal 热路径 Step 2 增加前置条件:若最大已覆盖 ordinal 已是 totalTurns - 1(仅到达被排除的记录类型,或两次 refresh 合并),跳过整个填补请求,仅采纳新 snapshot/totalTurns;并注明 limit 永不为 0。
2 LRU 允许驱逐最新页 → 下次刷新零重叠被误判 divergent → 清空仍有效的旧页 采纳 (a):LRU 中钉住最新页(永不驱逐),刷新永远有重叠可供 turnId 校验;零重叠只在真实大幅重写或快照失效(另有路径)时出现,不再把「无法验证」与「已分叉」混为一谈。
3 「universal resolver」不覆盖整段落在单个长 turn 内的 gap(gap 内无导航 turn 可锚) Resolver 列表扩为三项:新增 resolver 3——gap 内无导航 turn 时,改由较新邻居反向回填:beforeRecordId = <该页 firstRecordId> + 该页 snapshot,newest-to-oldest 走回 gap 内。同时把原「universal」表述改为三个 resolver 各自的分工说明。

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

第四轮 review(3c7838ca701c0fd4

上轮 3 条全部已修,修法都对:step 2 的跳过判断、最新页 pin(连带写清了「零重叠此后只出现在真正的大幅重写」这个推论)、resolver 扩为三项且第 3 项的 beforeRecordId + snapshot 组合合法性我核过。

prettier --check 两文件实跑通过。21 条 threads 里 16 条已因行变动 outdated,剩余 5 条我逐一核过都已在前几轮落地——GitHub 的 flag 跟的是锚定行而不是修复,不必据此判断。

本轮新增 2 条,都不触及结构决策:S1 是刷新分类少一个出口(一句话可修),S2 是文档自身的测试计划没跟上三轮的新规则。

收敛判断:四轮新发现量级 12 → 6 → 3 → 2,方案主体我认为已经稳定。这两条修完即可合,不必再开新一轮。其中 S2 建议在本 PR 内补齐——测试计划是这份设计文档的交付物之一,留到实现 PR 就失去了「先对齐契约、再对照契约评审实现」的意义。

Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md Outdated
Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Round-4 findings addressed in 741cad15 (docs-only, prettier clean). 第四轮 2 条处理完毕。

# Finding / 发现 Disposition / 处置
S1 跳过条件只有「等于」:尾部截断(largest covered > totalTurns - 1)会掉进 fill 分支,算出负数 limit 吃 400,且采纳了新 totalTurns 而页表仍持有已被截掉的 ordinal——状态不一致。validate 请求刻意不带 snapshot,所以不会失败、只会静默误判 Step 2 改为三路比较:== 保持跳过;> 明确路由到 divergent(仍持有活动链已不存在的 ordinal 即按定义是重写),落进已有的「清空页表 + admit validation 响应」分支;< 才发 clamped fill。divergent 分支措辞同步扩为「divergent, zero overlap, or shrunk」。
S2 Verification plan 自 v1 起没跟上三轮新规则,窄于 Goal 7 的「all of the above」 已在本 PR 内补齐:两步 merge 的四个分支(validation 永不入库 / clamped fill 落格 / == 跳过 / > 走 divergent)与分块迭代;最新页 pin;older 页非重叠 clamp 与 ensurePage(ordinal) 区间计算;三种 gap resolver(含 cursor 丢失后的 atRecordId 重锚、单长 turn 空洞的 newer 侧反向回填);三态尺寸失败(page_too_large / window_full 足迹闩锁 / window_impossible);适配器同时带出 sourceRecordIdspromptId 的身份铺管测试。

收敛确认:四轮新发现 12 → 6 → 3 → 2,方案主体稳定。CI 全绿。

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

第五轮 review(701c0fd4741cad15

上轮 2 条都已修。S1 的三路比较每条都有出口,还把「validate 请求刻意不带 snapshot,所以截断不会让请求失败、只会静默误判」这个理由写进了文档;divergent 分支标题也同步为「divergent, zero overlap, or shrunk」。S2 我逐条对照了上轮列出的 6 项缺口——两步 merge 的四个子情形、newest-page pin、非重叠 clamp 与 ensurePage(ordinal)、三种 gap resolver(逐个点名)、新增的 admission sizing 一项、新增的 identity plumbing 一项——全部覆盖

prettier --check 通过;markdown 结构也扫了一遍(行首 +/-、跨行 code span)无残留问题。

本轮只剩 1 条:T1,是刚修好那个问题在 older 路径上的对称遗漏,同样一句话可修。

收敛判断:五轮新发现 12 → 6 → 3 → 2 → 1,且本轮这条不涉及任何结构决策。方案主体(扁平 store + provider 页台账、每页 snapshot 权威、三种 gap resolver、三路刷新合并、三态尺寸失败)我已反复对照 80497a74d0 上的实际协议与客户端代码核过,一致。

结论:T1 修完即可合。 另按 AGENTS.md 的 review-round 约定(本 PR 已到第 5 轮,此后只收 correctness 级修复),Open question 5 那条 Suggestion 我单独留了一条评论作为延期记录,不建议在本 PR 处理。

Comment thread docs/design/web-shell/web-shell-global-turn-navigation-phase2.md
Comment on lines +655 to +657
5. Interior-page eviction commits via `store.reset` over retained pages —
measure commit cost at the 200-block page size; if visible, consider a
range-delete store method in the SDK as a follow-up.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

延期记录(Suggestion,不建议在本 PR 处理)。

Q5 目前只覆盖 interior-page evictionstore.reset 提交的成本。但第二轮新增的 insert-at-ledger-position admission variant(见「The reuse boundary is honest」那段)走的是同一套 store.reset over retained pages 机制,而且它的延迟可见性更高——锚定插入是用户主动点击跳转触发的,而中间页驱逐按文档自己的说法是「rare, bounded」。所以真正需要实测 commit 成本的是插入路径,Q5 却没提它。

建议把 Q5 扩成「interior-page eviction 与锚定插入 都经 store.reset 提交——一并实测 commit 成本;若可见,再考虑在 SDK 加 range-delete / splice 方法」。

按 AGENTS.md 的约定,本 PR 已到第 5 轮 review,此后只应收 correctness / security / data-loss / regression 级别的修复。这条是 Suggestion,建议延到 Phase 2 首个实现 PR,或跟随 Q1 的窗口预算实测一起处理。在此留痕,以免被静默丢掉。

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Round-5 findings addressed in f32bcbb5 (docs-only, prettier clean). 第五轮处理完毕。

# Finding / 发现 Disposition / 处置
T1 older 路径 clamp 缺 boundary > 0 前置条件:boundary == 0limit = boundary - start = 0 → 400。且这是多数会话的默认状态(种子页 max(0, totalTurns - min(limit, totalTurns))totalTurns ≤ limit 时直接算出 start=0),与 fill 路径刚修的 S1 同类同形状 已修:older 请求仅当 boundary > 0 才发起,== 0 表示最旧 turn 已覆盖、无更旧页可取,不发请求;ensurePage(ordinal) 明确为已覆盖即 no-op;种子页公式补全为含内层 min 的完整形态并加注说明。验证计划同步补上 boundary == 0 no-request guard 与 ensurePage no-op 两条测试项。
T2 Q5 只覆盖 interior-page eviction 的 commit 成本,未提第二轮新增的 insert-at-ledger-position admission variant(同走 store.reset,且由用户点击触发、延迟可见性更高) 接受延期:按 AGENTS.md 的 5 轮约定(此后只收 correctness 级修复),该 Suggestion 不在本 PR 处理。延期去向:随 Phase 2 首个实现 PR 的插入路径实测,或并入 Q1 窗口预算测量。评审者已在 #11020 线程留痕,不会静默丢失。

收敛确认:五轮新发现 12 → 6 → 3 → 2 → 1,本轮唯一 correctness 项已修。CI 全绿,评审者结论为「T1 修完即可合」。

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

第六轮 review(741cad15f32bcbb5)— 收尾

本轮 0 条新发现。这份文档我认为可以合了。

上轮 2 条的处置

T1 已修,且比我要求的多修了一处:

  • 加上 boundary > 0 前置条件,并把理由写进文档(boundary == 0 表示最旧 turn 已保留、不存在更旧页,clamp 会算出 0 → 400);
  • 顺手把种子页公式从 max(0, totalTurns - limit) 纠正为 max(0, totalTurns - min(limit, totalTurns)),与 session-transcript-reader.ts:2564-2565 完全一致,并点明「≤ limit 个 durable turn 的会话一开始就 seed 在 ordinal 0」——这正是该 bug 的高频触发前提;
  • ensurePage(ordinal) 增加「已覆盖则 no-op」,把并发重复触发那条路径也堵住了;
  • 测试计划同步补了 boundary == 0 no-request guard 与 ensurePage no-op 两项。

我复核了守卫后的算式:boundary > 0limit = boundary - max(0, boundary - limit) 恒 ≥ 1,边界退化已消除。

Q5 按建议延期,文本未动(已 diff 确认),延期理由留在对应行级 thread 里。

prettier --check 对两个文件实跑通过。

六轮累计核查结论

新发现量级:12 → 6 → 3 → 2 → 1 → 0

已逐条对照 80497a74d0 上的实际代码验证并确认无误的部分:

  • 协议契约:五个错误码及其 HTTP 状态、32 MiB 整响应序列化上限(2 × SESSION_TRANSCRIPT_MAX_EXPANDED_PAGE_BYTES,唯一抛出点 routes/session.ts:680,两类路由共用)、start 需伴随 snapshotcursor 必须单发、beforeRecordId 可与 snapshot 同发、锚定读取返回前向 cursor、200 字符 anchor 上限、256 MiB indexing ceiling、limit ≤ 500
  • 结构决策:扁平 store + provider 页台账(含「现有 admission 仅能头插、需新增按台账位置插入变体」这一诚实的复用边界)、每页 snapshot 为读取权威、三种 gap resolver 覆盖 cursor 丢失与无导航 turn 两类空洞、三路刷新合并(equal/greater/smaller 均有出口)、三态尺寸失败与现有 capacityReached/impossible 语义对齐、能力门控的双声明点。
  • 测试计划:现已覆盖六轮辩论出的全部规则,包括两步 merge 的四个子情形、newest-page pin、非重叠 clamp 与两个退化守卫、三种 resolver、admission sizing 三态、身份铺管双字段。
  • 父设计:delivery plan 已重新对齐 #10750 清单,两份文档不再互相矛盾。

唯一遗留事项见我在 pinned-base 那一行的评论:origin/main 已前进 4 个 commit 导致部分行号引用漂移——但承重事实经当前 main 复验仍然成立,且我建议不要在本 PR 追。

Comment on lines +21 to +22
Facts about current code were verified against `origin/main` (`80497a74d0`),
which includes the merged Phase 1.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

引用基线漂移记录 —— 承重事实已复验仍成立,建议不要在本 PR 追。

origin/main 在这几轮里已前进 4 个 commit(现为 c8595fc9d8)。其中 7f7bce3174 feat: chat transcript mr2a html export (#10076) 改动了本文档引用的 6 个文件,使约 9 处行号相对当前 main 漂移:

符号 文档 当前 main 漂移
getSessionTimelineEntries 1194 1084 −110
SessionTimeline 2537 2435 −102
scheduleTranscriptReload 3921 3819 −102
nextOrdinal 计数器 2075 2157 +82
trimTranscriptState 1708 1788 +80
DaemonTranscriptState 1124 1168 +44
extractSourceRecordIds 1015 1032 +17
transcriptBlocksToDaemonMessages 378 380 +2
createBase 653 650 −3

ui/types.ts 内其余引用整体后移约 +44。未受影响、引用仍精确的包括 routes/session.tserror-response.tscapabilities.tsconstants/sessions.tsDaemonSessionProvider.tsxapplyTranscriptHistory 仍是 502)、DaemonSessionClient.ts:988SESSION_TRANSCRIPT_MAX_LIMIT:86

关键复验:那条承重事实——sourceRecordIdspromptId 都还没到 message 层——在当前 main 上仍然成立,两个文件各 0 匹配。所以迁移 step 3、Files affected、以及测试计划里「neither exists there today」这句依然正确。那个 +342 行的改动是 HTML 导出功能,与身份铺管无关。

建议不追:这一行已经明确把引用钉在一个声明过的 commit 上,所以没有任何一句是假的——这本身就是正确的做法。而 main 还会继续动,每 rebase 一次引用就要重刷一次,正是 AGENTS.md 提醒的「每轮 review 都在扩大 diff」的循环。按第 5 轮之后只收 correctness 级修复的约定,这条应当延后;真要处理,最好的时机是 Phase 2 首个实现 PR——那时人本来就要对着当时的 main 读代码。

在此留痕,以免后续 reviewer 把行号漂移误当成新缺陷、或误以为承重事实已失效。

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ⚠️ not run — n/a - workflow run

This PR changes documentation/assets only — there is no code to execute, so a sandboxed verification has nothing to verify.

中文 — 判定:⚠️ 未运行 · 不适用

该 PR 仅改动文档/静态资源,没有可执行的代码,沙箱验证没有验证对象。

Qwen Code · sandboxed verification

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@qwen-code /verify

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ⚠️ not run — n/a - workflow run

This PR changes documentation/assets only — there is no code to execute, so a sandboxed verification has nothing to verify.

中文 — 判定:⚠️ 未运行 · 不适用

该 PR 仅改动文档/静态资源,没有可执行的代码,沙箱验证没有验证对象。

Qwen Code · sandboxed verification

@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 — the three items from the earlier pass are fixed and verified against the code, and CI is green on this commit. Ready to ship. ✅

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

Scope: Scan tier — two markdown files only. No code changes; no runtime behavior to exercise.
Not reviewed: DaemonSessionProvider.tsx line-number cross-references (207 KB, 5 159 lines; individual offsets too large to spot-check per-line at this tier). Line references in the document are guidance pointers rather than machine-processed anchors, so errors there carry bounded low impact.


Checked:

  • Claim matches change: PR states docs-only; diff confirms two .md files only ✓
  • CONTRIBUTING.md (base 80497a74): no AI-review ban ✓
  • Key constants cited in the design verified against 80497a74d0:
    • WEB_SHELL_HISTORY_PAGE_SIZE = 200 at constants/sessions.ts:22
    • WEB_SHELL_TRANSCRIPT_RELOAD_BLOCKS = 500 at constants/sessions.ts:23
    • WEB_SHELL_MAX_TRANSCRIPT_BLOCKS = 50_000 at constants/sessions.ts:40
    • transcriptBlocksToDaemonMessages function at adapters/transcriptToMessages.ts:378
    • sourceRecordIds/promptId genuinely absent from message adapter (grep confirms zero hits) ✓
    • session_turn_navigation capability at capabilities.ts:144
  • Phase boundary realignment: items moved from Phase 3→Phase 2 in parent design exactly match what the Phase 2 doc declares; Phase 3 renumbering is consistent ✓
  • Cross-check against prior reviews: author completed 6 explicit rounds fixing 20+ previously-identified issues (protocol shape, gap resolver coverage, boundary==0 guard, LRU newest-page pin, two-step merge divergent exit, verification plan completeness). CI bot approved after round 6. No external-reviewer findings remain unreconciled.

No blocking findings. No approval blockers.

Reviewed with AI assistance.

@doudouOUC
doudouOUC added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 419e8d5 Sep 4, 2026
118 checks passed
@doudouOUC
doudouOUC deleted the docs/web-shell-turn-navigation-phase2 branch September 4, 2026 16:24

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

Design-doc review pass on head f32bcbb. The Phase 2 doc is well-grounded: the consumed Phase 1 contract and anchor/error-code combinations match the shipped routes (snapshot+record-anchor pairing rules, forward-only cursor continuation, 413 semantics), the decision to keep the SDK store flat and add a provider page ledger avoids fighting the append/prepend-only store, and the phase-boundary reconciliation with the parent doc is handled in the same PR. No blocking findings; all checks are green.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.23.1.

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