Skip to content

feat(web-shell): add turn navigation phase 2 client data layer - #11143

Closed
doudouOUC wants to merge 1 commit into
QwenLM:mainfrom
doudouOUC:feat/web-shell-turn-navigation-phase2
Closed

doudouOUC wants to merge 1 commit into
QwenLM:mainfrom
doudouOUC:feat/web-shell-turn-navigation-phase2

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

What this PR does

Implements the Phase 2 client data layer of the Web Shell global turn navigation design (docs/design/web-shell/web-shell-global-turn-navigation-phase2.md, merged in #11020) on top of the Phase 1 daemon/SDK protocol (#10968, #11047).

The change introduces three cooperating pieces, all gated behind the session_turn_navigation capability and invisible when the daemon does not advertise it. First, a provider-owned page ledger over the flat transcript store records which block ranges arrived as which fetched page, plus the explicit gaps between them, so that trimming or rewinding the window leaves re-fetchable locators instead of untracked holes. Second, a session-wide turn-index store loads the newest metadata page first, pages older metadata independently of transcript blocks, keeps the tail fresh with a two-step merge on prompt terminal (append-only fast path, conservative reset on any divergence), bounds memory with an LRU that never shrinks the advertised turn count, and latches an explicit unsupported state when the daemon reports its indexing ceiling. Third, rendered messages now carry their persisted identity (sourceRecordIds / promptId), which feeds a canonical turn locator and record-id/prompt-id deduplication.

On top of those, the session actions gain an anchored open (openTranscriptAtTurn) and two continuations (older via beforeRecordId + snapshot, newer via the stored signed cursor sent alone). An anchored page is materialized, deduplicated, and admitted atomically at its ledger position without disturbing the connected live tail, with deliberately distinct failure reasons — daemon-side whole-response refusal, retryable client-window rejection, terminal window overflow, snapshot invalidation, and missing anchor — so the Phase 3 UI can react to each appropriately.

Existing behavior is unchanged: the load-older path, pagination anchoring, capacity handling, and rendering all work exactly as before, which the full provider test suite confirms.

Why it's needed

Long-running daemon sessions are the norm in the Web Shell, but today the transcript window can only grow sequentially from the newest edge: reaching an early turn means paging through everything in between, and once the window evicts old content there is no way back. Issue #10750 tracks making turns randomly addressable. Phase 1 shipped the daemon-side turn-index and anchored-read protocol; this PR lands the client-side data layer that consumes it — the page ledger, the turn-index store, and the anchored-admission actions that Phase 3's navigation rail will be built on.

Reviewer Test Plan

How to verify

This is a data-layer change with no UI surface of its own; verification is unit tests plus confirming existing transcript behavior is untouched.

  1. cd packages/web-shell && npx vitest run — the full client suite passes (6072 tests, including 56 new ones covering the ledger, the turn-index store, and the anchored-open/continuation actions).
  2. npx vitest run client/daemon/session/DaemonSessionProvider.test.tsx — the 295-test provider suite passes unmodified apart from the new turn navigation (Phase 2) describe block, demonstrating the load-older path, capacity handling, trimming, and session lifecycle are behavior-preserving.
  3. Optionally, open any Web Shell session against a current daemon and use it normally (scroll, load older history, send prompts, rewind): nothing in the visible behavior should change, because the new stores only observe and record alongside the existing flows.

Key behaviors a reviewer can spot-check in the new tests: older-page requests clamp to a butted limit and never fire when the boundary is 0; the tail validation response is never admitted while clamped fill pages land on the grid; a shrunk or rewritten chain resets conservatively; the unsupported latch sticks for the session; anchored opens leave the live tail untouched and dedupe by record id then prompt id; continuation requests send exactly the locator the protocol allows (snapshot pair vs. standalone cursor).

Evidence (Before & After)

N/A — no user-visible UI change in this phase (the navigation rail lands in Phase 3).

Tested on

OS Status
🍏 macOS ⚠️
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

Unit tests only (vitest under packages/web-shell), plus npm run build and npm run typecheck from the repository root.

Risk & Scope

  • Main risk or tradeoff: two new provider-owned stores now live alongside the transcript store and are reconciled on every trim/rewind/admission; a reconciliation bug could desync the ledger from the rendered window. The blast radius is contained to Phase 3 consumers — nothing in the current render path reads the new state — and the reconciliation invariants are pinned by dedicated tests (gap alignment, gaps-length invariant, displaced-gap preservation).
  • Not validated / out of scope: the navigation rail UI, App-level capability wiring, and gap sentinel row rendering are explicitly Phase 3 and intentionally absent. A known heuristic limitation: an anchored page containing no navigation turns uses an insertion-position heuristic that can misplace it visually; noted for Phase 3. The chunked tail-fill loop is defensive and unreachable in production conditions.
  • Breaking changes / migration notes: none. Daemons without session_turn_navigation get byte-identical behavior; the new session actions return unsupported without issuing requests.

Linked Issues

Part of #10750 (Phase 2 of 3; does not close the tracking issue). Design docs: #11020, #10968. Protocol: #11047.

中文说明

本 PR 做了什么

按照已合并的 Phase 2 设计文档(docs/design/web-shell/web-shell-global-turn-navigation-phase2.md#11020 并入),在 Phase 1 的 daemon/SDK 协议(#10968#11047)之上实现 Web Shell 全局 turn 导航的客户端数据层。

改动由三部分协作完成,全部由 session_turn_navigation 能力门控,daemon 未声明该能力时完全不可见。其一,provider 持有的页台账记录在扁平 transcript store 上哪些 block 区间来自哪一次抓取,以及区间之间的显式 gap,使窗口裁剪或 rewind 后留下的是可重新抓取的定位器而不是无法追踪的空洞。其二,会话级 turn-index store 优先加载最新元数据页,独立于 transcript block 向前翻页,在 prompt 终态用两步合并保持尾部新鲜(仅追加走快速路径,任何分歧都保守重置),用永不缩减通告 turn 总数的 LRU 限制内存,并在 daemon 报告索引上限时闩锁为显式 unsupported 状态。其三,渲染消息现在携带其持久身份(sourceRecordIds / promptId),支撑规范的 turn 定位器和按 record id / prompt id 的去重。

在此之上,会话动作新增锚定打开(openTranscriptAtTurn)和两个方向的 continuation(older 用 beforeRecordId + 快照,newer 用单独发送的存储签名游标)。锚定页经物化、去重后按台账位置原子入账,不扰动已连接的实时尾部;失败原因刻意区分——daemon 整页拒绝、可重试的客户端窗口拒绝、终态窗口溢出、快照失效、锚点缺失——便于 Phase 3 的 UI 分别处理。

现有行为不变:load-older 路径、分页锚定、容量处理与渲染均与之前完全一致,完整的 provider 测试套件证实了这一点。

为什么需要

长时间运行的 daemon 会话在 Web Shell 中是常态,但目前 transcript 窗口只能从最新边缘顺序增长:要到达早期 turn 必须逐页翻完中间所有内容,而窗口一旦驱逐旧内容就无法回头。issue #10750 跟踪让 turn 可随机寻址的目标。Phase 1 交付了 daemon 侧 turn-index 与锚定读协议;本 PR 落地消费该协议的客户端数据层——页台账、turn-index store,以及 Phase 3 导航条将构建其上的锚定入账动作。

评审者测试计划

如何验证

这是没有自身 UI 的数据层改动;验证手段为单元测试加上确认现有 transcript 行为未受影响。

  1. cd packages/web-shell && npx vitest run — 客户端全套测试通过(6072 项,含覆盖台账、turn-index store 与锚定打开/continuation 动作的 56 项新增)。
  2. npx vitest run client/daemon/session/DaemonSessionProvider.test.tsx — 295 项 provider 套件除新增 turn navigation (Phase 2) 分组外未经修改即通过,证明 load-older 路径、容量处理、裁剪与会话生命周期保持行为不变。
  3. 可选:对着当前 daemon 打开任意 Web Shell 会话并正常使用(滚动、加载更早历史、发送 prompt、rewind):可见行为不应有任何变化,因为新 store 只是伴随现有流程做观察与记录。

评审者可在新测试中抽查的关键行为:older 页请求会 clamp 到贴合的 limit,边界为 0 时不发请求;尾部 validate 响应永不被入账,而 clamped fill 页落在网格上;链缩短或被改写时保守重置;unsupported 闩锁在会话内持续生效;锚定打开不扰动实时尾部,并按 record id 然后 prompt id 去重;continuation 请求只发送协议允许的定位器(快照配对 vs 独立游标)。

证据(前后对比)

N/A —— 本阶段没有用户可见的 UI 变化(导航条在 Phase 3 落地)。

测试平台

Linux 已测试;macOS、Windows 未测试。

环境(可选)

仅单元测试(packages/web-shell 下的 vitest),外加仓库根目录的 npm run buildnpm run typecheck

风险与范围

  • 主要风险或取舍:两个新的 provider 持有的 store 与 transcript store 并存,并在每次裁剪/rewind/入账时对账;对账缺陷可能使台账与渲染窗口失同步。爆炸半径仅限 Phase 3 的消费者——当前渲染路径不读取新状态——且对账不变量由专门测试锁定(gap 对齐、gaps 长度不变量、被挤位 gap 保留)。
  • 未验证 / 超出范围:导航条 UI、App 层能力接线与 gap 哨兵行渲染明确属于 Phase 3,有意缺席。已知启发式限制:不含导航 turn 的锚定页使用插入位置启发式,可能造成视觉错位,已记录留待 Phase 3。分块尾部填充循环为防御性代码,生产条件下不可达。
  • 破坏性变更 / 迁移说明:无。未声明 session_turn_navigation 的 daemon 获得逐字节一致的行为;新会话动作返回 unsupported 且不发出请求。

关联 Issue

属于 #10750 的一部分(三阶段中的 Phase 2,不关闭该跟踪 issue)。设计文档:#11020#10968。协议:#11047

Implements the Phase 2 client data layer of the Web Shell global turn
navigation design (docs/design/web-shell/web-shell-global-turn-navigation-phase2.md,
part of QwenLM#10750), on top of the Phase 1 daemon/SDK protocol:

- Provider-owned transcript page ledger with explicit gap tracking over the
  flat block store, so eviction leaves re-fetchable locators instead of
  untracked holes; trim/rewind reconciliation preserves gap alignment.
- Session-wide turn-index store: newest-first seed, clamped older paging,
  two-step tail merge with divergent-reset, LRU eviction with pinned
  newest page, transcript_too_large unsupported latch, snapshot
  invalidation recovery, and promptId/record-id-only reconciliation.
- Persisted identity plumbing: messages carry sourceRecordIds/promptId,
  enabling the canonical turn locator and record/prompt-id dedup.
- Anchored admission actions openTranscriptAtTurn plus older/newer
  continuations with distinct failure reasons (page_too_large, window_full,
  window_impossible, snapshot_gone, invalid_anchor), admitted without
  disturbing the live tail.

The rail UI, App wiring, and gap sentinel rendering remain Phase 3.
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

E2E / 单元测试报告(Phase 2 数据层,无真实浏览器 E2E —— rail UI 属 Phase 3)

验证环境:Linux,Node 22,分支 feat/web-shell-turn-navigation-phase2(基于含 Phase 1 的 origin/main)。

  • cd packages/web-shell && npx vitest run268 files / 6072 tests,0 failed / 0 errors(含新增 56 项)。
  • npx vitest run client/daemon/session/DaemonSessionProvider.test.tsx — 295 通过:新增 turn navigation (Phase 2) 12 项(seed 门控、锚定打开、去重、window_full/impossible 区分、服务端拒绝映射、双向 continuation、live provisional、终态刷新),原有 283 项零修改通过。
  • npx vitest run client/daemon/session/actions.test.ts — 162 通过(onPromptAdmitted 回归)。
  • npx vitest run client/daemon/session/transcriptPageLedger.test.ts — 18 通过(含 displaced-gap 保留、gaps 长度不变量、trim 收缩、rewind 丢弃)。
  • npx vitest run client/daemon/session/turnIndexStore.test.ts — 26 通过(seed/older clamp/ensurePage/两步 merge/LRU/闩锁/对账/rewind/findTurn)。
  • npx vitest run client/adapters — 通过(新增 4 项身份管道)。
  • 根目录 npm run buildnpm run typecheck — 通过;eslint / prettier — 干净。

基线说明:worktree 初次 dry-run 的 4 个失败为环境性(共享主检出 node_modules 导致 SDK dist 过旧),worktree 内 npm install && npm run build 后消失,与本改动无关。

自审计:5 轮 presume-wrong diff 审计,修复 5 个实际问题(markUnsupported 在途响应、insertEntry 挤位 gap 保留、非 ready → unavailable、legacy 对账多 record 链接、trim/rewind gap 对齐与尾部 gap 丢失),R4/R5 连续清洁收敛。测试计划与完整结果存档于 .qwen/e2e-tests/web-shell-turn-navigation-phase2.md(工作树内)。

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on ebb7c62 and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— ebb7c62 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — every required heading is present and filled in with real content, including an honest Risk & Scope section that names its own weak spots.

Problem: documented, not theoretical. Tracking issue #10750 is open with priority/P2, type/feature-request and roadmap/session-management, and the Phase 2 design doc landed on main two days ago via #11020 with a Problem section verified against origin/main at 80497a74d0. The four couplings it names — a flat append/prepend-only store with nowhere to land a mid-history page, no persisted identity on materialized messages, one-way oldest-first eviction that leaves no gap record, and a rail derived from whatever happens to be retained — are all real and observable in the current client. This is a planned phase of an approved roadmap item, so 1b passes on evidence rather than on a reproduction.

Direction: aligned. This is Phase 2 of a three-phase plan whose design was reviewed and merged before any code was written, Phase 1 already shipped the daemon-side protocol, and the work sits squarely inside the Web Shell's own surface. The daemon already advertises session_turn_navigation (packages/cli/src/serve/capabilities.ts:145), so the client side is the missing half, and the PR correctly refuses to touch the SDK, the daemon routes, or the core reader — matching the design's "Files affected" note that those stay unchanged. CHANGELOG (claude-code): no direct reference for turn navigation or transcript pagination, but the area is relevant and already roadmap-tracked here.

Size: no core paths — all 12 files are under packages/web-shell/client/**, single package, so the two-tier core gate does not apply. Breakdown: 1965 production lines (1950 added + 15 deleted) vs 1618 test lines, 3583 total. That clears the 1000-line large-PR advisory, so: this is a big PR and splitting would make it easier to review if that is still feasible. The design doc itself lays out five independently shippable migration steps (ledger in degenerate form → gap tracking → identity plumbing → turn-index store → anchored admission), and steps 1–3 are explicitly behavior-preserving while 4–5 add the new surface. Landing 1–3 separately from 4–5 would have given a much smaller review with the same end state. Not a blocker — the work is already written and coherent — but worth knowing for Phase 3, which the design expects to be larger still.

Approach: the scope tracks the merged design closely, and the diff is disciplined for its size — 15 deletions across 3568 additions, no drive-by refactors, no formatting churn, no unrelated files. Two things I would like your read on before the code review goes further:

  1. The design's "Capability gate" row lists App.tsx pass-through (mirroring the session_transcript_pagination wiring at App.tsx:81/:3313) as part of Phase 2, but this PR defers it to Phase 3. Both constant declarations the design asks for are here — the provider-local one and the client/constants/sessions.ts one — so SESSION_TURN_NAVIGATION_FEATURE in constants/sessions.ts currently has no consumer. Is that deliberate staging (the App-level gate only becomes meaningful once the rail exists), or an omission? If deliberate, it is fine, just inert until Phase 3; if not, it is a one-line follow-up.
  2. The description cites fix(cli): restore green main CI after the live slash-command submit change (#10967) #10968 as one of the "Phase 1 daemon/SDK protocol" PRs. fix(cli): restore green main CI after the live slash-command submit change (#10967) #10968 is closed unmerged and is fix(cli): restore green main CI after the live slash-command submit change — unrelated. The design doc names feat(serve): add session turn navigation protocol #10751 as the merged Phase 1. Worth correcting so a reviewer following the trail does not land on the wrong PR.

One substantive question on the design's own terms: the doc's Open Question 1 says the window budgets "need a measurement pass against today's 50,000-block behavior before freezing", and Open Question 3 leaves the 500-block quiet-period reload in place pending measurement. The description confirms the reload stays and calls the chunked tail-fill loop "defensive and unreachable in production conditions". Since neither open question is resolved here, what is the plan for closing them — does Phase 3 carry the measurement pass, or should that be its own follow-up? Unreachable defensive code plus unfrozen budgets is the kind of thing that quietly ossifies.

Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk path set (Stage 1e clean), and this PR does not touch auth, sandbox, model selection, telemetry, release, or a public contract. The one thing a reviewer should focus on regardless: DaemonSessionProvider.tsx takes +813 lines in an already very large, very load-bearing file, and the description's own stated main risk — two new stores reconciled against the transcript store on every trim/rewind/admission, where a reconciliation bug desyncs the ledger from the rendered window — is exactly the failure mode that unit tests over a mocked daemon client are weakest at catching. The claim that existing behavior is byte-identical without the capability is the load-bearing claim of this PR, and it is asserted rather than demonstrated.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 所有必需小节都存在且填写了真实内容,Risk & Scope 还主动指出了自身的薄弱点。

问题: 有据可查,不是理论性加固。跟踪 issue #10750 处于 open 状态,带 priority/P2type/feature-requestroadmap/session-management 标签;Phase 2 设计文档两天前通过 #11020 合入 main,其 Problem 小节是针对 origin/main80497a74d0)核实过的。文档点出的四处耦合——扁平的仅追加/仅前插 store 无处安放历史中段页、物化后的消息没有持久身份、单向的最旧优先驱逐不留 gap 记录、导航条只能派生自当前恰好保留的内容——在当前客户端里都真实可见。这是一个已批准 roadmap 项目的规划阶段,因此 1b 依据的是证据而非复现步骤。

方向: 对齐。这是三阶段计划中的 Phase 2,设计先经评审并合入才开始写代码,Phase 1 已交付 daemon 侧协议,工作范围完全落在 Web Shell 自身界面内。daemon 已经声明 session_turn_navigationpackages/cli/src/serve/capabilities.ts:145),客户端正是缺失的另一半;PR 也正确地没有改动 SDK、daemon 路由与 core reader,与设计文档"这些保持不变"的说法一致。CHANGELOG(claude-code):未找到 turn navigation / transcript pagination 的直接条目,但该领域相关,且本仓库已列入 roadmap。

规模: 未触及核心路径——12 个文件全部位于 packages/web-shell/client/**,单一 package,因此两级核心门禁不适用。行数拆分:生产代码 1965 行(新增 1950 + 删除 15)、测试 1618 行,合计 3583 行。这超过了 1000 行的大 PR 提醒阈值,所以:这是一个大 PR,如果还来得及,拆分会让评审轻松很多。设计文档本身就给出了五个可独立发布的迁移步骤(退化形态的台账 → gap 跟踪 → 身份打通 → turn-index store → 锚定入账),其中 1–3 步明确是行为保持的,4–5 步才引入新界面。把 1–3 与 4–5 分开发布,可以在终态相同的情况下大幅缩小评审面。这不是阻塞项——代码已经写完且自洽——但对 Phase 3 有参考价值,设计预计 Phase 3 规模更大。

方案: 范围与已合入的设计贴合,且以这个体量而言 diff 很克制——3568 行新增里只有 15 行删除,没有顺手重构、没有格式化噪音、没有无关文件。在进入代码审查前,有两点想听你的判断:

  1. 设计的"Capability gate"一行把 App.tsx 的透传(对照 App.tsx:81/:3313session_transcript_pagination 接线)列为 Phase 2 的一部分,但本 PR 把它推到了 Phase 3。设计要求的两处常量声明都在——provider 本地的那份和 client/constants/sessions.ts 的那份——所以 constants/sessions.ts 里的 SESSION_TURN_NAVIGATION_FEATURE 目前没有消费者。这是有意的分阶段(App 级门控要等导航条存在才有意义),还是遗漏?如果是有意的,没问题,只是到 Phase 3 之前是惰性的;如果不是,那就是一行的后续补充。
  2. 描述里把 fix(cli): restore green main CI after the live slash-command submit change (#10967) #10968 列为"Phase 1 daemon/SDK 协议"PR 之一。fix(cli): restore green main CI after the live slash-command submit change (#10967) #10968 是关闭未合并的 fix(cli): restore green main CI after the live slash-command submit change,与此无关。设计文档写明 Phase 1 合入的是 feat(serve): add session turn navigation protocol #10751。建议更正,免得评审者顺着链接找到错误的 PR。

另有一个基于设计文档自身条款的实质问题:文档 Open Question 1 说窗口预算"需要针对当前 50,000 block 行为做一次测量后才能冻结",Open Question 3 把 500-block 静默期整页重载留待测量后再决定。描述确认该重载保留,并称分块尾部填充循环"是防御性的,在生产条件下不可达"。既然这两个开放问题在本 PR 中都没有结论,打算怎么收尾——由 Phase 3 承担这次测量,还是单独开一个后续项?不可达的防御代码加上未冻结的预算,正是容易悄悄僵化的组合。

风险: 无升级风险信号——改动文件均未命中与回滚相关的高风险路径集合(Stage 1e 干净),本 PR 也没有触及 auth、sandbox、模型选择、telemetry、发布或对外契约。评审者仍应重点关注一处:DaemonSessionProvider.tsx 在一个本已非常庞大且承重的文件里增加了 813 行;而描述自陈的主要风险——两个新 store 在每次裁剪/rewind/入账时与 transcript store 对账,对账缺陷会让台账与渲染窗口失同步——恰恰是基于 mock daemon client 的单元测试最不容易发现的失效模式。"缺少该能力时现有行为逐字节一致"是本 PR 的承重论断,目前是断言而非演示。

进入代码审查 🔍

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

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

Screenshots · before / after

terminal-turn-error-copy-narrow-dark before/after

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

Qwen Code · web-shell visuals

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code review

I read the whole diff against the merged Phase 2 design doc and against the current client code on main. No critical blockers — the protocol handling, the ledger geometry, and the store lifecycle are correct as far as I can tell statically, and the parts I expected to be wrong turned out to be handled. Five things worth your attention, none of which I'd gate the merge on by myself.

1. The capability gate is much narrower than the "byte-identical" claim. SESSION_TURN_NAVIGATION_FEATURE is read in exactly one place in the entire diff — constructing the turn-index store with enabled. Everything else runs unconditionally, including on a daemon that never advertises the capability:

  • the page ledger is maintained on initial load, on every prepend, and on every trim and rewind — ten mutation/setLedgerVersion sites, one of which is a React state update per truncation that this provider did not previously perform;
  • ledgerEntryFromBlocks re-estimates per-block bytes through estimateDaemonTranscriptBlockBytes, the same estimator the SDK store already ran to produce retainedBytes, so that work is duplicated for every page;
  • applyPersistedIdentity adds sourceRecordIds/promptId to every projected message on every transcriptBlocksToDaemonMessages call, building a Map over all blocks each time — including the hot useMessages render path, which is where a 50,000-block window lives.

Nothing reads those fields or that ledger yet, so no user would see a difference and I'm not calling this a regression. But "Daemons without session_turn_navigation get byte-identical behavior" is stronger than what the code does: message objects gain two fields and the provider does strictly more work per trim, per prepend, and per projection. Either widen the gate or soften the claim to "no user-visible change; the ledger and identity plumbing run unconditionally and are inert until Phase 3". I'd rather the description be corrected — it's the sentence a reviewer leans on when approving, and it's the one a future bisect would trust.

2. computeLedgerInsertIndex returns the first entry with no index-known turn id, regardless of ordinal. if (minOrdinal === undefined || minOrdinal > targetOrdinal) return index; treats an entry whose turnIds don't resolve against the currently retained index pages as newer than everything. That happens whenever an entry is made entirely of non-navigation records, or its records fell out of a non-pinned page under LRU eviction. Because applyTranscriptPageInsert splices into state.blocks, the consequence is blocks landing in the wrong order in the render source, not just a misplaced rail highlight. It's latent today — nothing calls openTranscriptAtTurn yet — and your Risk & Scope does disclose an insertion heuristic, but what you described is the incoming page containing no navigation turns, whereas this branch fires on an existing ledger entry containing none. Different case, and a stronger consequence than "visually". Continuing the scan past unresolvable entries instead of returning at them would remove the branch; otherwise it's worth recording as a named Phase 3 precondition.

3. The invalidation branch of handleFetchError bypasses the retry bound. transcript_snapshot_unavailable and invalid_transcript_cursor route to invalidateAndReseed() and return before retryAttempts and RETRY_DELAYS_MS are ever consulted. Under the documented protocol that terminates, and I checked why: both codes are snapshot-bound, and seed() sends neither a snapshot nor a start, so a reseed can't draw the same code. But nothing on the client enforces that. If a daemon ever returned one of those codes for a snapshot-less index request, the store would loop invalidate → reseed → invalidate with no backoff and no attempt cap, one real HTTP request per iteration. Cheap to close — route the invalidation branch through the same bounded counter, or cap consecutive reseeds. Flagging it as a defensive gap, not an observed bug; I have no evidence any daemon behaves that way.

4. applyPersistedIdentity intentionally survives stripSourceIdentity, and the comment says so. The consequence is that includeSourceIdentity: false no longer means "no source identity on messages" — it now controls only sourceBlockIds. I traced the one caller on that path, transcriptEventsToMessages in App.tsx, which feeds getFileChangesByTurn/getArtifactsByTurn for the review panel and is unaffected. So the behavior is fine; the option's name just under-describes it now, and the next reader will assume the strip is total. One line on the option would fix that.

5. Unconsumed surface, counted rather than estimated. Production references vs. test-only references, verified across the diff:

  • ensurePage, loadOlder, getKnownTurnIds, buildTurnLocator, updateEntry — defined and tested, zero production callers.
  • useDaemonTurnIndex, useDaemonTranscriptLedger — exported hooks, zero consumers. useDaemonTranscriptLedger's own doc comment says Phase 2 consumers are "the window actions and tests", which is honest.
  • openTranscriptAtTurn / continueTranscriptOlder / continueTranscriptNewer — genuinely wired into actions, but no component calls them.
  • SESSION_TURN_NAVIGATION_FEATURE in constants/sessions.ts — zero consumers; the provider deliberately uses its own local copy, which is the existing pattern.

This is deliberate phase staging against a merged design and the doc comments say so, which is the right way to land it. I'm naming it only so the merge is a conscious decision: roughly 1,965 production lines arrive with no user-visible effect and nothing exercised outside mocks, and a future find-simplifications sweep will flag all of these as dead-export candidates unless the Phase 3 intent stays documented next to them.

What I checked and believe is correct

Worth stating, because it's where the risk actually was:

  • The protocol constraints are honored and pinned by tests. continueTranscriptNewer sends {cursor, limit, clientId} alone; continueTranscriptOlder sends {beforeRecordId, snapshot, limit, clientId}; the anchored open sends {atRecordId, snapshot, limit, clientId}. Each is asserted with toHaveBeenLastCalledWith, so a later refactor that pairs the cursor with a snapshot — a 400 invalid_transcript_cursor against the real daemon — fails the suite rather than silently shipping.
  • transcriptWindowFailureReason maps exactly the five status/code pairs in the design's error matrix, keyed on status and code, and returns undefined for anything else rather than guessing.
  • The gap-array invariant (gaps.length === entries.length + 1, gaps[i] being the range before entries[i]) holds across recordInitialLoad, recordPrepend, insertEntry, clear, applyPrefixTrim, and applyRewind. I worked through each splice; recordPrepend's gaps.slice(1) and applyPrefixTrim's trailing-gap carry-over are both right.
  • nextOrdinal cannot collide. The isolated materialization store is seeded with current.nextOrdinal, so mergeTranscriptPage's nextOrdinal: history.nextOrdinal is monotonically ahead of the window's. Since nextOrdinal mints block ids (sdk-typescript/src/daemon/ui/transcript.ts:2156), a reset here would have produced duplicate ids on the next live block — it doesn't.
  • blockIndexById is not left stale by a mid-window splice. This was my main worry going in, since mergeTranscriptPage spreads ...current and never recomputes the index. store.reset() rebuilds it from the seed's blocks (sdk-typescript/src/daemon/ui/store.ts:306), and recomputes retainedBytes when the seed omits it. Handled.
  • Insert-position geometry is correct for all three actions, including the append-after-newest case in continueTranscriptNewer where entries[insertIndex] is undefined and it correctly falls back to lastIndex + 1, placing the page before the live tail.
  • Stale-response guards are consistent across all three actions: session identity, store identity, a monotonic selection counter, and paginationGenerationRef, re-checked after every await. One asymmetry — the catch blocks re-check session and store but not the selection counter, so a superseded request can still surface its error reason. Cosmetic given nothing consumes these yet.
  • React state identity is handled with unusual care: pages is replaced with a new Map on admission, evictLru deliberately keeps identity stable when nothing was evicted, liveEntries is always reassigned, and getState() caches. dispose() bumps generation, and every async continuation and scheduled retry re-checks it, so a disposed store cannot admit.
  • reuseUnchangedProjectedPrefix compares blocks by identity and messages by id/role, so the new array-valued sourceRecordIds does not defeat the prefix-reuse optimization. I checked this specifically because a fresh array per projection is exactly what would have broken it.

One nit I'm not asking you to change: label.slice(0, 160) in onPromptAdmitted truncates UTF-16 code units where the daemon's compactPreviewText caps at 160 code points, so an astral character at the boundary yields a lone surrogate. Cosmetic, and consistent with the existing boundedString helper in the same file.

Anchored open — the new request → admit → re-inject path
sequenceDiagram
    participant P1 as Caller (Phase 3, absent here)
    participant P2 as DaemonSessionProvider
    participant P3 as SessionTurnIndexStore
    participant P4 as DaemonSessionClient
    participant P5 as TranscriptPageLedger
    participant P6 as SDK transcript store
    P1->>P2: openTranscriptAtTurn(turnId)
    P2->>P3: findTurn(turnId)
    P3-->>P2: entry plus its own page snapshot
    P2->>P4: getTranscriptPage(atRecordId, snapshot, limit)
    P4-->>P2: page with events, hasMore, hasOlder, targetRecordId
    P2->>P2: normalize, then materialize with record-id and prompt-id dedup
    P2->>P5: computeLedgerInsertIndex by ordinal
    P5-->>P2: insertIndex
    P2->>P6: reset with the page spliced at insertBlockIndex
    P2->>P5: insertEntry(page, insertIndex, gapBefore)
    P2-->>P1: ok with targetRecordId, or a distinct failure reason
Loading

The live tail is never touched: the splice lands at a ledger-derived block index, and the page is admitted atomically — a rejected admission leaves the window unchanged.

Files changed (12)
File What changed
packages/web-shell/client/daemon/session/turnIndexStore.ts New, 640 lines. Session-wide turn-index store — newest-first seed, independent older paging, two-step tail merge, LRU with a pinned newest page, unsupported latch, bounded retry.
packages/web-shell/client/daemon/session/turnIndexStore.test.ts New, 634 lines. Covers the clamp, the boundary-zero guard, tail merge paths, LRU, the latch, and the capability gate.
packages/web-shell/client/daemon/session/transcriptPageLedger.ts New, 378 lines. Page ledger plus gap model, trim and rewind reconciliation, and the unconsumed buildTurnLocator.
packages/web-shell/client/daemon/session/transcriptPageLedger.test.ts New, 406 lines. Gap invariants, insert and update, prefix trim, rewind shrink, locator map.
packages/web-shell/client/daemon/session/DaemonSessionProvider.tsx Plus 813, minus 13. Store creation and disposal, ledger reconciliation at every mutation site, the three window actions, two new contexts and hooks, prompt-admission feed.
packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx Plus 514. A turn navigation describe block; the pre-existing 295 tests are otherwise untouched.
packages/web-shell/client/daemon/session/types.ts Plus 55. The failure-reason union, the two result types, and a core-actions Omit for the layered composition.
packages/web-shell/client/daemon/session/actions.ts Plus 10, minus 2. An optional onPromptAdmitted callback fired at both admission sites, and the return type narrowed to core actions.
packages/web-shell/client/adapters/transcriptToMessages.ts Plus 45. The applyPersistedIdentity pass, placed after tool-group sync and before the strip.
packages/web-shell/client/adapters/transcriptToMessages.test.ts Plus 64. Identity propagation across folded blocks.
packages/web-shell/client/adapters/messageTypes.ts Plus 8. sourceRecordIds and promptId on DaemonMessageMeta.
packages/web-shell/client/constants/sessions.ts Plus 1. The App-facing capability constant, currently with no consumer.

Testing

This is an unattended CI run. Per the gate's rules I did not build, run, or execute anything from this PR — no npm, no vitest, no checkout. All test evidence below is this PR's own CI, read through the API for the reviewed commit. There is no tmux capture, because there is nothing user-visible to capture: the PR adds no UI surface and the rail is Phase 3.

Not verified, and why:

  • Not verified: that the unit suite passes. Test (ubuntu-latest, Node 22.x) was still in_progress at the time of writing, as was Lint & Static. This repo's unit suite runs around 30 minutes and the gate does not poll or wait on pending checks, so I'm reporting the state as of the read rather than guessing the outcome. The Qwen Triage Finalize job rewrites the table below in place once CI settles and handles any deferred approval. No check had failed at the time of the read.
  • Not verified: the author's "6072 tests, 56 new" and "295-test provider suite passes" figures. Those are the author's claims from a local Linux run, quoted here as claims only — I did not re-run them and cannot confirm the counts.
  • Not verified: macOS and Windows. CI skips both Test (macos-latest, ...) and Test (windows-latest, ...), and the PR's own Tested-on table marks both ⚠️. The author's Linux-only ✅ is consistent with what CI actually covers, so this is not a discrepancy — but nothing here exercises the other two platforms.
  • Not verified against a real daemon. This is the gap that matters. All 56 new tests drive a MockDaemonSessionClient, so they pin the client against the author's model of the reader. Where that model and the real reader disagree — the shape of an anchored page's hasOlder/targetRecordId, whether nextCursor continues forward toward the frozen tail as the design asserts, how a snapshot replacement surfaces mid-paging — every test here passes and production still breaks. The protocol-level assertions are the strongest thing in this PR precisely because they'd catch a client-side regression, but they cannot catch a client/server model mismatch.
  • Not verified: performance. The design's Open Question 1 defers freezing the window budgets to a measurement pass against today's 50,000-block behavior, and no measurement is in this PR. Point 1 above adds unconditional per-projection and per-trim work on the ungated path, which is exactly the kind of cost that stays invisible until a long session hits it.

Sandboxed verification would settle the part static review cannot: @qwen-code /verify — that an anchored getTranscriptPage against a real daemon returns the page shape the ledger assumes (targetRecordId, hasOlder, and a nextCursor that continues forward toward the frozen tail), and that the unconditional ledger and identity work on the capability-absent path costs nothing measurable against the base build. Both are load-bearing claims of this PR and neither is observable from the diff or from a mocked suite. @qwen-code /tmux would add nothing here — there is no TUI or Web Shell surface change to drive until Phase 3.

Final CI results for ebb7c62 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

No check had failed at the time of this read, so there is no failing-job log excerpt to quote. If Test (ubuntu-latest, Node 22.x) or Lint & Static lands red, that is a finding against this PR until shown otherwise — both run the code this diff changes, and neither is a known-flaky lane.

中文说明

代码审查

我把整个 diff 对照已合入的 Phase 2 设计文档、以及 main 上现有的客户端代码通读了一遍。没有发现致命阻塞项——协议处理、台账几何与 store 生命周期在静态审查范围内都是正确的,而我原本预判会出问题的几处,实际上都处理到位了。有五点值得你关注,但我个人不会以其中任何一点单独阻塞合并。

1. 能力门控的范围远窄于"逐字节一致"这个论断。 整个 diff 里 SESSION_TURN_NAVIGATION_FEATURE 只被读取了一次——构造 turn-index store 时传入 enabled。其余全部是无条件执行的,包括在从未声明该能力的 daemon 上:

  • 页台账在初始加载、每次前插、每次裁剪与 rewind 时都会维护——共十处变更/setLedgerVersion 位点,其中一处是每次 truncation 都触发的 React 状态更新,而这个 provider 以前并不做这件事;
  • ledgerEntryFromBlocks 通过 estimateDaemonTranscriptBlockBytes 重新估算每个 block 的字节数,而 SDK store 为得出 retainedBytes 已经跑过同一个估算器,所以每页都重复了这份工作;
  • applyPersistedIdentity 在每次 transcriptBlocksToDaemonMessages 调用时,为每条投影消息加上 sourceRecordIds/promptId,并每次对所有 block 构建一个 Map——包括 useMessages 这条热渲染路径,而 50,000 block 的窗口正是在那里。

目前没有任何代码读取这些字段或这个台账,所以用户看不出差别,我也不把它称为回归。但"没有 session_turn_navigation 的 daemon 得到逐字节一致的行为"比代码实际做的更强:消息对象多了两个字段,provider 在每次裁剪、每次前插、每次投影时都确实多做了工作。要么把门控范围扩大,要么把论断改成"无用户可见变化;台账与身份打通无条件执行,在 Phase 3 之前是惰性的"。我更希望修正描述——那正是评审者批准时所倚赖的一句话,也是未来 bisect 时会信任的一句话。

2. computeLedgerInsertIndex 会返回第一个没有索引已知 turn id 的条目,而不管其 ordinal。 if (minOrdinal === undefined || minOrdinal > targetOrdinal) return index;turnIds 无法在当前保留的索引页中解析的条目当作比一切都新。当某个条目完全由非导航记录构成,或其记录在非固定页被 LRU 驱逐后失效时,就会出现这种情况。由于 applyTranscriptPageInsert 是拼接到 state.blocks 里的,后果是 block 在渲染源中落到错误的顺序上,而不只是导航条高亮位置偏移。今天它是潜伏的——还没有任何东西调用 openTranscriptAtTurn——而你的 Risk & Scope 确实披露了一个插入位置启发式,但你描述的是传入页不含导航 turn,而这个分支触发于已有台账条目不含导航 turn。是不同的情形,后果也比"视觉上"更强。让扫描跳过无法解析的条目而不是在此返回,就能消掉这个分支;否则值得把它记为 Phase 3 的一项前置条件。

3. handleFetchError 的失效分支绕过了重试上限。 transcript_snapshot_unavailableinvalid_transcript_cursor 会走到 invalidateAndReseed() 并直接返回,retryAttemptsRETRY_DELAYS_MS 根本不会被查询。在文档化的协议下这会终止,我核实了原因:两个 code 都与 snapshot 绑定,而 seed() 既不发 snapshot 也不发 start,所以重新 seed 不会再次拿到同样的 code。但客户端并没有强制这一点。如果某个 daemon 对一个不带 snapshot 的索引请求返回了这两个 code 之一,store 就会以 invalidate → reseed → invalidate 循环,既无退避也无次数上限,每轮发出一个真实 HTTP 请求。堵上它很便宜——让失效分支走同一个有界计数器,或者限制连续 reseed 次数。我把它作为防御性缺口提出,不是已观测到的 bug;我没有任何证据表明存在这样的 daemon。

4. applyPersistedIdentity 有意在 stripSourceIdentity 之后仍然保留,注释也这么写了。后果是 includeSourceIdentity: false 不再意味着"消息上没有来源身份"——它现在只控制 sourceBlockIds。我追查了该路径上唯一的调用方 App.tsx 里的 transcriptEventsToMessages,它服务于评审面板的 getFileChangesByTurn/getArtifactsByTurn,不受影响。所以行为没问题,只是这个选项的名字现在描述得不够,下一位读者会假定 strip 是彻底的。在选项上加一行说明就够了。

5. 未被消费的表面——是数出来的,不是估的。 在 diff 全范围内核对生产引用与仅测试引用:

  • ensurePageloadOldergetKnownTurnIdsbuildTurnLocatorupdateEntry——已定义并有测试,生产调用方为零。
  • useDaemonTurnIndexuseDaemonTranscriptLedger——导出的 hook,消费者为零。useDaemonTranscriptLedger 自己的文档注释说 Phase 2 的消费者是"窗口动作与测试",这是诚实的。
  • openTranscriptAtTurn / continueTranscriptOlder / continueTranscriptNewer——确实接进了 actions,但没有组件调用。
  • constants/sessions.ts 里的 SESSION_TURN_NAVIGATION_FEATURE——消费者为零;provider 刻意使用自己的本地副本,这与既有模式一致。

这是对照已合入设计的有意分阶段,且文档注释写明了,这是正确的落地方式。我点出来只是为了让合并成为一个有意识的决定:大约 1965 行生产代码落地后没有任何用户可见效果,也没有在 mock 之外被行使过;除非 Phase 3 的意图继续写在它们旁边,否则未来的 find-simplifications 清扫会把这些全部标为死导出候选。

我核对过并认为正确的部分

值得说明,因为风险恰恰在那里:

  • 协议约束被遵守,而且被测试钉住了。 continueTranscriptNewer 单独发送 {cursor, limit, clientId}continueTranscriptOlder 发送 {beforeRecordId, snapshot, limit, clientId};锚定打开发送 {atRecordId, snapshot, limit, clientId}。每一项都用 toHaveBeenLastCalledWith 断言,所以将来若有重构把 cursor 与 snapshot 配对发送——对真实 daemon 就是 400 invalid_transcript_cursor——会让测试失败,而不是静默上线。
  • transcriptWindowFailureReason 精确映射了设计错误矩阵里的五组 status/code,同时以 status 与 code 为键,其余情况返回 undefined 而不是猜测。
  • gap 数组不变量gaps.length === entries.length + 1gaps[i]entries[i] 之前的区间)在 recordInitialLoadrecordPrependinsertEntryclearapplyPrefixTrimapplyRewind 中都成立。我逐一推演了每个 splice;recordPrependgaps.slice(1)applyPrefixTrim 的尾随 gap 保留都是对的。
  • nextOrdinal 不会冲突。 隔离的物化 store 是用 current.nextOrdinal 播种的,所以 mergeTranscriptPagenextOrdinal: history.nextOrdinal 单调领先于窗口。由于 nextOrdinal 用于铸造 block id(sdk-typescript/src/daemon/ui/transcript.ts:2156),这里若被重置就会在下一个 live block 上产生重复 id——实际不会。
  • blockIndexById 不会因中段拼接而失效。 这是我一开始最担心的地方,因为 mergeTranscriptPage 展开了 ...current 且从不重算该索引。store.reset() 会从 seed 的 blocks 重建它(sdk-typescript/src/daemon/ui/store.ts:306),并在 seed 未提供时重算 retainedBytes。已处理。
  • 三个动作的插入位置几何都正确,包括 continueTranscriptNewerentries[insertIndex] 为 undefined 的追加到最新页之后的情形——它正确回退到 lastIndex + 1,把页放在实时尾部之前。
  • 陈旧响应防护在三个动作中一致:会话身份、store 身份、单调选择计数器、paginationGenerationRef,每次 await 之后都重新检查。有一处不对称——catch 分支重新检查了会话与 store,但没有检查选择计数器,所以被取代的请求仍可能抛出它的错误原因。考虑到还没有消费者,这只是表面问题。
  • React 状态身份处理得异常细致:pages 在入账时被替换为新 Map,evictLru 在未驱逐任何页时刻意保持身份稳定,liveEntries 总是重新赋值,getState() 带缓存。dispose() 会递增 generation,而每个异步延续与已排程的重试都会重新检查它,所以已 dispose 的 store 不可能入账。
  • reuseUnchangedProjectedPrefix 以身份比较 block、以 id/role 比较消息,所以新的数组型 sourceRecordIds 不会破坏前缀复用优化。我特意核对了这一点,因为每次投影都产生新数组正是会破坏它的那种写法。

一个我并不要求你修改的小点:onPromptAdmitted 里的 label.slice(0, 160) 截断的是 UTF-16 码元,而 daemon 的 compactPreviewText 上限是 160 个码点,所以边界上的astral字符会产生孤立代理项。属于外观问题,且与同文件中既有的 boundedString 辅助函数一致。

测试

这是一次无人值守的 CI 运行。按照门禁规则,我没有构建、运行或执行本 PR 的任何内容——没有 npm、没有 vitest、没有 checkout。下面所有测试证据都来自本 PR 自己的 CI,通过 API 针对被审查的 commit 读取。没有 tmux 抓取,因为没有用户可见的东西可抓:本 PR 不增加任何 UI 界面,导航条属于 Phase 3。

未验证项及原因:

  • 未验证:单元测试套件是否通过。 撰写时 Test (ubuntu-latest, Node 22.x) 仍为 in_progressLint & Static 亦然。本仓库的单元测试套件约需 30 分钟,而门禁不会轮询或等待 pending 的检查,所以我报告的是读取时刻的状态而非猜测结果。Qwen Triage Finalize 任务会在 CI 落定后就地重写下方表格,并处理任何延迟的批准。读取时刻没有任何检查失败。
  • 未验证:作者给出的"6072 项测试、56 项新增"与"295 项 provider 套件通过"。 这些是作者本地 Linux 运行的自述,此处仅作为自述引用——我没有重跑,无法确认这些数字。
  • 未验证:macOS 与 Windows。 CI 跳过了 Test (macos-latest, ...)Test (windows-latest, ...),PR 自己的测试平台表也把两者标为 ⚠️。作者只给 Linux ✅ 与 CI 实际覆盖范围一致,所以这不是矛盾——但这里没有任何东西行使另外两个平台。
  • 未针对真实 daemon 验证。 这是关键缺口。全部 56 项新测试都驱动 MockDaemonSessionClient,因此它们是把客户端钉在作者对 reader 的模型上。凡模型与真实 reader 不一致之处——锚定页的 hasOlder/targetRecordId 形状、nextCursor 是否如设计所断言的那样向前延续至冻结尾部、snapshot 替换在翻页中途如何呈现——这里所有测试都会通过,而生产依然会坏。协议层断言是本 PR 最强的部分,正因为它们能抓住客户端侧的回归,但它们抓不住客户端/服务端模型不一致。
  • 未验证:性能。 设计的 Open Question 1 把冻结窗口预算推迟到一次针对当前 50,000 block 行为的测量之后,而本 PR 中没有任何测量。上面第 1 点在未门控路径上增加了每次投影与每次裁剪的无条件开销,而这正是那种在长会话撞上之前始终不可见的成本。

沙箱验证可以解决静态审查解决不了的部分:@qwen-code /verify ——验证针对真实 daemon 的锚定 getTranscriptPage 返回台账所假定的页形状(targetRecordIdhasOlder,以及一个向前延续至冻结尾部的 nextCursor),并且缺少能力路径上的无条件台账与身份工作相对基线构建没有可测量的开销。这两者都是本 PR 的承重论断,且都无法从 diff 或 mock 套件中观测到。@qwen-code /tmux 在这里没有价值——在 Phase 3 之前没有 TUI 或 Web Shell 界面变化可以驱动。

CI 表格与英文部分相同,此处不重复。读取时刻没有检查失败,因此没有失败任务日志可引用。如果 Test (ubuntu-latest, Node 22.x)Lint & Static 最终为红,在排除之前都应视为针对本 PR 的发现——两者都运行本 diff 改动的代码,也都不是已知的 flaky 通道。

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — solid, and the parts I went in expecting to be wrong were already defended; what's left is an overstated claim in the description, a latent trap for Phase 3, and a lot of deliberately inert surface.

Before reading the diff I wrote down what I'd do from the title and the "Why it's needed" section alone: keep the SDK store flat rather than teaching a shared reducer about non-contiguous ranges, hang a provider-owned page index off it, keep turn metadata in a separate store because it's two orders of magnitude cheaper than transcript bytes, plumb persisted identity onto messages first since everything else keys on it, and gate the whole thing on the advertised capability. That is what this PR does. I don't think that's a coincidence or a sign I anchored — the design doc was reviewed and merged before any code existed, so the structural calls were already made and argued. I did not find a materially simpler path it missed. The only thing I'd have done differently is shipped it as two PRs, which I raised in Stage 1 and am not repeating as a blocker.

What raised my confidence is where the defenses were. I expected mergeTranscriptPage's nextOrdinal: history.nextOrdinal to reset the block-id counter when a page is spliced into the middle of the window, since nextOrdinal mints block ids and a reset would produce duplicates on the next live block — it doesn't, because the isolated materialization store is seeded with current.nextOrdinal. I expected the mid-window splice to leave blockIndexById stale, since the merge spreads ...current and never recomputes it — store.reset() rebuilds the index from the seed's blocks. I expected the two continuations to get the cursor pairing wrong, because sending cursor alongside a snapshot is a 400 and it's an easy mistake to make in three near-identical request builders — both are correct, and both are pinned with toHaveBeenLastCalledWith rather than left to a reviewer's attention. Finding the gap-array invariant intact across all six mutation sites, including the two reconciliation paths, is the same story. Whoever wrote this was thinking about the failure modes, not just the happy path.

So my reservations are all about things that don't bite today. The computeLedgerInsertIndex early return is the one I'd most want fixed before Phase 3 rather than before merge — it splices into the render source, so an unresolvable ordinal on an existing entry misorders blocks, and the limitation your Risk & Scope discloses is a different case from the one the code actually has. The unbounded invalidate → reseed path is defensive and unreachable under the documented protocol, but it's three lines to bound. And the description's "byte-identical behavior" is the claim I'd most want corrected, precisely because it's the sentence someone approves on: the capability is checked once, and the ledger maintenance, the byte re-estimation, and the identity pass all run unconditionally on daemons that will never use them.

The inert surface is the honest reservation, and I want to be clear that I don't think it's a mistake. Seven exported methods and hooks have no production caller, three wired actions have no component calling them, and roughly 1,965 production lines land with nothing user-visible. That is what a middle phase of a phased plan looks like, the doc comments name Phase 3 as the consumer, and deferring it because it isn't independently shippable would be second-guessing a plan that was already approved on its own merits. The six-months-from-now test comes down entirely to whether Phase 3 lands: if it does, the comments and invariants here make it a good foundation to build on; if it stalls, this becomes the largest block of dead code in the package and a find-simplifications sweep will rightly flag it. That's a bet on the roadmap, not on this diff.

Am I approving because I ran out of reasons to say no? No — I have five findings and I'd hold the same verdict with all five stated, because none of them is a defect in code that executes today, and the code that does execute today produces the same output it did before.

Approval is deferred until CI lands green on ebb7c62cca93afe268d18343198f04542771d63a. The PR's own Qwen Code CI workflow was still in progress at the time of writing — Test (ubuntu-latest, Node 22.x) and Lint & Static had not reported, and nothing had failed. Approving now would attest to a result that doesn't exist yet, so I'm posting no review in this run; the finalize job commits the approval against this exact commit once every check on it completes green, and withholds it if anything lands red or the head moves. The two checks I'd want green before trusting this are the unit suite and lint, since both run the code this diff changes and neither is a known-flaky lane.

@qwen-code /verify remains the one thing that would close the gap static review can't — that a real daemon's anchored page matches the shape the ledger assumes, and that the ungated path costs nothing measurable against the base build. Recommended, not required for this approval.

中文说明

信心度:4/5 —— 扎实;我原本预判会出错的地方都已有防护,剩下的是描述中一处过强的论断、一个留给 Phase 3 的潜在陷阱,以及大量有意为之的惰性表面。

在读 diff 之前,我仅凭标题和"Why it's needed"写下了自己的做法:保持 SDK store 扁平,不去教一个共享 reducer 理解非连续区间,而是在其上挂一个 provider 持有的页索引;把 turn 元数据放在独立 store 里,因为它比 transcript 字节便宜两个数量级;先把持久身份打通到消息上,因为其余一切都以它为键;整体由已声明的能力门控。这正是本 PR 所做的。我不认为这是巧合,也不说明我被锚定了——设计文档在写任何代码之前就已评审并合入,所以结构性决策早已做出并论证过。我没有找到它遗漏的、明显更简的路径。我唯一会做得不同的是拆成两个 PR,这一点我在 Stage 1 已提出,不再作为阻塞项重复。

真正提升我信心的是防护所在的位置。我原以为 mergeTranscriptPagenextOrdinal: history.nextOrdinal 会在页被拼接到窗口中段时重置 block id 计数器,因为 nextOrdinal 用于铸造 block id,重置会在下一个 live block 上产生重复——实际不会,因为隔离的物化 store 是用 current.nextOrdinal 播种的。我原以为中段拼接会让 blockIndexById 失效,因为合并展开了 ...current 且从不重算它——store.reset() 会从 seed 的 blocks 重建该索引。我原以为两个 continuation 会把 cursor 配对写错,因为在三个近乎相同的请求构造器里,把 cursor 与 snapshot 一起发送是 400,而且很容易犯——两者都正确,而且都用 toHaveBeenLastCalledWith 钉住了,而不是留给评审者的注意力。在全部六个变更位点(包括两条对账路径)上都发现 gap 数组不变量完好,是同一个故事。写这些代码的人在思考失效模式,而不只是happy path。

所以我的保留意见都集中在今天不会发作的事情上。computeLedgerInsertIndex 的提前返回是我最希望在 Phase 3 之前、而非合并之前修掉的一处——它拼接进渲染源,所以已有条目上出现无法解析的 ordinal 会打乱 block 顺序,而你 Risk & Scope 披露的限制与代码实际存在的情形不是同一个。无界的 invalidate → reseed 路径是防御性的,在文档化协议下不可达,但加上限制只需三行。而描述里的"逐字节一致的行为"是我最希望更正的论断,恰恰因为它是别人据此批准时倚赖的那句话:能力只被检查了一次,而台账维护、字节重估算与身份打通,在永远不会使用它们的 daemon 上都是无条件执行的。

惰性表面是我最真实的保留意见,而我想说清楚:我不认为它是错误。七个导出的方法与 hook 没有生产调用方,三个已接线的动作没有组件调用,大约 1965 行生产代码落地后没有任何用户可见效果。这就是分阶段计划中间阶段的样子,文档注释指明了 Phase 3 是消费者,而因为它不能独立发布就推迟它,等于去质疑一个已凭自身价值获批的计划。"六个月后"的检验完全取决于 Phase 3 是否落地:如果落地,这里的注释与不变量会成为一个好的基础;如果停滞,它就会成为整个 package 里最大的一块死代码,而 find-simplifications 清扫会正当地把它标出来。那是对 roadmap 的下注,不是对这个 diff 的。

我批准是因为我说不出反对理由了吗?不是——我有五项发现,且把这五项都讲明之后我仍会持同样结论,因为它们没有一项是今天会执行的代码中的缺陷,而今天确实会执行的代码产出与之前相同。

批准已推迟,直到 CI 在 ebb7c62cca93afe268d18343198f04542771d63a 上变绿。 撰写时本 PR 自己的 Qwen Code CI 工作流仍在进行中——Test (ubuntu-latest, Node 22.x)Lint & Static 尚未报告,且没有任何检查失败。现在批准等于为一个尚不存在的结果背书,所以本次运行不提交任何 review;finalize 任务会在该 commit 上所有检查变绿后,把批准绑定到这个确切的 commit,若有检查变红或 head 移动则不予批准。我最希望变绿的两项是单元测试套件与 lint,因为两者都运行本 diff 改动的代码,也都不是已知的 flaky 通道。

@qwen-code /verify 仍是唯一能闭合静态审查无法闭合缺口的手段——真实 daemon 的锚定页是否符合台账假定的形状,以及未门控路径相对基线构建是否没有可测量的开销。这是建议,不是本次批准的必要条件。

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

Reviewed at ebb7c62cca93afe268d18343198f04542771d63a · 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 — CI landed green after the review. ✅

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

Partially reviewed — gaps disclosed.

Not reviewed: reverse audit finding R1-59 — the verifier never ruled on it.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": executing the new describe('turn navigation (Phase 2)') block under vitest to confirm which refreshTail / admission branch each test actually takes — both f…; "agent 1c": I did not locate the legacy /session/:id/transcript bridge implementation's event-envelope construction ( AcpSessionBridge.getSessionTranscriptPage resolves ….

Not reviewed: reverse audit — stopped before round 3 by the review time budget.

Test Plan (not a blocker): client/daemon/session/DaemonSessionProvider.test.tsxno such file or directory.

[Critical] R1-14 [certifies-falsely] [new-surface] packages/web-shell/client/daemon/session/DaemonSessionProvider.tsx:2858 (with the sweep at :1093-1108 and the flush feed at :1503-1512) — shell: live-overlay entries are reconciled only by the onTruncation microtask sweep, so the wholesale store.reset() of a live-journal repair rebuild — which keeps the same turn-index store — leaves overlays pointing at blocks that no longer exist and never registers the rebuilt ones. A session restored mid-turn carries user_shell blocks whose shell:<blockId> overlays the batched flush registered via addLiveShell; a repair rebuild then calls store.reset({...replayState}) (:2673-2676), replacing the whole block set with freshly-minted block ids. store.reset() does NOT fire onTruncation (sdk-typescript/src/daemon/ui/store.ts), so the sweep never runs: removeLiveShell is never called for the dead block ids and addLiveShell is never called for the rebuilt ones. SessionTurnIndexState.liveEntries then lists shell overlays whose block ids resolve to nothing — Phase 3's rail renders running-command indicators for shells that finished or vanished and omits the ones actually running. The ring-eviction resync reset (:3547-3549) and the epoch reset (:3221-3223) have the same shape. witness: not run — no successful-repair fixture exists in the suite to drive this against; 2 of the 3 named reset sites were refuted as reachable with overlays live, and the confirmed site is the repair rebuild, traced through store.reset()'s non-emission of onTruncation. Fix: reconcile shell overlays wherever the store is wholesale-replaced, not only on truncation — after each store.reset() that keeps the same turn-index store, re-derive the overlay set from the new snapshot's user_shell blocks (drop entries whose blockId is absent, add entries for new ones), or route those resets through the same reconciliation helper the microtask sweep uses. The test that must pin the fix: a new DaemonSessionProvider.test.tsx case that emits a user_shell block so a shell: overlay is registered, drives a live-journal repair rebuild, and asserts getTurnIndex()?.liveEntries contains no entry whose id references a block absent from useDaemonTranscriptBlocks() — it must go red with the reconciliation removed.

中文说明

仅完成部分审查,审查缺口已披露。

未审查(原文为英文):reverse audit finding R1-59 — the verifier never ruled on it.

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 1)"executing the new describe('turn navigation (Phase 2)') block under vitest to confirm which refreshTail / admission branch each test actually takes — both f…"agent 1c"I did not locate the legacy /session/:id/transcript bridge implementation's event-envelope construction ( AcpSessionBridge.getSessionTranscriptPage resolves …

未审查:反向审计——评审时间预算不足,未能开始第 3 轮。

Test Plan(非阻断):client/daemon/session/DaemonSessionProvider.test.tsxno such file or directory

[Critical] R1-14 [certifies-falsely] [new-surface] packages/web-shell/client/daemon/session/DaemonSessionProvider.tsx:2858 (with the sweep at :1093-1108 and the flush feed at :1503-1512) — shell: live-overlay entries are reconciled only by the onTruncation microtask sweep, so the wholesale store.reset() of a live-journal repair rebuild — which keeps the same turn-index store — leaves overlays pointing at blocks that no longer exist and never registers the rebuilt ones. A session restored mid-turn carries user_shell blocks whose shell:<blockId> overlays the batched flush registered via addLiveShell; a repair rebuild then calls store.reset({...replayState}) (:2673-2676), replacing the whole block set with freshly-minted block ids. store.reset() does NOT fire onTruncation (sdk-typescript/src/daemon/ui/store.ts), so the sweep never runs: removeLiveShell is never called for the dead block ids and addLiveShell is never called for the rebuilt ones. SessionTurnIndexState.liveEntries then lists shell overlays whose block ids resolve to nothing — Phase 3's rail renders running-command indicators for shells that finished or vanished and omits the ones actually running. The ring-eviction resync reset (:3547-3549) and the epoch reset (:3221-3223) have the same shape. witness: not run — no successful-repair fixture exists in the suite to drive this against; 2 of the 3 named reset sites were refuted as reachable with overlays live, and the confirmed site is the repair rebuild, traced through store.reset()'s non-emission of onTruncation. Fix: reconcile shell overlays wherever the store is wholesale-replaced, not only on truncation — after each store.reset() that keeps the same turn-index store, re-derive the overlay set from the new snapshot's user_shell blocks (drop entries whose blockId is absent, add entries for new ones), or route those resets through the same reconciliation helper the microtask sweep uses. The test that must pin the fix: a new DaemonSessionProvider.test.tsx case that emits a user_shell block so a shell: overlay is registered, drives a live-journal repair rebuild, and asserts getTurnIndex()?.liveEntries contains no entry whose id references a block absent from useDaemonTranscriptBlocks() — it must go red with the reconciliation removed.

— qwen3.8-max via Qwen Code /review (v0.23.0)

minOrdinal =
minOrdinal === undefined ? ordinal : Math.min(minOrdinal, ordinal);
}
if (minOrdinal === undefined || minOrdinal > targetOrdinal) return index;

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.

[Critical] R1-1: [certifies-falsely] [new-surface] computeLedgerInsertIndex reads "this entry has no index-known turn id" as "newer than everything" and returns its index immediately, so an anchored page is spliced at the wrong end of the transcript.

ordinalByTurnId is rebuilt on every open from the retained index pages, which are bounded, and nothing in production calls loadOlder() / ensurePage(). Three routine states make an entry resolve to nothing: a load-older prepend entry older than the retained index window; any page of limit records falling entirely inside one long tool-heavy turn, so it holds no turn head (this includes the initial-load entry); and an anchored entry whose ordinals a 409-triggered reseed discarded.

Failure scenario: in a 500-turn session retaining ordinals 300-499, "load earlier history" prepends turns ~290-295, giving [E0(prepend, unknown ordinals), E1(load, known)]. A jump to turn 400 hits E0 first, minOrdinal stays undefined, and the function returns 0 instead of 1. computeInsertBlockIndex then resolves blockIndexById[E0.firstBlockId] === 0, so turn 400's blocks are spliced at the very top, above the older turn-290 page: newer content renders above older content, insertEntry records the same wrong order, and every later continuation compounds the scramble. When the unknown entry is the initial-load entry, every anchored open returns 0 regardless of target.

Witness:

not run — traced through computeLedgerInsertIndex / computeInsertBlockIndex and the bounded index
coverage at ebb7c62c. No provider fixture can reach the ordinal branch: the test fixture mints index
turn ids (turn-N) in a namespace disjoint from every block's record ids (record-*), so minOrdinal is
always undefined and replacing the whole ordinal branch with `return 0` changes no assertion.

Stop inferring recency from absence: record the ordinal range on the entry at admission (the anchored path already holds located.entry.ordinal) and position against the recorded ordinals. Minimal variant if the field is not added — continue past an entry with no resolvable ordinal instead of returning its index, returning entries.length only when no entry resolves.

Any fix that keeps resolving ordinals from retained pages must tolerate entries outside that window: const DEFAULT_PAGE_SIZE = 200; / const DEFAULT_MAX_PAGES = 8; (turnIndexStore.ts:64-65), with evictLru pinning only the newest page.

Please add the test that pins this — seed an index page that does not cover a prepended page's turn ids, prepend via load-older, then openTranscriptAtTurn for a newer turn and assert the anchored entry lands after the prepend entry and the block texts are chronological — and confirm it goes red with the fix removed (today the entry lands at index 0).

中文说明

computeLedgerInsertIndex 把「该 entry 没有任何索引已知的 turn id」当成「比一切都新」并立即返回其下标,导致锚定页被拼接到 transcript 的错误一端。

ordinalByTurnId 每次打开时都从当前保留的索引页重建,而保留范围是有限的,且生产代码中没有任何地方调用 loadOlder() / ensurePage()。三种常见状态都会让 entry 解析不到 ordinal:早于保留索引窗口的 load-older prepend entry;完全落在某个超长 tool 密集 turn 内部、因此不含 turn head 的页(初始加载 entry 就属于这种);以及被 409 触发的 reseed 丢弃了 ordinal 的锚定 entry。

触发场景: 在一个 500 turn、索引仅保留 ordinal 300-499 的会话中,「加载更早历史」prepend 了 turn ~290-295,台账变为 [E0(prepend, ordinal 未知), E1(load, 已知)]。此时跳转到 turn 400:循环先命中 E0minOrdinal 保持 undefined,函数返回 0 而不是 1computeInsertBlockIndex 随后解析为 blockIndexById[E0.firstBlockId] === 0,于是 turn 400 的 block 被拼接到最顶部,位于更旧的 turn-290 页之上:较新内容渲染在较旧内容之上,insertEntry 记录了同样的错误顺序,后续每次 continuation 都会加剧错乱。当 ordinal 未知的 entry 是初始加载 entry 时,无论目标是什么,每次锚定打开都返回 0

证据:

not run — 在 ebb7c62c 上沿 computeLedgerInsertIndex / computeInsertBlockIndex 与有限索引覆盖范围逐行追踪。
现有 provider 测试无法进入 ordinal 分支:测试 fixture 生成的索引 turn id(turn-N)与所有 block 的
record id(record-*)处于不同命名空间,因此 minOrdinal 恒为 undefined,把整个 ordinal 分支替换成
`return 0` 不会让任何断言失败。

修复方向:不要从「缺失」推断新旧——在入账时把 ordinal 区间记录到 entry 上(锚定路径已持有 located.entry.ordinal),并依据记录的 ordinal 定位。若不新增字段,最小改法是:遇到无法解析 ordinal 的 entry 时 continue 而不是返回其下标,仅在所有 entry 都无法解析时才返回 entries.length

约束:turnIndexStore.ts:64-65const DEFAULT_PAGE_SIZE = 200; / const DEFAULT_MAX_PAGES = 8;(且 evictLru 只固定最新页)意味着保留的索引覆盖范围有限,任何仍从保留页解析 ordinal 的修复都必须容忍落在该窗口之外的 entry。

请补上能锁定该行为的测试:先 seed 一个不覆盖 prepend 页 turn id 的索引页,通过 load-older 执行 prepend,再对更新的 turn 调用 openTranscriptAtTurn,断言锚定 entry 落在 prepend entry 之后且 block 文本按时间顺序排列——并确认移除修复后该测试变红(当前 entry 会落在下标 0)。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +413 to +415
displayedPromptIds !== undefined &&
event.promptId !== undefined &&
displayedPromptIds.has(event.promptId)

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.

[Critical] R1-9: [certifies-falsely] [new-surface] The new promptIdDedup filter can never fire on a fetched transcript-window page, so the anchored path — which explicitly turns boundaryEchoDedup off — has no echo dedup at all and renders the user's own prompt twice.

Neither side of the match carries a promptId. Page events are built daemon-side as {v:1, type:'session_update', data:update} with no envelope-level promptId (acpAgent.ts:9068-9072, forwarded unchanged by routes/session.ts:4864-4872), createBase reads only event.promptId, and the user_message_chunk case adds nothing — so every normalized page event has promptId === undefined. The optimistic block for this client's own prompt is created by createTextBlock with no sourceRecordIds and no promptId, and the daemon's recordId-stamped echo is suppressed for the originating client.

Failure scenario: the user sends a prompt, the turn is persisted, and they click that turn (or any turn within limit records of it — atRecordId pages run forward from the anchor). admitTranscriptWindowPage calls materializeTranscriptHistory(..., {boundaryEchoDedup:false, promptIdDedup:true}); the record-id filter misses (the displayed block has no record ids) and the prompt-id filter misses (event.promptId === undefined), so the page's copy of that prompt is materialized and spliced in just before the live tail: the same user message appears twice, and the ledger records the range as loaded so no gap affordance can repair it. boundaryEchoKey would not have caught it either — it only compares against current.blocks[0].

Witness:

probe (real provider, A/B): anchored open over a retained optimistic prompt
  -> allTexts ["my prompt", "live reply", "my prompt"]   # the duplicate
sweep over the 4 page-event construction paths: 0 of 4 stamp an envelope promptId

Dedup on an identity both sides actually carry: the provider knows which promptIds this client submitted (onPromptAdmitted) and the index maps turnId -> promptId, so resolve own-prompt ids to their persisted turn record ids via the retained index pages and pass them as an extra record-id set into materializeTranscriptHistory. If no such identity is available, do not claim the echo case is covered — keep boundaryEchoDedup semantics for live-tail-adjacent inserts instead of disabling them.

The fix must not widen to content/text matching across the window: DaemonSessionProvider.tsx:373-375 — "Keying on text window-wide would instead drop DISTINCT older prompts the user happened to send twice ("yes", a retry), permanently orphaning their assistant replies."

Please add the test that pins this — submit a prompt through the optimistic path, mock getSessionTranscriptPage to return a user_message_chunk carrying that prompt's persisted record id, call openTranscriptAtTurn, and assert exactly one user block with that text — and confirm it goes red with the fix removed (the existing dedupe test only covers an assistant event with a record id).

中文说明

新增的 promptIdDedup 过滤在抓取到的 transcript window 页上永远不可能命中,因此显式关闭了 boundaryEchoDedup 的锚定路径完全没有任何回显去重,会把用户自己的 prompt 渲染两次。

匹配两侧都不携带 promptId。页事件在 daemon 侧构造为 {v:1, type:'session_update', data:update},没有 envelope 级 promptIdacpAgent.ts:9068-9072,并由 routes/session.ts:4864-4872 原样转发);createBase 只读 event.promptId,而 user_message_chunk 分支不追加任何字段——所以每个归一化后的页事件都是 promptId === undefined。本客户端为自己 prompt 创建的乐观 block 由 createTextBlock 生成,既无 sourceRecordIds 也无 promptId,而 daemon 带 recordId 的回显对发起方客户端是被抑制的。

触发场景: 用户发送 prompt、该 turn 被持久化,然后点击这个 turn(或 limit 条记录范围内的任意 turn——atRecordId 页从锚点向展开)。admitTranscriptWindowPage{boundaryEchoDedup:false, promptIdDedup:true} 调用 materializeTranscriptHistory;record-id 过滤未命中(已显示的 block 没有 record id),prompt-id 过滤也未命中(event.promptId === undefined),于是该 prompt 在页中的副本被物化并拼接到实时尾部之前:同一条用户消息出现两次,且台账把该区间记为已加载,没有任何 gap 提示可以修复。boundaryEchoKey 同样拦不住——它只与 current.blocks[0] 比较。

证据:

probe(真实 provider,A/B):对已保留的乐观 prompt 执行锚定打开
  -> allTexts ["my prompt", "live reply", "my prompt"]   # 出现重复
对 4 条页事件构造路径的扫描:0 / 4 会写入 envelope promptId

修复方向:改用两侧都真正携带的身份去重——provider 知道本客户端提交过哪些 promptIdonPromptAdmitted),索引也维护 turnId -> promptId 映射,因此可通过保留的索引页把自己 prompt 的 id 解析为其持久化 turn record id,并作为额外的 record-id 集合传入 materializeTranscriptHistory。若拿不到这样的身份,就不要声称已覆盖回显场景——对紧邻实时尾部的插入保留 boundaryEchoDedup 语义,而不是关闭它。

约束:修复不得扩大到全窗口文本/内容匹配——DaemonSessionProvider.tsx:373-375:「按文本在全窗口去重会误删用户恰好发送过两次的不同旧 prompt(例如「yes」、重试),并使其 assistant 回复永久成为孤儿。」

请补上能锁定该行为的测试:通过乐观路径提交一个 prompt,mock getSessionTranscriptPage 返回携带该 prompt 持久化 record id 的 user_message_chunk,调用 openTranscriptAtTurn,断言只存在一个该文本的 user block——并确认移除修复后该测试变红(现有去重测试只覆盖了带 record id 的 assistant 事件)。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +2811 to +2813
const turnNavigationSupported =
Array.isArray(capabilities?.features) &&
capabilities.features.includes(SESSION_TURN_NAVIGATION_FEATURE);

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.

[Critical] R1-8: [fails-closed] [new-surface] The capability gate reads the run-loop-local capabilities, which is assigned only inside the if (!session) load block — so on the session-reuse path the store is constructed with enabled: false for a daemon that does advertise the feature.

Failure scenario: fresh Web Shell, provider mounted with no sessionId; the user sends the first prompt. createAndAttachSessionForPrompt -> actions.createSession() sets sessionRef.current, then attachSession() bumps attachSessionNonce without ever setting restoreSessionId. The connect effect re-runs, the reuse branch takes session = sessionRef.current and skips the whole if (!session) block — the only assignment site of capabilities (:1725) — so it stays undefined. The creation condition at :2805 is satisfied twice over (ledgerSessionReinit is true because transcriptHistoryRef.current.sessionId is still undefined, and turnIndexStoreRef.current === undefined), so the store is built with enabled: false. Result: seed() no-ops, getSessionTurnIndexPage is never requested, addLivePrompt / addLiveShell / refreshTail all no-op, useDaemonTurnIndex() reports 'disabled' and openTranscriptAtTurn() returns {ok:false, reason:'unsupported'}for the whole life of that page session, because recreation needs a PATH B reload carrying replay data. Meanwhile sessionCapabilitiesRef.current already held the real feature list.

Witness:

PROBE R1-8 ARM A (load path)        : status = ready    , indexPage calls = 1
PROBE R1-8 ARM B (create+attach)    : status = disabled , indexPage calls = 0
PROBE R1-8 ARM B openTranscriptAtTurn = {"ok":false,"reason":"unsupported"}
fix flip (capabilities?.features ?? sessionCapabilitiesRef.current?.features)
  -> ARM B: status = ready, indexPage calls = 1 ; ARM A unchanged
Suggested change
const turnNavigationSupported =
Array.isArray(capabilities?.features) &&
capabilities.features.includes(SESSION_TURN_NAVIGATION_FEATURE);
const turnNavFeatures =
capabilities?.features ?? sessionCapabilitiesRef.current?.features;
const turnNavigationSupported =
Array.isArray(turnNavFeatures) &&
turnNavFeatures.includes(SESSION_TURN_NAVIGATION_FEATURE);

sessionCapabilitiesRef is already the authoritative source for the sibling gates — const features = sessionCapabilitiesRef.current?.features; at :1144 and :1195 — and knownCapabilities at :1333-1336 fixes the precedence as workspace?.capabilities ?? sessionCapabilitiesRef.current ?? connection.capabilities; the fix must not introduce a fourth precedence order.

Please add the test that pins this — render with sessionId: undefined explicitly, advertise ['session_turn_navigation'], then createSession() + attachSession() and assert getSessionTurnIndexPage was called and getTurnIndex()?.status === 'ready' — and confirm it goes red with the fix reverted (every current turn-nav test goes through the load path and stays green either way).

中文说明

能力门控读取的是运行循环内的局部变量 capabilities,而它只在 if (!session) 加载分支内被赋值——因此在会话复用路径上,即使 daemon 确实声明了该能力,store 仍会以 enabled: false 构造。

触发场景: 全新 Web Shell,provider 挂载时没有 sessionId,用户发送第一个 prompt。createAndAttachSessionForPrompt -> actions.createSession() 设置 sessionRef.current,随后 attachSession() 递增 attachSessionNonce,但从未设置 restoreSessionId。connect effect 重新执行,复用分支取 session = sessionRef.current 并跳过整个 if (!session) 块——也就是 capabilities 唯一的赋值点(:1725)——于是它保持 undefined:2805 的创建条件被双重满足(ledgerSessionReinit 为真,因为 transcriptHistoryRef.current.sessionId 仍是 undefined;且 turnIndexStoreRef.current === undefined),store 以 enabled: false 构建。结果:seed() 空转,getSessionTurnIndexPage 从不被请求,addLivePrompt / addLiveShell / refreshTail 全部空转,useDaemonTurnIndex() 报告 'disabled'openTranscriptAtTurn() 返回 {ok:false, reason:'unsupported'}——并且在该页面会话的整个生命周期内都是如此,因为重建需要一次携带 replay 数据的 PATH B 重新加载。而此时 sessionCapabilitiesRef.current 早已持有真实的 feature 列表。

证据:

PROBE R1-8 ARM A(加载路径)      : status = ready    , indexPage calls = 1
PROBE R1-8 ARM B(create+attach) : status = disabled , indexPage calls = 0
PROBE R1-8 ARM B openTranscriptAtTurn = {"ok":false,"reason":"unsupported"}
修复翻转(capabilities?.features ?? sessionCapabilitiesRef.current?.features)
  -> ARM B:status = ready,indexPage calls = 1;ARM A 不变

修复方向:改用同类门控已经在用的 ref 快照(见上方 suggestion 块)。

约束:sessionCapabilitiesRef 已是同类门控的权威来源——:1144:1195const features = sessionCapabilitiesRef.current?.features;——且 :1333-1336knownCapabilities 已把优先级固定为 workspace?.capabilities ?? sessionCapabilitiesRef.current ?? connection.capabilities;修复不得引入第四种优先级顺序。

请补上能锁定该行为的测试:显式以 sessionId: undefined 渲染,声明 ['session_turn_navigation'],然后执行 createSession() + attachSession(),断言 getSessionTurnIndexPage 被调用且 getTurnIndex()?.status === 'ready'——并确认回退修复后该测试变红(当前所有 turn-nav 测试都走加载路径,两种情况下都是绿的)。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +4800 to +4802
return {
ok: false,
reason: admission.impossible ? 'window_impossible' : 'window_full',

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.

[Critical] R1-11: [fails-closed] [new-surface] window_full never evicts a page to make room and never latches the rejected footprint, so random access permanently fails in a saturated window — the eviction half of issue #10750's incident is unimplemented.

Issue #10750: "retained blocks are capped by count and bytes. Consequently, the rail cannot represent all turns in a long session"; this PR's own narrative: "once the window evicts old content there is no way back."

Failure scenario: in a long session pinned at maxBlocks / maxRetainedBytes, oldest-first trim keeps the store at the cap, so pageBlocks + current.blocks.length > maxBlocks (:466) is true for every anchored page. admitTranscriptWindowPage calls materializeTranscriptHistory once, gets admitted: false and returns window_full — it never evicts a ledger entry ("evicting whole pages farthest from the target first", design flow step 4), and TranscriptPageLedger exposes no interior-eviction primitive at all. It also never sets the rejectedPage footprint the design says window_full carries over ("the rejected page's footprint is remembered and the same page is re-offered only once enough capacity exists"), so no later trim re-offers it either. Net: Phase 3's rail gets a permanent window_full for every unloaded turn in exactly the long-session regime #10750 exists to fix.

Witness:

probe + latch-half flip: the saturated-window admission returns window_full with no eviction
attempted and no rejectedPage footprint recorded; design flow step 4 and the re-offer latch have
no implementation anywhere in the diff (TranscriptPageLedger's mutator set is clear/recordInitialLoad/
recordPrepend/setOlderGap/insertEntry/updateEntry/applyPrefixTrim/applyRewind — no interior eviction).

When !admission.admitted && !admission.impossible, evict whole ledger entries farthest from the target (a new TranscriptPageLedger.removeEntry(id) plus a provider-directed store.reset over the retained pages), then re-run materializeTranscriptHistory once. If it still does not fit, return window_full and latch the footprint next to history.rejectedPage so the existing trim-time re-open gate (:1190-1240) re-offers it.

Two premises the fix must respect: the live tail must never be the eviction victim (design ledger invariant — "Streaming keeps writing through the existing batcher; historical admission never touches tail blocks"), and a count trim leaves no slack to absorb a page — :1206-1207: "A count trim restores the window to exactly maxBlocks (zero headroom), so without this check every live block during streaming would re-open the latch into an immediate fetch/reject cycle."

Please add the test that pins this — fill the window with an admitted historical page far from the target plus a small live tail under a tight maxBlocks, then assert openTranscriptAtTurn(target) resolves {ok:true, targetRecordId}, the far page's blocks are gone and the live-tail text survives — and confirm it goes red today (it returns {ok:false, reason:'window_full'}). The existing window_full / window_impossible case must stay green.

中文说明

window_full 既不会为腾出空间而驱逐任何页,也不会记录被拒页的 footprint,因此在窗口饱和时随机访问会永久失败——issue #10750 所述事故中「驱逐后无法回头」这一半并未实现。

issue #10750:「保留的 block 受数量与字节双重上限约束。因此导航条无法表示长会话中的全部 turn」;本 PR 自述:「窗口一旦驱逐旧内容就无法回头」。

触发场景: 在已被 maxBlocks / maxRetainedBytes 顶满的长会话中,oldest-first 裁剪使 store 始终贴着上限,因此 pageBlocks + current.blocks.length > maxBlocks:466)对每一个锚定页都成立。admitTranscriptWindowPage 只调用一次 materializeTranscriptHistory,得到 admitted: false 后返回 window_full——它从不驱逐台账 entry(设计流程第 4 步要求「优先驱逐离目标最远的整页」),而 TranscriptPageLedger 根本没有暴露任何内部驱逐原语。它也不会设置设计文档所说 window_full 应当承接的 rejectedPage footprint(「被拒页的 footprint 会被记住,只有在容量足够时才重新提供同一页」),因此后续裁剪也不会重新提供它。最终结果:Phase 3 的导航条在 #10750 正是为解决其而存在的长会话场景下,对每一个未加载的 turn 都只能得到永久的 window_full

证据:

probe + latch 半边翻转:饱和窗口下的入账返回 window_full,既未尝试驱逐,也未记录 rejectedPage
footprint;设计流程第 4 步与重新提供闩锁在本 diff 中完全没有实现(TranscriptPageLedger 的变更方法集为
clear/recordInitialLoad/recordPrepend/setOlderGap/insertEntry/updateEntry/applyPrefixTrim/applyRewind
——没有内部驱逐)。

修复方向:当 !admission.admitted && !admission.impossible 时,驱逐离目标最远的整条台账 entry(新增 TranscriptPageLedger.removeEntry(id),并由 provider 在保留页上执行定向 store.reset),然后重跑一次 materializeTranscriptHistory。若仍放不下,返回 window_full 把 footprint 记录到 history.rejectedPage 旁,使既有的裁剪期重开闸门(:1190-1240)能够重新提供该页。

约束:修复必须尊重两个前提——实时尾部绝不能成为驱逐对象(设计台账不变量:「流式写入继续经由既有 batcher;历史入账绝不触碰尾部 block」);且按数量裁剪不留任何余量——:1206-1207:「按数量裁剪会把窗口恢复到恰好 maxBlocks(零余量),因此没有这个检查的话,流式期间每一个实时 block 都会重新打开闩锁,陷入立即抓取/立即拒绝的循环。」

请补上能锁定该行为的测试:在紧凑的 maxBlocks 下,用一个远离目标的已入账历史页加一小段实时尾部填满窗口,然后断言 openTranscriptAtTurn(target) 返回 {ok:true, targetRecordId}、远端页的 block 已消失、实时尾部文本仍在——并确认该测试今天是红的(当前返回 {ok:false, reason:'window_full'})。现有的 window_full / window_impossible 用例必须保持绿色。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +4805 to +4807
const ledger = transcriptPageLedgerRef.current;
const state = store.getSnapshot();
store.reset(

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.

[Critical] R1-12: [fails-closed] [new-surface] Anchored and continuation admissions are merged into the visible store but not into the open live-journal repair episode's checkpoint, so a repair reload silently discards the historical page the user jumped to.

#10750 acceptance criterion: "A live session remains connected and continues receiving output while the user inspects historical pages."

Failure scenario: attaching to a session restored mid-turn creates a repair episode with checkpoint = replayStore.getSnapshot() (:2647, :2695-2698). While the turn streams, openTranscriptAtTurn(turnId) splices the page into the visible store and records the ledger entry but never touches repair.checkpoint — contrast loadMoreTranscript at :4617-4621, which does repair.checkpoint = applyTranscriptHistory(repair.checkpoint, historyMaterialization) for exactly this reason. The turn then completes, the session goes idle, tryLiveJournalRepair fires reloadSession(..., {replaySource:'memory'}); markerStillVisible is true (an insert does not remove the marker block, :2373-2379), so replayStore is seeded from the stale checkpoint and store.reset({...replayState}) replaces the visible store. The anchored page's blocks are gone and the ledger is rebuilt by recordInitialLoad, so the loss is silent — the turn the user was reading disappears with no notice, while prepended history in the same window survives.

Witness:

not run — no successful-repair fixture exists in the suite to drive this against. The asymmetry with
the prepend path's checkpoint merge at DaemonSessionProvider.tsx:4617-4621 was traced at ebb7c62c:
that path merges into repair.checkpoint, the anchored path does not, and markerStillVisible (:2373-2379)
is the gate that makes the stale checkpoint authoritative on the rebuild.

Apply the same checkpoint merge on the anchored path — after the store.reset, when liveJournalRepairRef.current?.sessionId === activeSession.sessionId, rebase repair.checkpoint with applyTranscriptPageInsert at the page's position within the checkpoint state — or, if that position is not derivable, invalidate the open repair episode (liveJournalRepairRef.current = undefined) whenever a non-prepend page is admitted, so the rebuild cannot drop admitted content.

The prepend path's merge is the shape to match (:4617-4621), and the checkpoint is only used when the marker survives (markerStillVisible, :2373-2379).

Please add the test that pins this — attach with a live-journal repair marker in the replay snapshot, admit an anchored page via openTranscriptAtTurn, drive the repair (terminal + idle) and assert the anchored block's text is still in useDaemonTranscriptBlocks() — and confirm it goes red today.

中文说明

锚定与 continuation 入账只合并进可见 store,没有合并进当前打开的 live-journal 修复回合的 checkpoint,因此一次修复重载会静默丢弃用户刚刚跳转到的历史页。

#10750 验收标准:「用户查看历史页时,实时会话保持连接并继续接收输出。」

触发场景: 附加到一个在 turn 中途恢复的会话时,会创建修复回合,其 checkpoint = replayStore.getSnapshot():2647:2695-2698)。在该 turn 流式进行期间,openTranscriptAtTurn(turnId) 把页拼接进可见 store 并记录台账 entry,但从不触碰 repair.checkpoint——对比 :4617-4621loadMoreTranscript,它正是为此执行 repair.checkpoint = applyTranscriptHistory(repair.checkpoint, historyMaterialization)。随后该 turn 完成、会话进入 idle,tryLiveJournalRepair 触发 reloadSession(..., {replaySource:'memory'});此时 markerStillVisible 为真(插入不会移除 marker block,:2373-2379),于是 replayStore过期的 checkpoint 播种,store.reset({...replayState}) 替换可见 store。锚定页的 block 消失,台账被 recordInitialLoad 重建,因此丢失是无声的——用户正在阅读的那个 turn 毫无提示地消失,而同一窗口内 prepend 进来的历史却得以保留。

证据:

not run — 测试套件中不存在可用于驱动该路径的「修复成功」fixture。已在 ebb7c62c 上追踪其与 prepend
路径 checkpoint 合并(DaemonSessionProvider.tsx:4617-4621)的不对称:该路径会合并进 repair.checkpoint,
锚定路径不会,而 markerStillVisible(:2373-2379)正是在重建时让过期 checkpoint 成为权威来源的闸门。

修复方向:在锚定路径上做同样的 checkpoint 合并——store.reset 之后,当 liveJournalRepairRef.current?.sessionId === activeSession.sessionId 时,用 applyTranscriptPageInsertcheckpoint state 内部的对应位置重建 repair.checkpoint;若该位置无法推导,则在入账任何非 prepend 页时作废当前修复回合(liveJournalRepairRef.current = undefined),使重建不可能丢弃已入账内容。

约束:应以 prepend 路径的合并为范式(:4617-4621),且 checkpoint 只在 marker 存活时才被使用(markerStillVisible:2373-2379)。

请补上能锁定该行为的测试:在 replay 快照中带有 live-journal 修复 marker 的情况下附加会话,通过 openTranscriptAtTurn 入账一个锚定页,驱动修复(terminal + idle),断言该锚定 block 的文本仍存在于 useDaemonTranscriptBlocks() 中——并确认该测试今天是红的。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +5065 to +5066
mintForwardCursor: true,
buttAfter: true,

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.

[Critical] R1-3: [certifies-falsely] [new-surface] continueTranscriptNewer passes buttAfter: true unconditionally, so insertEntry overwrites the displaced older-gap locator with {} even when the page reported hasMore — the range becomes an untracked hole.

A forward continuation butts against its predecessor (gaps[index]), not its successor, yet buttAfter writes gaps[index+1]. This is the same call that mints a forward nextCursor precisely because page.hasMore was true.

Failure scenario: entries [A(anchored, records 1..5, nextCursor 'cursor-1'), L(load, records 50..60)], gaps [{older:record-1},{older:record-50},{}] — the range 10..49 between A and L is unloaded but has a locator. continueTranscriptNewer(A) returns records 6..9 with hasMore:true, nextCursor:'cursor-2'. The gaps become [{older:record-1},{},{},{}]: the {older:{beforeRecordId:'record-50'}} locator for records 10..49 is destroyed, and the published useDaemonTranscriptLedger() view now asserts N and L are contiguous while the same call records that more content remains ahead. The only surviving handle is N's nextCursor, which applyPrefixTrim and applyRewind both clear (transcriptPageLedger.ts:270, :334); after either, records 10..49 are an untracked hole with no gap sentinel — precisely what the ledger exists to prevent (:1035-1037, "eviction leaves re-fetchable locators instead of untracked holes"). The same overwrite closes the trailing gap toward the live tail when A is the newest entry and the continuation did not reach the live blocks.

Witness:

executed against the real TranscriptPageLedger:
  before: gaps [{older:record-1},{older:{beforeRecordId:'record-50'}},{}]
  after continueTranscriptNewer (hasMore:true): gaps [{older:record-1},{},{},{}]
fix flip (buttAfter: page.hasMore !== true) -> the displaced {older:{beforeRecordId:'record-50'}} survives
Suggested change
mintForwardCursor: true,
buttAfter: true,
mintForwardCursor: true,
buttAfter: page.hasMore !== true,

A preserved-but-redundant locator is benign (a re-fetch at that beforeRecordId is dropped by materializeTranscriptHistory's record-id dedup); a destroyed one is unrecoverable. Leave continueTranscriptOlder's buttAfter: true alone — a backward page always butts against the entry it continued from.

Two facts the fix must respect: insertEntry's contract (transcriptPageLedger.ts:197-204) is that the displaced gap becomes the gap after the new page unless an explicit gapAfter is passed, and the gaps array must stay entries.length + 1 long; and hasMore is journal-relative, not window-relative (session-transcript-reader.ts:3477-3480), so hasMore === false is sufficient but not necessary for adjacency — do not read hasMore === true as proof the next entry is unreachable.

Please extend continues newer with the stored cursor sent alone with a continuation page carrying hasMore:true, nextCursor:'cursor-2' and a following entry whose older gap is populated, asserting getLedger()?.gaps[2] still equals the displaced {older:{beforeRecordId:...}} — and confirm it goes red without the fix. The current test uses hasMore:false, where closing is correct, so it cannot distinguish the two.

中文说明

continueTranscriptNewer 无条件传入 buttAfter: true,因此即使页报告了 hasMoreinsertEntry 也会用 {} 覆盖被挤位的 older gap 定位器——该区间由此变成无法追踪的空洞。

向前方向的 continuation 紧贴的是它的前驱gaps[index]),而不是后继,但 buttAfter 写的是 gaps[index+1]。而正是同一次调用,因为 page.hasMore 为真才生成了向前的 nextCursor

触发场景: entries 为 [A(anchored, 记录 1..5, nextCursor 'cursor-1'), L(load, 记录 50..60)],gaps 为 [{older:record-1},{older:record-50},{}]——A 与 L 之间的 10..49 区间尚未加载但有定位器。continueTranscriptNewer(A) 返回记录 6..9,hasMore:true, nextCursor:'cursor-2'。gaps 变为 [{older:record-1},{},{},{}]:记录 10..49 的 {older:{beforeRecordId:'record-50'}} 定位器被销毁,而对外发布的 useDaemonTranscriptLedger() 视图此时断言 N 与 L 连续,同一次调用却又记录了前方仍有内容。唯一残存的抓手是 N 的 nextCursor,而 applyPrefixTrimapplyRewind 都会清除它(transcriptPageLedger.ts:270:334);两者之一发生后,记录 10..49 就成为没有 gap 哨兵的不可追踪空洞——这正是台账存在的目的所要避免的(:1035-1037:「驱逐留下的是可重新抓取的定位器,而不是无法追踪的空洞」)。当 A 是最新 entry 且 continuation 未抵达实时 block 时,同样的覆盖会关闭通向实时尾部的末尾 gap。

证据:

在真实 TranscriptPageLedger 上执行:
  之前:gaps [{older:record-1},{older:{beforeRecordId:'record-50'}},{}]
  continueTranscriptNewer(hasMore:true)之后:gaps [{older:record-1},{},{},{}]
修复翻转(buttAfter: page.hasMore !== true)-> 被挤位的 {older:{beforeRecordId:'record-50'}} 得以保留

保留一个冗余定位器是无害的(在该 beforeRecordId 上的重新抓取会被 materializeTranscriptHistory 的 record-id 去重丢弃);销毁一个则是不可恢复的。continueTranscriptOlderbuttAfter: true 应保持不动——向后方向的页总是紧贴它所延续的那个 entry。

约束:修复必须尊重两点——insertEntry 的契约(transcriptPageLedger.ts:197-204)是「被挤位的 gap 成为新页之后的 gap,除非显式传入 gapAfter」,且 gaps 数组长度必须保持 entries.length + 1;以及 hasMore 是相对于 journal 而非窗口的(session-transcript-reader.ts:3477-3480),因此 hasMore === false 对「紧贴」是充分条件但非必要条件——不要把 hasMore === true 读作「下一个 entry 不可达」的证明。

请把 continues newer with the stored cursor sent alone 扩展为使用携带 hasMore:true, nextCursor:'cursor-2' 的 continuation 页、且其后继 entry 的 older gap 非空,断言 getLedger()?.gaps[2] 仍等于被挤位的 {older:{beforeRecordId:...}}——并确认没有修复时该测试变红。当前测试用的是 hasMore:false,此时关闭 gap 是正确的,因此它无法区分这两种情形。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +275 to +277
firstBlockId: newFirst.id,
firstRecordId:
detail.oldestRetainedRecordId ?? firstRecordIdOf(retainedSlice),

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.

[Critical] R1-6: [certifies-falsely] [new-surface] applyPrefixTrim (and applyRewind at :359-364) shrink a straddling entry's block range, record ids and byteSize, but leave turnIds describing the pre-trim range — so the entry keeps advertising record ids whose blocks were just evicted.

Failure scenario: the initial-load entry E covers blocks b-1..b-100 with turnIds = [r-1..r-50]. Streaming overflows maxBlocks / maxRetainedBytes; the SDK fires onTruncation with blockCount = 60, so cut = 40 straddles E. The reconciler rewrites E.firstBlockId / firstRecordId / byteSize but E.turnIds still contains r-1..r-20, whose blocks are gone. The user then jumps back to evicted turn r-10 through the action this same PR ships: indexStore.findTurn('r-10') succeeds (index metadata survives eviction), and computeLedgerInsertIndex walks for (const turnId of entry.turnIds)E's minOrdinal is 1 from the stale r-1, 1 > 10 is false, so it returns entries.length = 1 instead of 0. computeInsertBlockIndex then splices the fetched older page at blockIndexById[E.lastBlockId] + 1, i.e. after turns 21-50: the flat render store shows turn ~5-15 content below newer history. This breaks the ledger's documented invariant (entries "cover exactly the block ranges present in the store") and poisons every later insert-index computation plus the useDaemonTranscriptLedger() read model. With turnIds recomputed over retainedSlice, minOrdinal would be 21 > 10 -> index 0 -> correct order.

Witness:

not run — traced through both reconcilers and computeLedgerInsertIndex at ebb7c62c. The existing
straddle test builds its entry via pageInput, which defaults turnIds: [], so nothing pins the
recompute: giving it turnIds ['r-1','r-2','r-3'] and asserting ['r-2','r-3'] after
applyPrefixTrim(preTrim, {blockCount:3, oldestRetainedRecordId:'r-2'}) is RED today.

Extract the collection loop from ledgerEntryFromBlocks into a turnIdsOf(blocks) helper and recompute it in both shrink branches: add turnIds: turnIdsOf(retainedSlice) to the straddled entry literal in applyPrefixTrim, and next to lastRecordId: lastRecordIdOf(retainedSlice) in applyRewind. Fully dropped entries already disappear with their ids, and entries kept untouched (firstIndex >= cut) keep valid ids.

The recompute must keep all retained record ids, not only index-known ones: computeLedgerInsertIndex treats an entry with no index-known turn ids as newest (DaemonSessionProvider.tsx:613-615, documented at :598-600 as "Entries without index-known turn ids are live-tail-adjacent and count as newer than everything"), and this file's :52-54 states "turnIds collects every persisted record id present on the page (a superset of navigation turn ids until the turn-index store narrows it)" — narrowing to index-known ids would flip retained-but-unindexed entries to "newer than everything".

Please add that assertion to the existing "shrinks a straddling entry to the oldest retained block" case (and mirror it in the rewind straddle case), plus a provider case that trims first and then asserts openTranscriptAtTurn on an evicted turn yields entries[0] as the anchored page — and confirm each goes red with the recompute removed.

中文说明

applyPrefixTrim(以及 :359-364applyRewind)会收缩跨切 entry 的 block 区间、record id 与 byteSize,却把 turnIds 留成裁剪前的区间——于是该 entry 继续宣称拥有那些 block 已被驱逐的 record id。

触发场景: 初始加载 entry E 覆盖 block b-1..b-100turnIds = [r-1..r-50]。流式输出使 maxBlocks / maxRetainedBytes 溢出,SDK 以 blockCount = 60 触发 onTruncation,于是 cut = 40 跨切 E。对账逻辑改写了 E.firstBlockId / firstRecordId / byteSize,但 E.turnIds 仍包含 r-1..r-20,而这些 block 已经消失。用户随后通过本 PR 自身提供的动作跳回被驱逐的 turn r-10indexStore.findTurn('r-10') 成功(索引元数据不受驱逐影响),computeLedgerInsertIndex 遍历 for (const turnId of entry.turnIds)——EminOrdinal 因过期的 r-1 而为 11 > 10 为假,于是返回 entries.length = 1 而不是 0computeInsertBlockIndex 随后把抓取到的更早页拼接在 blockIndexById[E.lastBlockId] + 1,即 turn 21-50 之后:扁平渲染 store 把 turn ~5-15 的内容显示在更新的历史下方。这破坏了台账文档化的不变量(entry「恰好覆盖 store 中存在的 block 区间」),并污染之后每一次插入位置计算以及 useDaemonTranscriptLedger() 读模型。若基于 retainedSlice 重算 turnIdsminOrdinal 将是 21 > 10 -> 下标 0 -> 顺序正确。

证据:

not run — 在 ebb7c62c 上沿两个对账器与 computeLedgerInsertIndex 逐行追踪。现有的跨切测试通过 pageInput
构造 entry,而它默认 turnIds: [],因此没有任何断言锁定这次重算:给它 turnIds ['r-1','r-2','r-3'] 并在
applyPrefixTrim(preTrim, {blockCount:3, oldestRetainedRecordId:'r-2'}) 之后断言得到 ['r-2','r-3'],
今天是红的。

修复方向:把 ledgerEntryFromBlocks 中的收集循环抽成 turnIdsOf(blocks) 辅助函数,并在两个收缩分支中重算——在 applyPrefixTrim 的跨切 entry 字面量中加入 turnIds: turnIdsOf(retainedSlice),在 applyRewind 中放在 lastRecordId: lastRecordIdOf(retainedSlice) 旁边。被完全丢弃的 entry 本就随其 id 一起消失,未被触碰的 entry(firstIndex >= cut)其 id 仍然有效。

约束:重算必须保留全部存活的 record id,而不只是索引已知的那些——computeLedgerInsertIndex 把没有任何索引已知 turn id 的 entry 视为最新(DaemonSessionProvider.tsx:613-615,其 :598-600 注释为「没有索引已知 turn id 的 entry 紧邻实时尾部,视为比一切都新」),且本文件 :52-54 写明「turnIds 收集页上存在的每一个持久化 record id(在 turn-index store 收窄之前,它是导航 turn id 的超集)」——收窄为索引已知 id 会把「已保留但未被索引」的 entry 翻转成「比一切都新」。

请把上述断言加入现有的「shrinks a straddling entry to the oldest retained block」用例(并在 rewind 跨切用例中镜像一份),另加一个 provider 用例:先裁剪,再断言对被驱逐 turn 执行 openTranscriptAtTurn 时锚定页落在 entries[0]——并确认移除重算后每个用例都变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +331 to +333
const next = new Map(this.pages);
next.set(page.start, this.retain(page));
this.pages = next;

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.

[Critical] R1-7: [fails-closed] [new-surface] status: 'error' is one-way — only seed() assigns 'ready' (:235) — so one transient failure disables turn navigation for the rest of the session even while the store keeps serving fresh pages.

Failure scenario: the session seeds fine ('ready'). A prompt terminal triggers refreshTail() (provider :3397); its validation fetch fails once with a transient 500 / network blip, so handleFetchError sets status = 'error' (:626) and schedules a bounded retry. The retry — or the next turn_complete's refreshTailsucceeds: snapshot, totalTurns and pages are repopulated and retryAttempts is reset to 0. But none of refreshTail's three success paths (:392-397, :400-405, :428-431) touches status, and refreshTail cannot reach seed() because this.snapshot !== undefined (:356). From then on getState().status reads 'error' forever, so openTranscriptAtTurn short-circuits at if (indexStatus !== 'ready') (provider :4866-4868) and returns {ok:false, reason:'unavailable'} for every turn, and useDaemonTurnIndex() hands Phase 3 a permanently-'error' rail over a fully populated, working index. Only a rewind or a 409 routes through invalidateAndReseed() -> seed(). The same omission in fetchOlderSlice also skips this.retryAttempts = 0, so older-page failures consume the MAX_RETRY_ATTEMPTS = 3 budget cumulatively across a session even when each one recovered; after the third recovered blip, handleFetchError returns at :628 and later older-page failures are never retried at all. The asymmetry is the evidence: all three refreshTail success paths do the retryAttempts half of the recovery bookkeeping, seed() does both halves, and fetchOlderSlice does neither.

Witness:

AFTER_SEED               status=ready
AFTER_LIFECYCLE_REJECT   status=error  scheduled=[1000]
AFTER_RETRY_SUCCESS      status=error  totalTurns=4 snapshot=snap-b pages=[0,3]   # retry SUCCEEDED
CONTRAST (seed path)     SEED_FAIL status=error -> SEED_RETRY_OK status=ready

Do the recovery bookkeeping once, in the post-admission hook every path already calls, instead of duplicating it per path:

Suggested change
const next = new Map(this.pages);
next.set(page.start, this.retain(page));
this.pages = next;
private afterAdmission(): void {
this.status = 'ready';
this.retryAttempts = 0;
this.reconcileLiveEntries();
this.evictLru();
this.notify();
}

…then drop the now-redundant this.status = 'ready'; this.retryAttempts = 0; from seed() and the three retryAttempts = 0 lines in refreshTail().

'unsupported' and 'disabled' must not be resurrected by an unconditional assignment: markUnsupported() latches with this.status = 'unsupported'; (:263) and bumps generation, so no admission path can reach afterAdmission() afterwards (each checks if (generation !== this.generation) return; right after its await), and the constructor's this.status = this.enabled ? 'idle' : 'disabled'; (:133) is unreachable there because every entry point starts with if (!this.enabled || this.status === 'unsupported') return;. If you would rather not make that generation/latch reasoning load-bearing, guard the assignment as if (this.status === 'error' || this.status === 'loading') this.status = 'ready';.

Please add the test that pins this — seed OK, make fetchPage reject once so refreshTail() lands in 'error', run the scheduled retry so it succeeds, then expect(store.getState().status).toBe('ready') — and confirm it goes red with the restore removed (today it reads 'error'). A provider-level companion (after a transient rejection and a successful retry, openTranscriptAtTurn('turn-1') must still return {ok:true}) pins the user-visible half; neither exists today.

中文说明

status: 'error' 是单向的——只有 seed() 会赋 'ready':235)——因此一次瞬时失败就会在本次会话余下的时间里禁用 turn 导航,即使 store 之后一直在正常提供新页。

触发场景: 会话正常 seed('ready')。某个 prompt 终态触发 refreshTail()(provider :3397),其校验抓取因一次瞬时 500 / 网络抖动失败,于是 handleFetchError 设置 status = 'error':626)并安排有限次重试。重试——或下一次 turn_completerefreshTail——成功了:snapshottotalTurnspages 都被重新填充,retryAttempts 也归零。但 refreshTail 的三条成功路径(:392-397:400-405:428-431)都不触碰 status,而 refreshTail 又因为 this.snapshot !== undefined:356)无法走到 seed()。从此 getState().status 永远是 'error',于是 openTranscriptAtTurnif (indexStatus !== 'ready')(provider :4866-4868)处短路,对每一个 turn 都返回 {ok:false, reason:'unavailable'},而 useDaemonTurnIndex() 交给 Phase 3 的是一条永远 'error' 的导航条,底下的索引其实完整可用。只有 rewind 或 409 才会经由 invalidateAndReseed() -> seed() 清除。fetchOlderSlice 中同样的遗漏还跳过了 this.retryAttempts = 0,因此即便每次都恢复成功,更早页的失败也会在整个会话中累计消耗 MAX_RETRY_ATTEMPTS = 3 预算;第三次恢复之后,handleFetchError 会在 :628 直接返回,之后的更早页失败将完全不再重试。这种不对称正是证据:refreshTail 的三条成功路径都做了恢复记账中 retryAttempts 的那一半,seed() 两半都做,而 fetchOlderSlice 两半都不做。

证据:

AFTER_SEED               status=ready
AFTER_LIFECYCLE_REJECT   status=error  scheduled=[1000]
AFTER_RETRY_SUCCESS      status=error  totalTurns=4 snapshot=snap-b pages=[0,3]   # 重试已成功
对照(seed 路径)        SEED_FAIL status=error -> SEED_RETRY_OK status=ready

修复方向:把恢复记账只做一次,放在每条路径都已经调用的入账后钩子里(见上方 suggestion 块),然后删除 seed() 中已冗余的 this.status = 'ready'; this.retryAttempts = 0;refreshTail() 中的三处 retryAttempts = 0

约束:无条件赋值不得复活 'unsupported''disabled'——markUnsupported()this.status = 'unsupported';:263)闩锁并递增 generation,因此之后没有任何入账路径能抵达 afterAdmission()(每条路径都在 await 之后立刻检查 if (generation !== this.generation) return;);构造函数的 this.status = this.enabled ? 'idle' : 'disabled';:133)在那里也不可达,因为每个入口都以 if (!this.enabled || this.status === 'unsupported') return; 开头。如果不希望让上述 generation/闩锁推理成为承重前提,可把赋值加守卫写成 if (this.status === 'error' || this.status === 'loading') this.status = 'ready';

请补上能锁定该行为的测试:先成功 seed,让 fetchPage 拒绝一次使 refreshTail() 落入 'error',再执行已安排的重试使其成功,然后断言 expect(store.getState().status).toBe('ready')——并确认移除该恢复赋值后测试变红(今天读到的是 'error')。另加一个 provider 层的配套用例(瞬时拒绝并成功重试之后,openTranscriptAtTurn('turn-1') 仍须返回 {ok:true})以锁定用户可见的那一半;目前两者都不存在。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +586 to +587
if (provisional !== undefined) {
this.linkedRecordIds.set(provisional.id, recordIds);

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.

[Critical] R1-54: [certifies-falsely] [new-surface] observeAdmittedBlocks overwrites rather than unions the provisional's linked record ids, so the assistant/thought blocks of the same turn replace the user echo's link — and assistant record uuids are never navigation turn ids, so reconcileLiveEntries' record-UUID fallback can never match.

Failure scenario: prompt P is accepted -> addLivePrompt({promptId:'P'}) (provider :4421, fed from actions.ts:993). The flush feed (:1504) first admits the user echo block (promptId:'P', sourceRecordIds:['<user-record-uuid>']), linking live:P to that uuid; then it admits the assistant/thought blocks of the same turn — same promptId, their own record uuids — and each set() overwrites the link. Assistant blocks really do carry the turn's promptId and their own record uuids (ui/transcript.ts:856-864; envelope field promptId?: string, "Admitted prompt identifier for events belonging to a specific turn", daemon/types.ts:4466-4467; the SDK's own test asserts an assistant block ends with promptId:'prompt-1', sourceRecordIds:['assistant-record']). Index turnIds are always user record uuids (session-transcript-reader.ts:1016-1018 if (record.type !== 'user') return undefined; -> :2160-2166 turnId: uuid). So for a legacy index whose entries carry no promptId, the fallback linked.some((id) => turnIds.has(id)) (:522) can never match: the provisional survives forever, the rail shows a completed turn as still in-flight, and the some(kind === 'prompt') fast paths at :500 and :578 never short-circuit again.

Witness:

PROBE R1-54 CONTROL liveEntries: []                                  # only the user echo block fed
PROBE R1-54 liveEntries: [{"id":"live:prompt-P","kind":"prompt","promptId":"prompt-P","label":"hello"}]
PROBE R1-54 totalTurns/pages: 3 [0,2]                                # the turn's index page WAS admitted
fix flip (union instead of overwrite at :587) -> liveEntries: [] , all 26 store tests still pass
Suggested change
if (provisional !== undefined) {
this.linkedRecordIds.set(provisional.id, recordIds);
if (provisional !== undefined) {
this.linkedRecordIds.set(provisional.id, [
...new Set([...(this.linkedRecordIds.get(provisional.id) ?? []), ...recordIds]),
]);

Scope, so the fix is sized correctly: where a later turn_result record supplies promptId, a subsequent refreshTail retires the provisional by promptId, so the permanent phantom is confined to index entries with no promptId — legacy transcripts, the path this diff deliberately supports (JSDoc :571-575 and the test reconciles a legacy no-prompt-id provisional by record UUID) — plus any turn that never persists a turn_result promptId.

Please add the test that pins this — addLivePrompt, then observeAdmittedBlocks with a user-echo block followed by an assistant block for the same promptId, then admit an index page whose turnId is the user record uuid, and assert liveEntries is empty — and confirm it goes red with the union removed (today the provisional survives).

中文说明

observeAdmittedBlocks 对临时项已关联的 record id 是覆盖而非并集,因此同一 turn 的 assistant/thought block 会替换掉 user 回显 block 的关联——而 assistant 的 record uuid 永远不是导航 turn id,于是 reconcileLiveEntries 的 record-UUID 回退分支永远无法匹配。

触发场景: prompt P 被接受 -> addLivePrompt({promptId:'P'})(provider :4421,由 actions.ts:993 触发)。flush 馈送(:1504)先入账 user 回显 block(promptId:'P'sourceRecordIds:['<user-record-uuid>']),把 live:P 关联到该 uuid;随后入账同一 turn 的 assistant/thought block——promptId 相同、record uuid 各不相同——每次 set() 都覆盖前一次的关联。assistant block 确实携带该 turn 的 promptId 和自己的 record uuid(ui/transcript.ts:856-864;envelope 字段 promptId?: string,「属于特定 turn 的事件所对应的已接受 prompt 标识」,daemon/types.ts:4466-4467;SDK 自身的测试断言 assistant block 最终带有 promptId:'prompt-1'sourceRecordIds:['assistant-record'])。而索引的 turnId 永远是 user 记录的 uuid(session-transcript-reader.ts:1016-1018if (record.type !== 'user') return undefined; -> :2160-2166turnId: uuid)。因此对于条目不带 promptId 的旧式索引,回退分支 linked.some((id) => turnIds.has(id)):522)永远无法匹配:临时项永久存活,导航条把一个已完成的 turn 显示为仍在进行,且 :500:578some(kind === 'prompt') 快速路径再也无法短路。

证据:

PROBE R1-54 CONTROL liveEntries: []                                  # 只馈送 user 回显 block
PROBE R1-54 liveEntries: [{"id":"live:prompt-P","kind":"prompt","promptId":"prompt-P","label":"hello"}]
PROBE R1-54 totalTurns/pages: 3 [0,2]                                # 该 turn 的索引页确实已入账
修复翻转(在 :587 改为并集而非覆盖)-> liveEntries: [],且全部 26 个 store 测试仍通过

修复方向见上方 suggestion 块:改为并集。

范围界定(便于把修复做准):当后续的 turn_result 记录提供了 promptId 时,之后的 refreshTail 会按 promptId 清除该临时项,因此永久性幽灵仅局限于不带 promptId 的索引条目——即本 diff 刻意支持的旧式 transcript(JSDoc :571-575 与测试 reconciles a legacy no-prompt-id provisional by record UUID)——外加任何从未持久化 turn_result promptId 的 turn。

请补上能锁定该行为的测试:addLivePrompt,然后用同一 promptId 的 user 回显 block 与 assistant block 依次调用 observeAdmittedBlocks,再入账一个 turnIduser record uuid 的索引页,断言 liveEntries 为空——并确认移除并集后测试变红(当前临时项会存活)。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines 992 to +993
options?.onAdmitted?.();
onPromptAdmitted?.({ promptId: accepted.promptId, label: text });

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.

[Critical] R1-53: [certifies-falsely] [new-surface] onPromptAdmitted carries no session identity and fires with no session guard, so a prompt admitted to session A is written into session B's turn-index store as a phantom live: entry that can never be reconciled away.

The payload type is {promptId, label} with no sessionId (:220), and both fires are post-await with no identity re-check — while adjacent lines do re-check (:989 if (activePromptsRef.current.get(sessionId)?.controller === ctrl), :1000 if (sessionRef.current?.sessionId === sessionId)). The provider handler writes through a ref (turnIndexStoreRef.current?.addLivePrompt(...), provider :4420-4425).

Failure scenario: the user sends a prompt in session A, then clicks another session in the sidebar while session.submitPrompt(...) is in flight (loadSession has no prompt-status gate). startSessionSwitch only rejects the waiter and deletes the map entry (:742-752), never aborting the controller — the sole aborter clearActiveSessionState (:585-592) runs only from clearSession (:1790) and newSession (:1818) — so A's admission still resolves. The provider's run loop has by then disposed A's store and installed B's, so addLivePrompt writes live:prompt-A, labelled with A's prompt text, into B's liveEntries. addLivePrompt has no session gate and reconcileLiveEntries (turnIndexStore.ts:499-527) matches only promptIds / record ids from that session's pages, so B can never retire it. This also defeats the invariant this diff states at provider :5109-5111 — "a store created for a previous session must never leak its snapshot-bound pages into the new session's consumers".

Witness:

PROBE R1-53 [connected A]          session-a []
PROBE R1-53 [switched to B]        session-b []
PROBE R1-53 [A admission resolved] session-b [{"id":"live:prompt-A","kind":"prompt",
                                                 "promptId":"prompt-A","label":"hello from A"}]
  -> AssertionError: expected [ { id: 'live:prompt-A', ...(3) } ] to deeply equal []
fix flip (sessionId on the payload + guard on indexStore.getState().sessionId)
  -> session-b [] , and the PR's own 'appends a live provisional when a prompt is admitted' still passes

Add sessionId to the onPromptAdmitted payload and have the provider handler drop the signal when it does not match the store's session. Compare against turnIndexStoreRef.current.getState().sessionId, not only sessionRef.current.

The store-replacement gate already compares turnIndexStoreRef.current.getState().sessionId !== activeSession.sessionId (provider :2806-2808), so the guard must key on the store's sessionId — the same-session reconnect path leaves sessionRef.current unchanged while the store instance is swapped.

Please add the test that pins this — admit a prompt with a deferred submitPrompt, switch sessions, resolve the submit, and assert the new session's getTurnIndex()?.liveEntries is empty — and confirm it goes red without the guard.

中文说明

onPromptAdmitted 不携带会话身份、触发时也没有会话守卫,因此提交给会话 A 的 prompt 会被写入会话 B 的 turn-index store,成为一个永远无法被对账清除的幽灵 live: 条目。

其载荷类型是 {promptId, label},没有 sessionId:220),且两处触发都位于 await 之后而没有身份复核——而相邻的行确实做了复核(:989if (activePromptsRef.current.get(sessionId)?.controller === ctrl):1000if (sessionRef.current?.sessionId === sessionId))。provider 侧的处理函数通过 ref 写入(turnIndexStoreRef.current?.addLivePrompt(...),provider :4420-4425)。

触发场景: 用户在会话 A 中发送 prompt,随后在 session.submitPrompt(...) 仍在进行时于侧边栏点击另一个会话(loadSession 没有 prompt 状态闸门)。startSessionSwitch 只拒绝等待者并删除 map 条目(:742-752),从不 abort 该 controller——唯一的 abort 者 clearActiveSessionState:585-592)只在 clearSession:1790)与 newSession:1818)中被调用——因此 A 的入账仍会 resolve。此时 provider 的运行循环已经销毁 A 的 store 并装入 B 的 store,于是 addLivePrompt 把带着 A 的 prompt 文本的 live:prompt-A 写进了 B 的 liveEntriesaddLivePrompt 没有会话闸门,而 reconcileLiveEntriesturnIndexStore.ts:499-527)只按本会话索引页中的 promptId / record id 匹配,因此 B 永远无法清除它。这也违背了本 diff 在 provider :5109-5111 自己声明的不变量——「为上一个会话创建的 store 绝不能把它绑定快照的页泄漏给新会话的消费者」。

证据:

PROBE R1-53 [已连接 A]            session-a []
PROBE R1-53 [切换到 B]            session-b []
PROBE R1-53 [A 的入账 resolve]    session-b [{"id":"live:prompt-A","kind":"prompt",
                                              "promptId":"prompt-A","label":"hello from A"}]
  -> AssertionError: expected [ { id: 'live:prompt-A', ...(3) } ] to deeply equal []
修复翻转(载荷带上 sessionId + 以 indexStore.getState().sessionId 做守卫)
  -> session-b [],且 PR 自身的 'appends a live provisional when a prompt is admitted' 仍然通过

修复方向:为 onPromptAdmitted 载荷加上 sessionId,并让 provider 处理函数在与 store 的会话不匹配时丢弃该信号。要与 turnIndexStoreRef.current.getState().sessionId 比较,而不只是 sessionRef.current

约束:store 替换闸门本身比较的就是 turnIndexStoreRef.current.getState().sessionId !== activeSession.sessionId(provider :2806-2808),因此守卫必须以 store 的 sessionId 为键——同会话重连路径会在 store 实例被替换的同时保持 sessionRef.current 不变。

请补上能锁定该行为的测试:以挂起的 submitPrompt 入账一个 prompt,切换会话,再让 submit 返回,断言新会话的 getTurnIndex()?.liveEntries 为空——并确认没有守卫时测试变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Closing for the same reason as #11053: #11054 merged as 9c0fcbb78 on 2026-09-06 and delivers this Phase 2 client data layer through a separate historical page table with its own memory budget. One phase, one implementation.

To be clear about what this PR was: of the three, it was the leanest and the closest to the merged Phase 2 design document — 3,568 added lines against 5,941, the module boundaries and file names the design named, and the sequential-pagination migration kept in the order the design prescribed rather than deferred. What did not survive is the structural decision underneath it, not the execution.

Why the other route was kept. Sharing the live window's admission budget means a jump competes with live output: at capacity a valid anchor is refused, and a streaming session offers no bounded moment at which space frees again. The design's own open question already conceded that fixing that through interior page eviction may require a new range-delete method in the SDK store. A separate historical budget gets the property structurally, which is what #10750's acceptance criteria ask for.

Exposures recorded before closing, so they are not rediscovered later. An anchored page's surviving records are spliced at a single computed offset, so a dedup hole inside the page can place newer records ahead of a retained page while the open still reports success — the same shape as R2-22 on #11053. The ledger insert search treats an entry whose turn ids do not resolve against currently retained index pages as newer than everything, which lands blocks in the wrong order rather than merely misplacing a rail highlight; the risk note in the description covered the incoming page having no navigation turns, which is the narrower case. And rewind reaches the index only through the store's truncation callback, which is emitted only when a block is actually dropped. All three were latent — nothing called the new surface — and none applies to the implementation on main.

One correction to the description: it cited #10968 as part of the Phase 1 protocol. That is an unrelated CI fix; the protocol landed in #10751, with the routing follow-up in #11047.

Worth lifting: the persisted-identity plumbing onto the message layer, with its adapter tests, if Phase 3 decides it needs a message-to-turn mapping in the render path. Main locates turns inside the navigation store instead, so that is an open decision on #10750 rather than a gap.

Conflicts with main in four files today. Tracking stays on #10750.

中文说明

#11053 同因关闭:#11054 已于 2026-09-06 合并为 9c0fcbb78,以独立历史页表 + 独立内存预算实现了本阶段数据层。同一阶段只保留一个实现。

需要说清的是:三者之中本 PR 最精简、也最贴合已合并的 Phase 2 设计文档——3568 行对 5941 行,模块边界与文件命名都与设计一致,并按设计要求先做顺序分页迁移而非推迟。落选的是其下的结构决策,不是实现质量。

与实时窗口共用准入预算意味着跳转与实时输出争抢容量:满载时合法锚点被拒,而持续产出的会话没有确定的腾位时刻。设计自身的 open question 已承认,用内部页淘汰来修可能需要在 SDK store 新增 range-delete。独立历史预算从结构上满足 #10750 的验收标准。

关闭前登记三处暴露,避免以后重新发现:一是锚定页的存活记录按单一偏移整段拼接,页内被去重打洞时较新记录可能落到保留页之前,而调用仍返回成功(与 #11053 的 R2-22 同形);二是账本插入位置查找把"turn id 无法在当前保留索引页解析"的条目视为最新,导致 block 落错顺序而不只是高亮错位——描述里的风险提示说的是"来页不含导航 turn",是更窄的情形;三是 rewind 只经 store 的 truncation 回调抵达索引,而该信号仅在确有 block 被丢弃时发出。三者今天都是潜伏的(新面无调用方),也都不适用于 main 上的实现。

描述里有一处需更正:把 #10968 当作 Phase 1 协议的一部分,实为无关的 CI 修复;协议落在 #10751,路由后续在 #11047

值得摘出的部分:消息层持久身份透传及其适配器测试——前提是 Phase 3 确认需要"渲染消息 → durable turn"的映射。main 是在导航 store 内定位的,因此这是 #10750 上的待定项,而非缺口。

与 main 已在 4 个文件上冲突。追踪继续留在 #10750

@doudouOUC doudouOUC closed this Sep 6, 2026
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.

2 participants