Skip to content

fix: advance the transcript per committed event so running timers stop resetting - #5365

Merged
Astro-Han merged 19 commits into
mainfrom
fix/transcript-per-event-watermark
Sep 16, 2026
Merged

Astro-Han merged 19 commits into
mainfrom
fix/transcript-per-event-watermark

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

While a Team or any long Turn runs, the elapsed timer restarted at 0 every time a new tool or live event arrived, and the zh duration read 用时 35秒 with the number glued to its unit.

Root cause

#4879 moved the transcript onto RuntimeEvents and computed the durable watermark from ended Turns. A running Turn therefore never appeared in durable pages; it was only visible through a one-shot active overlay read when the subscription opened (#2922), which went stale immediately. The renderer could not find the running Turn, so overlayLiveTurn synthesized one with startedAt: Date.now() on every projection, and useTurnElapsedTime reset.

Fix: the watermark follows committed events

  • storage: the transcript high water is the newest committed event ordinal; pages select invocations by their own events up to that ordinal, and a transaction-settled hook notifies runtime event commits.
  • runtime: the transcript projector has no active mode; any committed prefix projects to final rows (unclaimed thinking is diagnosed only once its invocation ended).
  • runtime-host: event commits enqueue a coalesced transcript_advanced refresh, held back while a terminal publication is fenced. The active overlay, session.transcript.overlay.release, page source, overlay fragments, the reader's active scan and message-id lookup are deleted. Text already streamed when a subscriber opens is paid out as ordinary deltas from offset 0, bounded by half the subscriber queue; a completion that arrives before the subscriber catches up is deferred behind that payout. Compatibility epoch 158; persisted overlay.release grants are released.
  • desktop / CLI: the replica, preload IPC contract and renderer range store keep durable rows only. Mid-turn refreshMessages calls on text_complete / tool_result (readiness checks only) are removed. Leaving a conversation no longer drops the subscription to a Turn the Host is still running.
  • ui: a live Turn records its first event's timestamp, used instead of Date.now() when the transcript has not reached the Turn yet; zh / zh-TW durations read 用时 3 分 33 秒.

Resume and crash recovery are unaffected: they read the ledger directly, and now the transcript shows every committed step of a running Turn as well.

Catch-up: one delivery order, and far less of it

Review found that replacing the overlay with server-side replay had put catch-up on a path of its own, beside the subscriber's queue. Three defects followed from that single fork, all reproduced against the real coordinator:

  • a message needing no catch-up was delivered and completed while an earlier message was still being paid out, inverting the order the Host produced them in;
  • the fenced terminal publication cleared every unpaid backlog, leaving a subscriber with a truncated answer that the client then reported as complete;
  • replay was replenished on sink.send() resolution — the frame leaving the server, not the client consuming it.

Every assistant frame and projection now passes through one per-subscriber gate, so later work queues behind a prefix still being paid; the terminal publication captures unpaid streams and queues behind the payout instead of dropping them.

Separately, the Desktop was asking for most of that catch-up itself. Leaving a conversation released the subscription whenever no renderer target, watched Turn or transcript consumer remained — with no regard for whether the Host was still producing — so returning opened a second subscription and the Host re-sent the whole in-flight answer from offset 0. Measured on the real coordinator, client and observer: 307,200 characters over 19 frames, 204,800 of which the client had already received. "This Turn is still running, so do not let go" already existed as a retention reason, but only as watchedTurnIds, which only the local submitter populates; a Turn started by the CLI, by another client, or restored after a restart had no such ticket. That fact now comes from the canonical rootTurn, so one authority answers it for every Turn.

Frames start where the subscriber says they can

Review asked for flow control over the replay this PR introduced. Server-side replay turned out to be self-limiting already — the assistant backlog is paid through a half-gate on the subscriber's own queue, and same-stream deltas merge — but reproducing that on the real coordinator, client and Desktop replica surfaced a worse failure the same review points at.

A Client that subscribes mid-answer has to assemble the transcript those frames apply to before it can fold them, and buffers what arrives meanwhile: 32 frames or 256 KiB in the Desktop, 512 frames in the CLI. The Host began sending as soon as the open response flushed, and what it sends first is the entire in-flight answer. Past roughly 258,000 characters the Desktop's byte bound is reached at frame 16, every later frame is dropped, and the subscription never completes preparing — a deterministic livelock at 100% CPU, not a slow-consumer eviction. This is new in this PR: on main the in-flight text was carried inside the open result itself, so there was nothing to buffer.

The Host already had the right gate — a subscriber is not pumped until it is activated — but it was triggering that gate itself, from its own transport progress. A new subscription.ready operation moves the trigger to the consumer: the Host holds every frame, including the in-flight answer and a subscription.closed it has queued, until the subscriber says it can take them. The Desktop replica, the CLI channel and the Desktop Bot adapter each call it once their state is in place, and both pre-readiness buffers are deleted — a frame arriving before readiness is now reported as the contract breach it is. Two "drain the subscription so the bounded transport stays healthy" loops around transcript reads go with them: a read that never declares readiness is never sent anything to drain. The compatibility epoch was already moving for this PR, so the new operation costs nothing extra.

Nested Turns in forward paging

Forward selection was changed to reach running invocations through their own events, but the reader's cursor still advanced past one invocation's bounds, so a Turn nested entirely inside another was never selected and never yielded — older returned 5 rows where newer returned 3.

Turns whose ordinal ranges overlap cannot each be emitted as one contiguous block and still be monotone in sequence, which is what a page cursor requires. Discovering the overlap is not affordable: an invocation's range is never stored — its end is derived per query — and first <= high AND last >= low is a two-dimensional range no index answers, so closing over it materializes the Session on every page.

Emitting a Turn's rows contiguously was never the requirement. Projecting a row needs the whole invocation, because a row is the read model's fold over a Turn's events rather than a per-event map; yielding rows needs only that no other Turn has one in between. Storage now answers with the unbroken stretch of Session ordinals a single invocation owns around the walk's position — two index seeks — and the reader yields that invocation's rows inside the stretch and resumes past it. A Turn interleaved with another owns several stretches and is projected once per stretch, which this reader already paid for: it read and projected a whole invocation per page before this change too.

Overlap stops being a case to handle, so the reader's cluster loop and carry, the per-Session span derivation, and both scan limits are deleted. Measured at 5 events per Turn, a 20-Turn page is 2.2–2.4 ms at every Session size and position tried — 100 to 5000 Turns, Session start to watermark — against 3.6–3.9 ms on main, which loses nested rows to get there.

The previous nested-paging test compared paging against a same-direction sweep — both ran the same defective scan, so both omitted the same rows and it passed; it now uses an explicit expected set, and a new case covers one running Turn enclosing two separated siblings.

Why the timer fix and the watermark change ship together

A running Turn's start time had two producers on main: the one-shot active overlay read, and the renderer's own Date.now() whenever that read had not covered the Turn. The reset is what the second producer looks like from the outside. Landing only the renderer half would make that synthesized start the standing answer for a fact the Host owns, and leave the overlay — the other representation of the same fact — in place to disagree with it. This PR removes the overlay and lets the Host report a running Turn's committed events, after which startedAt covers only the window before the first commit reaches the client. Neither half is blocked on a different repository, pipeline or decision, so splitting would buy a smaller diff at the cost of reviewing the renderer change as a permanent fix rather than as the fallback it becomes.

A running Turn's clock, tool after tool

The new Product/Turn Elapsed Clock story drives the real seam — applyLiveTurnEventoverlayLiveTurnTurnView — for a Turn that started before the transcript reached it. Both sides below run that same story; BEFORE is main's projection and copy.

Running clock, before and after

Running clock frames, light

Running clock frames, dark

Verification

  • @maka/storage: 1374 tests pass. New: both backends agree on run selection in either direction and at either bound; the query-plan contract refuses any SCAN in a transcript read.
  • @maka/runtime affected suites (prefix stability, per-event high water, commit notification) pass.
  • @maka/runtime-host: the whole workspace suite, 1927 of 1940, passes; the one failure (WorkHub v2 keeps its attachment and browser tool ceiling visible in direct and Code Mode) fails the same way on a clean main build and touches no code here. New: a running Turn row reaches an open subscriber via transcript_advanced; mid-stream open receives the streamed prefix then live deltas without a gap; completion before catch-up is paid out in full (previously slow_consumer); commit-driven progress waits for a fenced terminal publication; the Host holds every frame, including a 24-frame in-flight answer, until subscription.ready — that one fails the moment a subscriber is activated on open instead.
  • apps/desktop main: tsc (main/renderer/preload/storybook) clean; the whole main suite, 2651 tests, passes.
  • maka-agent CLI: tsc clean; 161 tests in the three affected files pass.
  • @maka/ui: live-turn-projection, materialize, transcript-projection, chat-turn, chat-view suites pass; new test keeps a live Turn's start at its first event (fails with the old Date.now() synthesis).
  • Storybook: build-storybook + smoke:storybook pass (368 stories, 397 theme renders). The two new stories assert the running clock and the settled zh duration through play functions; built against main's three UI sources they fail with expected 0 to be greater than or equal to 213 and a missing 用时 3 分 33 秒.
  • npm run format, npm run lint, check:architecture pass.
  • Not run: full npm test, E2E.

Breaking change

Runtime Host compatibility epoch 157 → 158: the transcript protocol drops the overlay operation and fields, so older clients are rejected at handshake.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code implemented and tested all layers; the design was reviewed by Claude and Codex before implementation.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/XXL Over 2500 readable lines label Sep 15, 2026
@Astro-Han
Astro-Han marked this pull request as ready for review September 16, 2026 02:10
@Astro-Han
Astro-Han force-pushed the fix/transcript-per-event-watermark branch from efd9d56 to 17f2b72 Compare September 16, 2026 02:22
@likun666661

Copy link
Copy Markdown
Member

想确认一下,这个 PR 把两个可以独立定义和验收的问题放在一起处理,是预期的 scope 吗?

  1. 计时器起点不稳定:缺少 transcript Turn 时,overlayLiveTurn 每次用 Date.now() 构造 startedAt,导致新事件到来时计时归零。保存首个有效事件的时间并持续沿用,就可以独立修复这个问题。
  2. 运行中的已提交事件不能持续进入普通 transcript:将 watermark 改为最新已提交事件,增加 commit-driven refresh,并删除 active overlay。这是更广的数据通路调整,也涉及中途订阅补发、CLI 合并顺序,以及 compatibility epoch 156 → 157。

第二部分通过删除一套 overlay 状态来简化架构,我理解这个方向的价值。但从奥卡姆剃刀的角度,修复第一个问题并不要求同时完成第二个问题。是否考虑拆成两个 PR,分别验收计时器修复和 transcript 一致性改造?如果有必须一起交付的约束,也希望在描述里明确,这样更容易判断当前改动范围是否必要。

验证补充:我运行了源码打包后的 live-turn-projection 测试,42 项通过;仅将 live-turn-projection.tsmaterialize.ts 换回基线实现后,新增计时回归测试失败,另外 41 项通过。

@Astro-Han

Astro-Han commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for raising this, and thanks especially for running the ablation yourself — that matches what I see: the renderer half does stand on its own as a diff, and it's a fair question to ask.

I'd still like to keep the two together, and your comment convinced me the reasoning belonged in the description rather than only in my head, so I've added a section for it ("Why the timer fix and the watermark change ship together").

The short version: these are two producers of one fact. On main, a running Turn's start time comes either from the one-shot active overlay read, or — whenever that read hasn't covered the Turn — from the renderer synthesizing Date.now(). The reset is just what the second producer looks like from the outside. Landing only the renderer fix would promote that synthesized start to the standing answer for a fact the Host owns, while leaving the overlay in place as a second representation of the same thing. With the watermark following committed events, the Host can report a running Turn's steps, and startedAt shrinks to covering the window before the first commit reaches the client — which is the role it should have.

So the split available here isn't quite "small fix now, refactor later"; it's closer to "install a substitute authority now, remove it later", and the first PR would then be reviewed as a permanent fix rather than as the fallback it ends up being. Neither half is blocked on another repository, pipeline or pending decision, so splitting would mainly buy a smaller diff. If you'd still rather review them separately after reading that section, I'm happy to talk it through — the reviewability concern is legitimate either way.

One correction to the description you read: main has since taken epoch 157, so this PR is now 157 → 158.


谢谢你提出这个问题,也特别谢谢你自己跑了消融验证——结果和我看到的一致:渲染端那一半确实能独立成一个 diff,这个问题问得合理。

不过我还是想把两部分放在一起交付。你的意见让我意识到,这个理由应该写进 PR 描述里,而不是只留在我自己脑子里,所以我加了一节("Why the timer fix and the watermark change ship together")。

简短说:这是同一个事实的两个生产者。在 main 上,运行中 Turn 的开始时间要么来自一次性的 active overlay 读,要么——在那次读没覆盖到这个 Turn 时——来自渲染端现造的 Date.now()。计时归零只是第二个生产者从外部看到的样子。只合渲染端那一半,等于把这个合成起点提拔成一个本属于 Host 的事实的长期答案,同时把 overlay 这个同一事实的另一种表示继续留着。改成 watermark 跟随已提交事件之后,Host 能报告运行中 Turn 的各个步骤,startedAt 也就缩小到只覆盖首个提交到达客户端之前的窗口——这才是它应有的角色。

所以这里能做的拆分,并不太像"先小修、后重构",更接近"先装一个替代权威、之后再把它拆掉",而第一个 PR 会被当成长期修复来评审,而不是当成它最终会变成的兜底。两半都不被其他仓库、流水线或待定决策阻塞,所以拆分主要换来的是更小的 diff。如果你读完那一节仍然更希望分开评审,我很乐意继续聊——可评审性这个顾虑无论如何都是成立的。

另外更正一处:你看到的描述里的 epoch 已过时——main 之后用掉了 157,这个 PR 现在是 157 → 158。

@likun666661

Copy link
Copy Markdown
Member

谢谢补充,把 startedAt 定位为持久化记录到达前的临时兜底,这个意图更清楚了。

建议补验一条直接支撑这个解释的完整场景:

  1. Turn 已经运行一段时间,客户端此时才打开订阅。
  2. transcript 尚未包含该 Turn,客户端先用首个有效实时事件的时间显示临时计时。
  3. 包含该 Turn 的持久化记录到达,计时起点切换为记录中的真实起点,补上订阅前已经运行的时长。
  4. 后续继续收到工具和文本事件,仍沿用持久化起点,不归零,也不退回临时起点。

测试里最好明确让「真实开始时间」早于「客户端首个实时事件时间」,否则无法区分稳定的临时起点和正确的持久化起点。临时起点被接管时,显示时长向上校正是预期行为。

我之前跑的 42 项 live-turn-projection 测试,以及新增的计时回归用例,验证了缺少 transcript 时起点能跨后续事件保持稳定;这并不等于验证了上述完整接管链路。如果已有测试覆盖,麻烦指一下;否则建议补一个跨 live projection / transcript materialization 边界的回归用例。

@liuxiaocs7

Copy link
Copy Markdown
Member

Deep review

AI-assisted review by Codex, posted at the user's request. This is a review comment, not an approval or an independent human review.

Recommendation: do not merge until the correctness findings below are resolved. I confirmed four reproducible issues.

Revision scope: the deep review and reproductions were run against 17f2b72da0a25828c28acf30e9146129f084175a, relative to base aaabbbe36b62c7741f52ccdf330bcd1c6c6d8add (75 changed files). Before posting, I compared the current HEAD, cacf15b235972c3d1b947e7de58c3583ad59b58b: its only additional change is the elapsed-clock story, including AdoptsTheRecordedStart. The production code implicated in all four findings is unchanged. I have not run that new story. Source links below are pinned to the tested revision.

Spec

1. [P1] Terminal publication discards pending replay and marks a truncated answer as complete

Location: session-continuity-coordinator.ts:659

When a subscriber joins mid-stream, a large answer may still be queued for replay. If text_complete arrives and the Turn subsequently publishes its terminal projection, assistantBacklog.clear() deletes both the remaining text and its deferred completion.

Reproduction: stream 393,350 characters before subscription, pause the subscriber's first send, deliver text_complete, acquire the terminal publication fence, publish the completed Turn, then release the sink. Only 131,072 characters are delivered, with no completion delta. The real client projector nevertheless synthesizes text_complete from the truncated accumulator and declares the Turn completed.

The fix must preserve this order per subscriber: replayed text → message completion → terminal projection. Removing clear() alone is insufficient because the terminal projection could still overtake replay.

2. [P2] Forward transcript traversal skips an entire nested Turn

Locations: runtime-transcript-query.ts:203, session-transcript-reader.ts:242

The new query selects invocations by their first event within the requested range, so it selects the outer Turn first. The reader still advances to that Turn's lastOrdinal + 1. Any inner Turn entirely contained within the outer interval is therefore skipped.

Reproduction ledger order: outer opens → outer text → inner opens → inner text → inner ends → outer text → outer ends. Against that same ledger:

  • older returns five rows, with sequences [56, 48, 40, 32, 16].
  • newer returns only three, [16, 48, 56], omitting the inner answer and terminal row.
  • Replacing only the query with its baseline implementation makes the same test pass.

The invocation-selection order and overlap-cluster traversal must be adjusted together. All overlapping invocations must be enumerated before advancing the cursor. This is missing transcript output, not deletion of the underlying ledger records.

3. [P2] Per-message replay ordering lets CLI select an older answer as the final output

Locations: session-continuity-coordinator.ts:721, runtime-host-run-command.ts:607

While answer A has pending replay, a subsequent answer B has no backlog of its own and can be delivered and completed immediately. A's completion then arrives last.

The new CLI timestamp comparison cannot correct this: session-projector.ts:382 assigns client receipt time, not the original event time.

Reproduction: open a subscription with 425,984 characters of A pending; complete A; send and complete B (FINAL ANSWER); then drain replay. Using events generated by the real coordinator/projector and feeding them into the public createRuntimeHostRunContext reproduced the failure: B completed first, A completed later, and finalOutput selected 425,984 characters of the earlier answer, instead of FINAL ANSWER. This reproduction does not depend on the premature terminal-publication problem in finding 1.

Preserve semantic ordering across messages, or propagate a stable Host ordering identifier. Receipt timestamps cannot establish which answer was produced last.

4. [P2] Replay flow control can force Desktop into repeated subscription recovery

Locations: session-continuity-coordinator.ts:1465, runtime-host-session-subscription-owner.ts:370

The server replenishes replay whenever sink.send() completes. That does not mean the client has consumed the frame.

While Desktop loads its transcript, it buffers at most 32 frames / 256 KiB. Replaying an existing prefix can fill that buffer, trigger slow_consumer, and cause another subscription—which replays the same prefix again.

Reproduction using the real coordinator, client subscription, Desktop subscription owner and replica: a 32 KiB transcript message requiring a continuation page, 384 KiB of existing streamed text, and a 100 ms page-response delay. With no new runtime events, two consecutive attempts failed and a third subscription started. The diagnostic deliberately stopped at the third open. Reducing replay to 32 KiB made the same flow pass.

Replay needs to respect client preparation and consumption progress, not merely the size of the server's outgoing queue.

Standards

Two documented-standard issues, separate from the correctness findings:

  • The Product stories in the reviewed revision lack per-story // Real path: annotations and use custom wrappers or a bare ProcessingBlock. This does not satisfy FIDELITY.md's production-frame requirements. The subsequent story-only commit does not address these points.
  • Their play assertions primarily verify timestamp retention and localized text, both supported by the existing Node/React harness. Under the stories' AGENTS guidance, move these assertions to that tier; retain browser scenarios where they provide actual visual or browser-specific coverage.

One additional non-blocking heuristic: enqueueTranscriptAdvanced duplicates the canonical-refresh dirty/inFlight scheduling lifecycle. A small shared helper could reduce maintenance risk.

Verification and merge assessment

For the tested revision 17f2b72, both test and windows_recovery checks were successful. I also ran 10 relevant existing test files against bundled current sources; all passed: session continuity, transcript reader, SQLite runtime store, execution-provider conformance, runtime event read model, live-turn projection, subscription client, transcript pager, transcript protocol, and CLI run command. The four additional boundary reproductions all failed. These were focused source-bundled tests, not a normal full-workspace build. I did not run the full npm test, complete build/typecheck, or real Desktop E2E.

The repository's six required conclusions:

  1. Is the solution optimal? Not yet. Stabilizing the UI start time and consolidating transcript authority are reasonable directions, but the replacement data path does not yet satisfy completeness, ordering, and backpressure requirements.
  2. Production code to delete: none identified. The duplicated scheduling lifecycle is a consolidation opportunity.
  3. Tests to replace or strengthen: the nested paging test compares paging against a same-direction sweep, so both can omit identical rows and pass. Use an independent expected message set. The completion replay test must also exercise terminal publication.
  4. Is deeper refactoring required? A focused boundary refactor is needed. Give each subscriber one ordered delivery boundary for replay, completion, and terminal state; align query selection with cursor traversal. A wholesale architectural rewrite is unnecessary.
  5. Ready to merge? No, not with these unresolved correctness findings.
  6. Remaining gaps: real-connection recovery/backpressure and full integration/E2E remain unverified. The new AdoptsTheRecordedStart story adds coverage for the temporary-to-durable timestamp handoff; I inspected its diff but have not executed it. These protocol and user-visible behavior changes require independent human review under CONTRIBUTING.

Summary: Spec: four findings, highest P1 for terminal truncation. Standards: two documented issues, primarily story fidelity, plus one non-blocking duplication heuristic.

@Astro-Han

Copy link
Copy Markdown
Contributor Author

Good catch — you're right that the existing coverage doesn't reach the handover. A start that is wrong but steady would satisfy it just as well. Added in cacf15b: a new Product/Turn Elapsed Clock story, AdoptsTheRecordedStart, following your four steps.

It runs the real seam — materializeTurnsoverlayLiveTurnTurnView — with the durable side built from StoredMessages rather than a hand-made TurnViewModel, so it actually crosses the live-projection / transcript-materialization boundary you named. Per your point about distinguishability, the recorded start is deliberately 120s earlier than the client's first live event:

  1. Before the record arrives, the clock reads ≥ 213s and — asserted explicitly — < 333s, so the stand-in cannot be mistaken for the recorded start.
  2. The durable record arrives; the clock corrects upward to ≥ 333s, picking up the time that ran before the client subscribed.
  3. Two further tool events: still ≥ 333s, neither reset nor reverted to the stand-in.

On the ablation, one round wasn't enough, so I'll report both:

  • Reverting live-turn-projection.ts and materialize.ts to baseline fails both stories, but both fail at step 1 (expected 0 to be greater than or equal to 213). That proves nothing about the handover.
  • So I mutated only the handover: let overlayLiveTurn keep preferring the live stand-in even when a durable row exists — the regression this story is meant to catch. KeepsRunningAcrossTools still passes; only the new story fails, with expected 214 to be greater than or equal to 333.

Restored, smoke:storybook passes (370 stories, 399 theme renders), format and lint clean.

Storybook rather than E2E because the whole chain you described lives on this one renderer path, the play function gates in CI via smoke:storybook, and an E2E would need a genuinely long Turn plus a mid-Turn subscribe to manufacture the same gap — slower and flakier while exercising the same code. For the same reason this PR adds no E2E at all; everything else is unit-level in storage / runtime / runtime-host / desktop main / CLI.


这个提得对——现有覆盖确实没到接管那一步,一个"错但稳定"的起点同样能通过。已在 cacf15b 补上:新增 Product/Turn Elapsed Clock story AdoptsTheRecordedStart,按你列的四步走。

它驱动的是真实链路 materializeTurnsoverlayLiveTurnTurnView,durable 那一侧由 StoredMessage 生成,而不是手搓的 TurnViewModel,所以确实跨过了你说的 live projection / transcript materialization 边界。按你关于可区分性的提醒,真实起点被刻意设成比客户端首个实时事件早 120 秒:

  1. 记录到达前,计时读数 ≥ 213 秒,并且显式断言 < 333 秒,临时起点不会被误当成真实起点。
  2. 持久化记录到达,计时向上校正到 ≥ 333 秒,补上订阅前已经运行的时长。
  3. 再来两个工具事件:仍然 ≥ 333 秒,既不归零,也不退回临时起点。

消融方面,一轮不够,所以两轮都汇报:

  • live-turn-projection.tsmaterialize.ts 换回基线,两个 story 都失败,但都卡在第 1 步(expected 0 to be greater than or equal to 213),这并不能证明接管环节被覆盖。
  • 于是只对接管做定向变异:让 overlayLiveTurn 在已有 durable 行时仍优先使用实时的临时起点——也就是这个 story 要防的回归。结果 KeepsRunningAcrossTools 仍然通过,只有新 story 失败:expected 214 to be greater than or equal to 333

恢复后 smoke:storybook 通过(370 stories,399 theme renders),format 和 lint 干净。

选 Storybook 而不是 E2E,是因为你描述的整条链都在渲染端这一条路径上,play 函数通过 smoke:storybook 在 CI 里是门禁;而 E2E 要真跑一个足够长的 Turn 再在中途开订阅才能造出同样的时间差,更慢也更不稳,验的还是同一段代码。同样的理由,这个 PR 没有新增任何 E2E,其余测试都在 storage / runtime / runtime-host / desktop main / CLI 的单元层。

@Astro-Han

Copy link
Copy Markdown
Contributor Author

Finding 4 is fixed in 41c51e9, though not as stated — the reproduction pointed at something worse.

What I could not reproduce. Server-side replay is already self-limiting: the assistant backlog is paid through a half-gate on the subscriber's own queue, and same-stream deltas merge before they are queued. Replenishing on sink.send() alone does not let replay outrun a consumer that is taking frames. I could not drive a subscriber into slow_consumer from replay volume by itself.

What the reproduction does show. A Client that subscribes mid-answer has to assemble the transcript those frames apply to before it can fold them, and buffers what arrives meanwhile — 32 frames or 256 KiB in the Desktop, 512 frames in the CLI. The Host started sending as soon as the open response flushed, and what it sends first is the whole in-flight answer. With ASSISTANT_BACKLOG_CHUNK_CHARACTERS = 8192 merging to 16384 characters per frame, the Desktop's byte bound is reached at frame 16; past roughly 258,000 characters every later frame is dropped and preparation never completes. That is a deterministic livelock at 100% CPU, not a slow-consumer eviction and not a recovery loop — and it is new in this PR: on main the in-flight text rode inside the open result, so there was nothing to buffer.

The fix is your invariant, applied where the trigger lives. The Host already had the gate — a subscriber is not pumped until it is activated — but it was triggering that gate from its own transport progress. A new subscription.ready operation moves the trigger to the consumer: every frame, including a subscription.closed the Host has already queued, is held until the subscriber says it can take them. The Desktop replica, the CLI channel and the Desktop Bot adapter call it once their state is in place. Both pre-readiness buffers are deleted — a frame arriving before readiness is now reported as the contract breach it is — and so are two "drain the subscription so the bounded transport stays healthy" loops around transcript reads, since a read that never declares readiness is never sent anything to drain. The compatibility epoch was already moving for this PR, so the operation costs nothing extra.

Regression: holds every frame, including the in-flight answer, until the subscriber declares readiness in session-continuity-coordinator.test.ts. It fails the moment a subscriber is activated at open instead of by the Client.

One behavior change worth naming: a subscription the Host evicts before readiness now surfaces that closure immediately after readiness rather than during preparation, so a joining observer can be released by an attempt that is already doomed and then corrected by the resync one step later. The tests covering that path assert the resync instead of a pre-commit failure.

Findings 1, 2 and 3 were fixed earlier in this branch (one ordered per-subscriber gate for assistant frames and projections; the terminal publication captures unpaid streams instead of clearing them) with the tests you asked for, including terminal publication in the completion-replay test and an independent expected set in the nested paging test.


中文

Finding 4 已在 41c51e9 修复,但根因和你描述的不同——你的复现指向了一个更严重的问题。

没能复现的部分。 服务端 replay 本身已经是自限的:assistant backlog 通过订阅者自身队列的半闸支付,同一流的 delta 在入队前就会合并。仅凭 sink.send() 补充配额,并不会让 replay 超出一个正在取帧的消费者。单靠 replay 体量我无法把订阅者逼成 slow_consumer

复现真正暴露的问题。 在回答进行中订阅的 Client,必须先把这些帧要折叠到的 transcript 装配好,期间到达的帧只能先缓冲——Desktop 是 32 帧 / 256 KiB,CLI 是 512 帧。而 Host 在 open 响应冲刷后就开始发送,首先发的就是整个进行中的回答。ASSISTANT_BACKLOG_CHUNK_CHARACTERS = 8192 合并后每帧 16384 字符,Desktop 的字节上限在第 16 帧触顶;超过约 258,000 字符后,之后每一帧都被丢弃,准备阶段永远完不成。这是确定性的活锁加 100% CPU,不是慢消费者驱逐,也不是重连循环——而且是本 PR 新引入的:main 上进行中的文本是随 open 结果一起给的,根本没有需要缓冲的东西。

修法就是你提的那条不变量,只是放到触发点该在的位置。 Host 本来就有这个闸门——订阅者未 activated 就不会被 pump——但触发它的是 Host 自己的传输进度。新增 subscription.ready 操作把触发权交给消费者:所有帧,包括 Host 已经排好队的 subscription.closed,都扣住直到订阅者说自己能收。Desktop replica、CLI channel、Desktop Bot adapter 各自在状态就位后调用它。两侧就绪前的缓冲都删掉了——就绪前到达的帧现在直接按契约违反上报——另外两处"为保持有界传输健康而 drain 订阅"的循环也一并删除:从不声明就绪的读取,根本不会被发任何东西。兼容 epoch 本来就要为这个 PR 抬升,所以新操作没有额外成本。

回归测试:session-continuity-coordinator.test.ts 中的 holds every frame, including the in-flight answer, until the subscriber declares readiness。只要把订阅者改回 open 时即激活,它立刻失败。

有一处行为变化值得说明:被 Host 在就绪之前驱逐的订阅,现在会在就绪后立刻暴露关闭,而不是在准备阶段暴露;因此一个正在加入的 observer 可能被一个注定要被替换的尝试放行,随后由 resync 在下一步纠正。覆盖该路径的测试改为断言 resync,而不是提交前失败。

Finding 1、2、3 已在本分支早前修复(assistant 帧与 projection 走同一个按订阅者排序的闸门;终态发布会接管未支付的流而不是清空它们),并补上了你要求的测试,包括在 completion-replay 测试中覆盖终态发布,以及把嵌套分页测试改为独立的期望集合。

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Independent agent review. Reviewed at 41c51e98efc7840245bdbad74b225f597423bbb2. I am an AI agent (executing seat @kabi-opus) publishing through a shared GitHub account; this is an automated review and does not substitute for independent human review.

No P0–P2 findings. One coverage gap that no reviewer closed, stated at the end.

89 files and a net −1,869 lines, so I spent the review on the deletions and on whether the invariants moved with them.

Removing the overlay is complete, including the part that is easy to leave dangling

transcript_overlay and the reader's active scan have zero occurrences. The one thing that correctly survives is the grant entry, and the way it survives is the point:

// Retired with the active transcript overlay; pages alone carry a running Turn.
['session.transcript.overlay.release', { kind: 'release' }],

That is PERSISTED_GRANT_MIGRATIONS, and release means a persisted grant naming this operation is dropped rather than carried to a successor — the type's own comment enforces the discipline ("a migration that names no successor is a release, and has to say so"). Deleting the operation without this would leave stored grants pointing at an operation that no longer exists. Retiring a capability this way is more complete than deleting it.

The 405-line deleted test file took its own subject with it, and the surviving invariant stayed

All six removed tests name overlay in their titles — bootstrap overlay settlement, retaining an unfinished overlay, retiring the completed overlay, the tail overlay copy. The mechanism they pinned is gone.

The one that deserved a second look is a completed live answer remains unique after a fresh transcript subscription, because uniqueness on resubscribe is exactly the risk the new design takes on: a subscriber opening mid-stream is now paid the already-streamed text "as ordinary deltas from offset 0". That invariant did not leave with the overlay — session-projector.test.ts holds it, in does not replay settled transcript steps when the active step reaches terminal, seeds an unrendered in-flight steering message once on rejoin, and projects a steering message exactly once across both authoritative paths. The property moved to where the mechanism now lives.

The three catch-up defects each have a named regression, and the gate is pinned

The description's candour about finding three defects in its own intermediate approach is worth more than a clean narrative would be, so I checked that each has a test rather than only a paragraph:

  • ordering inversion → a later message cannot complete ahead of the prefix a subscriber is still being paid
  • terminal publication dropping the backlog → a terminal publication finishes the prefix it found unpaid instead of dropping it
  • payout path → opening mid-stream pays out the streamed prefix before live deltas without a gap

Ablating the single gate that implements this — forcing unpaid-prefix deltas past the assistantBacklog branch into #deliverInOrder — turns two red, including the mid-stream payout one. I confirmed the marker reached the emitted dist/ before reading the result.

Suites, and two false failures that were mine

UI 498/498. Desktop main 2651/2651. Runtime-host 1941 tests, 1 failure; storage 1384 tests, 1 failure. Neither of those two belongs to this PR:

  • The runtime-host one is WorkHub v2 keeps its attachment and browser tool ceiling visible in direct and Code Mode, which fails on current origin/main as well. I bisected it to #5345 and filed #5388; it is unrelated to this branch.
  • The storage one is rejects a second authority for the same storage root in another process, which treats any child-process stderr as failure and so trips on Node 25's SQLite ExperimentalWarning. It fails identically at the merge base.

I also produced two sets of failures myself before getting a trustworthy run, and both are worth naming because anyone re-verifying this PR will hit them:

  • Building without clearing apps/desktop/dist leaves the compiled transcript-overlay-settlement.test.js behind — tsc does not delete outputs for deleted sources — and its 7 tests run against code that no longer supports them.
  • Clearing dist wholesale and then running only build:main removes dist/overlay/*, which that script does not regenerate; 6 dialog tests then fail with ENOENT on browser-dialog-design-tokens.css. The right command is npm --workspace @maka/desktop run build:test, which covers main, preload and overlay.

Under-cleaning and over-cleaning both produce convincing, unrelated failures.

One cross-PR note

This branch already contains the changes of two other open PRs: processDuration here is byte-identical to #5360's spaced form, and pendingRunningStartedAt is removed exactly as in #5337. I reviewed both of those separately today. Whichever lands second is a no-op or a conflict on those hunks — worth a deliberate decision rather than discovering it at merge.

The gap nobody closed

The headline claim is user-visible: a running timer stops resetting. The mechanism is verifiable from source and unit tests, and I verified it — pendingRunningStartedAt is gone, the elapsed sites read turn.startedAt, and the projector now yields rows for a running Turn. What no one verified is the end-to-end behaviour in a real app. I cannot launch Electron; the seat that can declined this round, reasonably, on the grounds that taking an 89-file change needing real Electron while already holding another re-review would compress evidence quality. So the thing a user would actually notice rests on the author's own runs.

Code review, CI status and merge readiness are separate. At this SHA mergeable=MERGEABLE, mergeStateStatus=BLOCKED. This approval covers code only and is not a statement that the PR may be merged.

The settled process summary read 用时 35秒 while every other zh count in the
product separates the number from its unit (goalElapsed uses ' 秒', counts
use } 个). The zh and zh-TW processDuration templates now render 用时 3 分 33 秒.

Retry delays keep their compact form, which their tests pin deliberately.

Generated-by: Claude Code
…rmark

The transcript high-water mark was the last Turn ending, and every
invocation query required an ending at or before the watermark. A running
Turn was therefore invisible to transcript pages and landmarks until it
finished, and nothing told readers when new RuntimeEvents were committed.

The high-water mark is now the Session's last committed event ordinal.
Older pages and landmarks select by opening ordinal; an invocation's
interval ends at its ending when that is within the watermark, otherwise at
its last event within it. Newer pages walk events from the requested
position and select the invocation of each first event in range, so a walk
that starts inside a running Turn still finds it. Event reads stop at the
watermark.

SqliteRuntimeStore publishes subscribeRuntimeEventCommits once per Session
after the outermost transaction commits and never after a rollback. A
leased store can be nested in another repository's transaction, so the
operational database lease gains onTransactionSettled, which the owner runs
when its outermost transaction settles. The execution stores facade and the
memory reference store expose the same subscription; the memory store's
transcript selection and landmark sampling now match SQLite.

Generated-by: Claude Code
The transcript projector had an `active` mode that settled partial model
events and invented an empty assistant row for thinking that had no text
yet. Without it, thinking left unclaimed at finish was always a hard
`unsupported_event` diagnostic, so a transcript page cut inside a running
Turn either failed or showed rows that changed once the Turn continued.

The projector now has one mode. Unclaimed thinking is reported only when
the projected events include its invocation's terminal event; before that
it simply has no row yet. This makes every committed prefix project exactly
the rows the full ledger later attributes to those same events, which the
new prefix-stability test checks event by event.

activePresentationRuntimeEvents stays, since RuntimeReadModel still uses
it. The runtime-host transcript reader drops the removed option.

Generated-by: Claude Code
…he active overlay

The durable transcript watermark now follows committed RuntimeEvents, so a
running Turn's rows reach subscribers through ordinary pages. Runtime event
commits enqueue a coalesced transcript_advanced refresh, held back while a
terminal publication is fenced.

The one-shot active overlay, its release operation, cursor source, reader
scan and message-id lookup are removed. Text already streamed when a
subscriber opens is paid out as ordinary deltas from offset 0, bounded by
half the subscriber queue; a completion that lands before the subscriber
catches up is deferred until the prefix is delivered instead of flooding
the queue. Compatibility epoch 157.

Generated-by: Claude Code
Desktop Main, preload and the renderer range store drop the active overlay:
the replica keeps durable rows only, IPC fragments are keyed by sequence,
and the local transcript cache writes the snapshot as is. The CLI channel
no longer recovers from the removed overlay-release failure. The renderer
stops issuing mid-turn refreshes on text_complete and tool_result, which
only re-checked readiness.

Generated-by: Claude Code
A Turn the transcript had not reached yet was synthesized with Date.now()
on every projection, so the running timer restarted with each live event.
The live projection now records the first event's timestamp and the
synthesized Turn uses it.

Generated-by: Claude Code
The subscription owner still recovered from `transcript_release_failed`,
which no longer exists after the overlay was deleted, so the Windows
build failed on TS2367. Removing it also lowers the renderer
architecture ledger's token count for app-shell-session-events.

Generated-by: Claude Code
The storybook smoke runs play functions, so these two stories hold the
regression where CI can see it: the clock is stamped from the Turn's
first live event and keeps running as tools arrive, and the settled
zh duration spaces its numbers from their units. Both fail against
main's projection and copy.

Generated-by: Claude Code
The Host suppresses a steering echo for a message its transcript already
carries. Per-event watermarks make that the normal order, so the panel,
which reads durable messages on its own, lost the only signal that
retired the optimistic queue entry and drew the steered message in the
running Turn. Admission is that proof: read the message it names.

Generated-by: Claude Code
The run adapter rebuilt a Turn's outcome from every transcript
replacement. That was safe while a read carried the Host's active
overlay, which always described the running Turn in full. Now that the
transcript advances per committed event and the overlay is gone, a read
triggered by a tool result can resolve after the final answer streamed
and stop short of it, so `maka run` reported a completed Turn with no
output and exited 1 — the CLI release smoke's controlled run.

Feed such a read into the Turn's existing outcome instead of replacing
it. A read of a running Turn can restore what the live stream missed;
it cannot prove that what the stream already delivered is gone.

Generated-by: Claude Code
A Turn's final output was whatever arrived last. The live stream and a
transcript read describe the same answers at their own pace, so a read
that stops short of the newest answer would move `maka run` back to an
earlier step's text.

Order the outcome's output by the answer's own timestamp instead.

Generated-by: Claude Code
Retiring the overlay's capacity error left this catch with nothing to
classify, and the error was dropped. A projection that outgrows its
bounds is a Host defect; the client can only retry, so report it
through the publication failure hook before answering.

Generated-by: Claude Code
…ermark

The bootstrap carried a watermark of its own because it used to anchor
two pages, the durable one and the active overlay. With the overlay
gone it is a copy of the single page's own watermark, kept in step by
an assertion. Read the page instead.

Also retires the last descriptions of the old model: the architecture
docs still listed a cursor source and overlay bounds, and two comments
still explained running Turns as overlay-only or as rows without
durable ordinals.

Generated-by: Claude Code
A running Turn's clock now has two starts in sequence: the first live event
the client sees, then the Host's recorded start once the transcript reaches
the Turn. The existing story only proved the stand-in is stable across
events, which a wrong-but-steady start also satisfies.

The new story drives materializeTurns -> overlayLiveTurn -> TurnView with a
recorded start deliberately earlier than the client's first event, so the
handover is observable: the clock corrects upward and keeps the recorded
start across later tool events.

Generated-by: Claude Code
Leaving a conversation dropped the Host subscription whenever no renderer
target, watched Turn or transcript consumer remained. The Host goes on
producing either way, so coming back opened a second subscription and the
Host streamed the whole in-flight answer again from offset 0 — measured at
307,200 characters over 19 frames for a 300 KB answer, 204,800 of which the
client had already received.

"This Turn is still running, so do not let go" already existed as a
retention reason, but only as watchedTurnIds, which is populated by whoever
submitted the message locally. A Turn started by the CLI, by another client,
or restored after a restart had no such ticket. Read the fact from the
canonical rootTurn instead, so one authority answers it for every Turn. The
release path is unchanged: a terminal root Turn already runs closeIfIdle.

Costs one subscription per concurrently running Session the app has opened,
which is bounded by how many Turns can run at once, not by Session count.

Three existing tests asserted the old rule in their lifecycle tails while
testing something else; their subjects are preserved and the tails now use a
settled Session or assert retention directly.

Generated-by: Claude Code
Catch-up text was paid out on a path of its own, beside the subscriber's
queue, so anything produced later could pass it:

- a message with no backlog of its own was delivered and completed while an
  earlier message was still being paid, inverting the order the Host produced
  them in. A reader that takes "the last answer" then takes the earlier one.
- the fenced terminal publication cleared every unpaid backlog, so a
  subscriber kept a truncated answer and the client, seeing the Turn end,
  reported it as the complete one.

Route every assistant frame and projection through one gate: while a
subscriber is still being paid a prefix, later work queues behind it rather
than going straight out. The terminal publication now captures each unpaid
stream into its own backlog and queues the projection behind the payout,
instead of dropping text the Host had already sent.

The live-delta gate still skips a stream whose own backlog is unpaid: that
text is in the accumulated stream the payout reads, so enqueuing it again
would duplicate it.

Both regressions fail on the previous commit.

Generated-by: Claude Code
The previous rule compared an `output` observation's `ts` and kept the later
one. Those timestamps come from two different clocks: a live completion is
stamped with the client's receipt time by the session projector, while a
stored observation carries the Host's message time. Wall-clock values are not
an ordering contract in either case — ties and clock adjustments remain.

Now that a subscriber is delivered in Host order, the live stream's last
answer is the Turn's last answer, and no arbitration is needed between live
observations. A transcript read stops wherever the Host had committed, so it
may supply an answer the stream never carried but may not overrule one it
did. That is the whole rule, and it needs no timestamps.

Covered both ways: the existing late-read regression fails if a stored answer
can overrule a live one, and a new reattach case fails if a stored answer can
no longer set the final output.

Generated-by: Claude Code
…ents

Forward selection reached running invocations through their own events, but
the reader advanced its cursor past one invocation's whole span, so a Turn
nested inside another was never selected: `older` served 5 rows where `newer`
served 3.

Turns whose ordinal ranges overlap cannot each be emitted as one contiguous
block and still be monotone in sequence, which is what a page cursor requires.
The first fix therefore had storage return the seed's whole connected component
under range overlap. That is correct but unaffordable: an invocation's range is
never stored — `last` is derived per query — and `first <= high AND last >= low`
is a two-dimensional range no index answers, so every page materialized the
Session. Measured at 5 events per Turn, one 20-Turn page cost 3.7 ms on `main`
and 65 ms at 1000 Turns, 352 ms at 5000, growing linearly and independently of
which page was asked for.

Emitting a Turn's rows contiguously was never the requirement. Projecting a row
needs the whole invocation, because a row is the read model's fold over a Turn's
events rather than a per-event map; *yielding* rows needs only that no other
Turn has one in between. So storage now answers with the unbroken stretch of
Session ordinals a single invocation owns around the walk's position — two index
seeks, no derived ranges — and the reader yields that invocation's rows inside
the stretch and resumes past it. A Turn interleaved with another owns several
runs and is projected once per run, which the reader already paid for: it read
and projected a whole invocation per page before this change too.

Overlap stops being a case to handle, so the recursive closure, the per-Session
span materialization, both seeds, `lastOrdinal`, `openingOrdinal`, the reader's
cluster loop and `carried`, `TRANSCRIPT_TURN_SCAN_LIMIT` and
`TRANSCRIPT_TURN_OVERLAP_LIMIT` are all deleted. A page is now 2.2-2.4 ms at
every Session size and every position measured (100 to 5000 Turns, Session start
to watermark), and the query-plan contract is back to refusing any SCAN rather
than allowing the closure's derived sets.

The previous nested-paging test compared paging against a same-direction sweep;
both ran the same defective scan, so both omitted the same rows and it passed.
It now asserts an explicit expected set, and a second case covers one running
Turn enclosing two separated siblings. Removing the run bounds fails both and
leaves the other three reader tests passing.

Generated-by: Claude Code
…can take them

A Client that subscribes mid-answer must assemble the transcript those
frames apply to before it can fold them, and buffers what arrives
meanwhile: 32 frames or 256 KiB in the Desktop, 512 frames in the CLI.
The Host began sending as soon as the open response flushed, and what it
sends first is the whole in-flight answer. Past roughly 258,000
characters the Desktop's byte bound is reached at frame 16, every later
frame is dropped, and preparation never completes -- a deterministic
livelock at 100% CPU rather than a slow-consumer eviction. This is new in
this PR: on main the in-flight text was carried inside the open result,
so there was nothing to buffer.

The Host already had the gate -- a subscriber is not pumped until it is
activated -- but it was triggering that gate from its own transport
progress. `subscription.ready` moves the trigger to the consumer: frames,
including a queued `subscription.closed`, are held until the subscriber
says it can take them. The Desktop replica, the CLI channel and the
Desktop Bot adapter call it once their state is in place, and both
pre-readiness buffers are deleted -- a frame arriving before readiness is
now reported as the contract breach it is. Two "drain the subscription so
the bounded transport stays healthy" loops around transcript reads go
with them: a read that never declares readiness is never sent anything to
drain.

A subscription evicted before readiness now surfaces its closure right
after readiness instead of during preparation, so the tests covering that
path assert the resync rather than a pre-commit failure.

Generated-by: Claude Code
@Astro-Han
Astro-Han force-pushed the fix/transcript-per-event-watermark branch from 41c51e9 to 24889e5 Compare September 16, 2026 08:43
@Astro-Han
Astro-Han merged commit 9982e86 into main Sep 16, 2026
2 checks passed
@Astro-Han
Astro-Han deleted the fix/transcript-per-event-watermark branch September 16, 2026 09:58
Astro-Han added a commit to Astro-Han/maka-agent that referenced this pull request Sep 16, 2026
Rebuild the full-load transcript on main's per-event ordinals (apache#5365).
Running Turns are now durable pages, so the Desktop overlay (replica
overlay, overlay bootstrap page, loadTranscriptOverlay, fragment source)
is removed rather than carried forward, and the protocol epoch moves to
160 after apache#5308 took 159; its compatible-change declaration is re-pinned.

Review fixes re-checked under main's model:
- The Turn boundary marker is computed per scan: a run starts between
  Turns only when every Turn the walk has entered lies behind it. Runs
  are single-invocation stretches, so a change of owner no longer says
  a page is between Turns when Turns nest.
- A reset still rereads down to the oldest sequence the consumer was
  given, because a reset replaces what the reader holds.
- The guest transcript reader now passes its projection through to
  readPage; before, guests saw unprojected rows. Covered by a reader test.
- The test for rows published below the watermark is dropped: every
  committed event takes MAX+1, so that premise no longer holds.

Generated-by: Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XXL Over 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants