fix(summary): integrate sidebar into heartbeat protocol to eliminate duplicate polling (#334)

[lml2468]

Summary

Fix #334 — ChatSummaryHistory (sidebar) ran its own 5s polling → batchStatus() completely outside the summary-batch-heartbeat coordination protocol. When sidebar and main summary tab were both visible, they independently polled overlapping task batches, causing redundant API calls.

Root cause

• SummaryListPage.tsx:152 dispatches summary-batch-heartbeat every poll ✅
• SummaryDetailPage.tsx:137 listens to summary-batch-heartbeat ✅
• ChatSummaryHistory.tsx — neither sends nor receives ❌ (completely outside coordination)

Fix

Integrate ChatSummaryHistory into the existing heartbeat protocol:

1. Listen to summary-batch-heartbeat from SummaryListPage — when task IDs overlap, set listPageActive_ = true and stop own polling
2. Listen to summary-status-change from SummaryListPage — when changed task IDs overlap, reload history via listSummaries() (no duplicate batch-status call)
3. Listen to summary-list-unmount — SummaryListPage already dispatches this on componentWillUnmount; use it to reset listPageActive_ and resume polling

Design decision: event-driven coordination (no Date.now() timeout). The sidebar's poll state is a pure function of list page lifecycle events — simpler, deterministic, fully testable with fake timers.

Changes

File	Change
ChatSummaryHistory.tsx	Add 3 event listeners, listPageActive_ flag, suppress/resume logic
ChatSummaryHistory.test.tsx	Add 3 tests: heartbeat suppression, unmount resume, status-change reload

Verification

• All 14 tests in ChatSummaryHistory.test.tsx pass (11 existing + 3 new)
• SummaryListPage and SummaryDetailPage heartbeat logic unchanged
• Sidebar falls back to independent polling when list page is not mounted (no regression)

Fixes #334

[OctoBoooot]

Review: fix(summary): integrate sidebar into heartbeat protocol to eliminate duplicate polling (#334) (#472)

Verdict: Request changes

This supersedes #471 and fixes its self-suppression blocker correctly — the redesign makes the sidebar a pure consumer of summary-batch-heartbeat (listens, never dispatches), so the synchronous self-echo loop that degraded #471's cadence is gone. Good pivot. But the binary listPageActive_ flag replaces #471's 15s-expiry safety net with reset-only-on-unmount, which re-introduces an over-suppression path.

#471 blockers — status:

• ✅ Self-suppression: resolved. Sidebar no longer dispatches the heartbeat it listens for; it only sets listPageActive_ from heartbeats originating in SummaryListPage.
• ✅ Partial coverage (Jerry-Xin's 2nd 🔴): improved — the suppression now means "list page is polling, defer to it + consume its summary-status-change," rather than a per-tick taskId-overlap skip, so the sidebar's non-overlapping tasks are covered by the status-change → loadHistory path.

Major

• ChatSummaryHistory.tsx:listPageActive_ over-suppression with no expiry. The flag is set true on any overlapping heartbeat and reset false only on summary-list-unmount. But SummaryListPage.doBatchPoll/maybeStartBatchPoll (SummaryListPage.tsx:139-141) calls stopBatchPoll() when its own active tasks reach zero while staying mounted — and that path dispatches no summary-list-unmount (that event fires only in componentWillUnmount, :84). Failure sequence: list page + sidebar both showing, overlapping active tasks → sidebar suppresses (listPageActive_=true). The list page's tasks all reach terminal state → it stopBatchPoll()s and stops emitting heartbeats, but stays mounted → no unmount event. The sidebar's listPageActive_ stays true indefinitely. If the sidebar then gets a new active task (new summary created in the channel) that the list page isn't tracking, the sidebar never resumes polling → that task's status never live-updates in the sidebar until a full remount or a summary-status-change that happens to overlap. #471's 15s-silence expiry was exactly the safety net for "coordinator went quiet without unmounting"; the binary flag drops it. Fix: also reset listPageActive_ on heartbeat silence (e.g., a timestamp + staleness check, like #471 had), or have SummaryListPage.stopBatchPoll emit an "idle" signal the sidebar treats like unmount.

Minor

• Tests cover heartbeat-suppress, unmount-resume, and status-change-reload, but not the above "list page stops polling without unmounting → sidebar stuck suppressed." That's the exact gap the major describes; a regression test (heartbeat → list goes idle, no unmount → sidebar should resume after staleness window) would pin it.

Praise

• Making the sidebar a pure consumer of the heartbeat (rather than #471's dispatch-and-listen) is the right redesign — it structurally eliminates the self-suppression class instead of patching around it, and consuming summary-status-change to loadHistory means a suppressed sidebar still reflects status changes without its own batchStatus call. The direction is correct; it just needs the coordinator-went-quiet safety net restored.
• handleStatusChange_ reloading via the existing loadHistory (a listSummaries call) rather than a second batchStatus poll keeps the dedup honest — the suppressed sidebar gets fresh data through a different endpoint, so suppression doesn't mean staleness in the common case.

Out of scope (informational)

• Draft PR. The <!-- multica:verify v1 --> marker in the body is presumably tooling; harmless.

[mochashanyao]

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).

---

octo-web PR#472 Review Report

Reviewer: Octo-Q (automated review)
Head SHA: 1f070040668f6cc4301db0ad67cbd719f1a36d19
Files changed: 2 (ChatSummaryHistory.tsx +59, ChatSummaryHistory.test.tsx +100)

1. 验证结论

• ✅ 事件注册/注销对称性：3 个 addEventListener 在 componentDidMount (L51-53) 与 componentWillUnmount (L61-63) 一一对应，无泄漏。
• ✅ 心跳抑制轮询：handleBatchHeartbeat_ (L185-195) 正确检查 overlap → 设 listPageActive_=true → stopPoll()。maybeStartPoll() (L89-95) 在启动前和 interval 回调内 (L100) 双重检查 listPageActive_。
• ✅ 卸载恢复：handleListPageUnmount_ (L201-204) 重置 flag → maybeStartPoll()，与 SummaryDetailPage 同模式。
• ✅ 状态变更重载：handleStatusChange_ (L210-219) 检查 overlap（取全部 items 的 task_id，不仅 active）→ loadHistory()，abort-controller 防竞态。
• ✅ 测试覆盖：3 个新测试分别覆盖心跳抑制、卸载恢复、状态变更重载，断言正确。

2. 发现的问题（P0/P1/P2）

P2-1: handleStatusChange_ 触发完整 listSummaries API 重载

• 严重度：P2（性能 / 可优化，非阻塞）
• diff-scope 三问：new — 本 PR 新增的 handleStatusChange_ (L210-219) 在每次 summary-status-change 事件命中时调用 loadHistory()，发起 listSummaries API（page=1, page_size=50）。
• 分析：SummaryListPage.doBatchPoll (SummaryListPage.tsx:152-170) 每 2s 轮询一次。当任意 task 状态变更时，同时 dispatch summary-batch-heartbeat + summary-status-change。每次状态变更都触发 sidebar 的 loadHistory()（abort + 新请求）。在多 task 快速连续完成时（如批量任务几乎同时结束），sidebar 可能在短时间内发出多次 listSummaries 请求。
• 缓解：abort controller 保证不会堆积响应；listSummaries 是只读 GET，幂等安全。但相比原来 sidebar 自己的 5s batchStatus（轻量），现在是 2s 频率的 listSummaries（更重）。
• 建议：考虑在 handleStatusChange_ 中做本地 patch（仅更新变更的 task status）而非全量 reload，或在 loadHistory() 前加 debounce（如 500ms 合并窗口）。非阻塞。

P2-2: 缺少 Date.now() 防御性超时（设计选择差异）

• 严重度：P2（防御性缺口，理论场景）
• diff-scope 三问：new — 本 PR 采用纯事件驱动设计，故意不含 timeout。
• 分析：SummaryDetailPage 有 Date.now() - lastEventTime > 15000 超时回退 (SummaryDetailPage.tsx:468-469)，在 15s 无事件后自动重置 listPageActive。ChatSummaryHistory 无此机制。若 SummaryListPage 在极罕见场景下冻结（JS 引擎 stall / tab backgrounded 后 setInterval 暂停但未 unmount），sidebar 轮询会被永久抑制直到 unmount 事件到达。
• 实际风险：极低。浏览器对 background tab 的 setInterval 节流是减速而非停止，heartbeat 仍会到达。且 SummaryListPage 有 componentWillUnmount → summary-list-unmount 的硬保证。
• 建议：可接受当前设计。若后续发现边界 case，加一行 lastEventTime 检查即可。

3. 建议

1. P2-1：若后续观察到 sidebar 频繁 listSummaries 调用，考虑改为本地 patch 或加 debounce。
2. P2-2：当前纯事件驱动设计 OK。可作为后续 hardening 项。

4. 额外发现

• SummaryListPage 轮询频率 2s vs sidebar 5s：PR 消除了 sidebar 的独立 5s batchStatus 轮询，改由 SummaryListPage 的 2s 轮询驱动。当 SummaryListPage 活跃时，sidebar 通过事件获得更新（零额外 API 调用）；当 SummaryListPage 不活跃时，sidebar 退回 5s 自轮询。净效果：消除重复 API 调用，符合 PR 目标。
• handleStatusChange_ 使用全量 items.task_id 做 overlap 检查（L213: this.state.items.map(item => item.task_id)），而 handleBatchHeartbeat_ 使用 getActiveTaskIds() 过滤活跃态。两者差异是 intentional：状态变更可涉及任意 task（含已完成的），心跳只关注活跃 task。正确。

5. 数据流回溯

消费点	数据来源	上游追踪	是否流通
handleBatchHeartbeat_.detail.taskIds	summary-batch-heartbeat event	SummaryListPage.doBatchPoll L152: dispatch(new CustomEvent("summary-batch-heartbeat", { detail: { taskIds } })) — taskIds 来自 currentActiveIds（active items 的 task_id）	✅ 事件 detail 非空，handler 正确解构
handleStatusChange_.detail.taskIds	summary-status-change event	SummaryListPage.doBatchPoll L170: dispatch(new CustomEvent("summary-status-change", { detail: { taskIds: changedIds } })) — changedIds 来自状态比对后的变更 task_id 列表	✅ changedIds 非空（由 if (changed) 门控），handler 正确解构
summary-list-unmount	SummaryListPage.componentWillUnmount L84	无 detail payload	✅ handler 无参数，正确
listPageActive_ flag	handleBatchHeartbeat_ 设 true / handleListPageUnmount_ 设 false	事件链：ListPage mount → heartbeat 2s → sidebar sets true; ListPage unmount → unmount event → sidebar sets false	✅ 生命周期正确闭环
getActiveTaskIds() 消费于 overlap 检查	this.state.items 过滤 PENDING/WAITING_CONFIRM/PROCESSING	items 来自 loadHistory() → listSummaries() API 响应 → setState({ items })	✅ 数据从 API 正确流入

6. 盲点 checklist（R5）

• C1 双路径 parity：✅ Clear。本 PR 只修改 ChatSummaryHistory（纯消费者），不改 SummaryListPage（生产者）。事件注册/注销对称性已验证（mount/unmount 各 3 对）。
• C2 control-flow ordering / 嵌套复用：✅ Clear。3 个新 handler 是独立的叶子回调，无嵌套调用链。loadHistory() 的 abort controller 防竞态正确。maybeStartPoll() 内部先 stopPoll() 再 start，无重复 interval 风险。
• C3 授权边界 ≠ 能力边界：N/A。本 PR 不涉及权限/jail/tool 暴露。
• C4 授权生命周期 / 容器-成员状态级联：N/A。不涉及鉴权改动。
• C5 build/note 通过 ≠ 运行期路径正确：N/A。不涉及构建产物/扩展/打包。
• C6 治理/策略/安全文档自洽性：N/A。不涉及治理文档。

7. 跨轮 blocker 复检（R6）

N/A — 首次 review。

---

[Octo-Q] verdict: APPROVE — 无 P0/P1 finding。PR 正确将 sidebar 集成进已有心跳协议，事件注册/注销对称，数据流回溯通畅，测试覆盖 3 个核心场景。两个 P2 建议为后续优化方向（listSummaries debounce / 防御性超时），不阻塞合入。

[Jerry-Xin]

The PR is in-scope for octo-web, but the new sidebar coordination can stop polling tasks that the summary list is not actually covering.

🔴 Blocking

🔴 Critical — partial-coverage stranding: sidebar disables ALL polling on any single overlapping heartbeat. handleBatchHeartbeat_ sets listPageActive_ = true and calls stopPoll() whenever taskIds.some(id => myIds.includes(id)) finds even one overlapping id (packages/dmworksummary/src/components/ChatSummaryHistory.tsx:190), and maybeStartPoll() then suppresses all sidebar polling whenever that single boolean flag is true (packages/dmworksummary/src/components/ChatSummaryHistory.tsx:88, and the per-tick guard at :103).

This is unsafe because the two pollers track different active sets:

• SummaryListPage polls only the active ids in its current paginated + status/keyword-filtered state.items (packages/dmworksummary/src/pages/SummaryListPage.tsx:117, dispatch at :152).
• The sidebar (ChatSummaryHistory) is channel-scoped, loads its own page_size: 50 history for one channel with no status filter (getActiveTaskIds() over state.items).

So if the sidebar has active tasks [1, 2] and the list-page heartbeat only covers [1] (task 2 is off the current page / filtered out), the .some() overlap fires → the sidebar stops polling entirely → task 2 is no longer polled by anyone. summary-status-change only carries the list page's own changedIds, which never include 2 (the list page never polls it), so 2's status goes stale until summary-list-unmount. This is the same coverage-loss class as the prior round, just reshaped from heartbeatTaskIds.some(...) into a binary listPageActive_ flag — overlap detection is still .some(...), so it suppresses on any overlap rather than full coverage.

Stale-suppression variant: once listPageActive_ is latched true, a later non-overlapping heartbeat is return-ed early and never clears the flag (:193). If the list page changes page/filter so it no longer includes the previously-overlapped task, the sidebar stays suppressed (and the now-uncovered task stays unpolled) until the list page actually unmounts.

Fix: suppress only when the heartbeat covers ALL sidebar active ids (getActiveTaskIds().every(id => heartbeatIds.includes(id))), or keep polling just the uncovered subset (ids.filter(id => !covered)). Add tests for (a) partial overlap — sidebar [1,2], heartbeat [1], assert task 2 is still polled — and (b) list-page coverage shrinking after the flag is set.

💬 Non-blocking

🟡 The self-emission re-entry from the prior round is resolved structurally: ChatSummaryHistory no longer dispatches summary-batch-heartbeat itself (it only listens), so there's no synchronous self-suppression and a source/owner id is not required here. Good. The remaining risk is purely the cross-component coverage divergence above.

🟡 The 3 new tests cover full-overlap suppression, unmount-resume, and status-change reload — but none exercise partial overlap, which is exactly where the blocking bug lives. Please pin the uncovered-subset behavior.

🟡 Could not run the targeted test file in this checkout (no node_modules / vitest available); review is static-analysis based.

✅ Highlights

• Reuses the existing summary-batch-heartbeat / summary-status-change / summary-list-unmount protocol shared with SummaryListPage and SummaryDetailPage rather than inventing a parallel mechanism — directionally consistent.
• Dropping the self-dispatched heartbeat removes the prior self-re-entry/cadence-degradation defect cleanly.

[yujiawei]

Code Review — PR #472 (octo-web)

Reviewed at head 1f070040668f6cc4301db0ad67cbd719f1a36d19 against merge-base a3228c58. Scope: 2 files, +159/-0. Package tests pass locally (14/14 in ChatSummaryHistory.test.tsx). I traced the full event contract across SummaryListPage.tsx, SummaryDetailPage.tsx, and ChatSummaryHistory.tsx.

1. Spec compliance

Spec: ✅

Issue #334 asks to bring the sidebar's polling under the existing summary-batch-heartbeat coordination so it stops issuing redundant batchStatus() calls. The diff does exactly that and nothing extra:

• Listens to summary-batch-heartbeat / summary-status-change / summary-list-unmount, with matching removeEventListener in componentWillUnmount (ChatSummaryHistory.tsx:51-53,61-63). The event names and payload shapes (detail.taskIds) match the dispatchers in SummaryListPage.tsx:84,152,170. ✓

• Suppresses its own poll on overlap and reloads via listSummaries() on status-change instead of calling batchStatus(). ✓

• No new flags, fields, or out-of-scope surface added.

• Under-build: none for the requested feature

• Over-build: none

• Deviation: none at the spec level (the protocol-integration was requested and delivered)

The correctness problem below is an implementation defect, not a spec gap, so it is assessed in the Code Quality stage.

2. Code quality

Quality: Changes-Requested

P1 — listPageActive_ global latch can permanently freeze sidebar polling for active tasks

ChatSummaryHistory.tsx:39,95,103,190-198,204-207

listPageActive_ is set true on any overlapping heartbeat (handleBatchHeartbeat_, line 196) and is reset only by summary-list-unmount (handleListPageUnmount_, line 205). Once true, maybeStartPoll() (line 95) and every poll tick (line 103) early-return, so the sidebar's own polling cannot restart by any other path. This drops the staleness self-heal that the established reference implementation in the same package relies on:

// SummaryDetailPage.tsx:468-470 (reference pattern this PR mirrors)
if (this.listPageActive && Date.now() - this.lastEventTime > 15000) {
    this.listPageActive = false;   // release control if heartbeats stopped
}

Two distinct, reachable ways this freezes a live task:

1. Partial overlap (no unmount needed). The sidebar tracks N tasks (channel-scoped), but SummaryListPage's heartbeat carries only the IDs it is currently polling — which can be a subset (pagination, a status filter, or the task simply not on the current list page). A single overlapping ID latches listPageActive_ = true and calls stopPoll(), which halts polling for every sidebar task, including the non-overlapping ones the list page never polls. Those tasks then update nowhere: the sidebar is suppressed and the list page never emits a summary-status-change for IDs it isn't polling. They sit stuck (e.g. "processing") until the list page unmounts.

2. List page stops heartbeating while staying mounted. When the list page's active set empties, maybeStartBatchPoll() calls stopBatchPoll() (SummaryListPage.tsx:127-129) — it stays mounted but stops dispatching heartbeats. A task that later becomes active only in the sidebar (e.g. created from the sidebar; note SummaryListPage does not listen for chat-summary-created) is never polled by either side, because listPageActive_ is still stuck true.

Root cause: the global on/off latch is correct for SummaryDetailPage, which tracks exactly one task (this.taskId), so "overlap" means "my one task is covered." ChatSummaryHistory tracks a set of tasks, so a global latch over-suppresses. Either (a) add the same lastEventTime + TTL self-heal as the reference, or (b) coordinate per-task (suppress only the IDs present in the heartbeat) rather than flipping a component-wide flag.

P2 — Overlap set is inconsistent between the two handlers

ChatSummaryHistory.tsx:193 vs :217

handleBatchHeartbeat_ computes overlap against getActiveTaskIds() (active-only), while handleStatusChange_ computes it against this.state.items.map(...) (all items, including completed history). A summary-status-change for an already-finished history item triggers a full loadHistory() refetch even though that ID never participates in poll coordination. Minor extra network, and an inconsistent contract between the two handlers. Prefer keying both off the same active set.

P2 — Heartbeat suppression + change-only refresh can leave progress stale

ChatSummaryHistory.tsx:184 (suppression) and SummaryListPage.tsx:166-170 (dispatch on change only)

summary-status-change is dispatched by the list page only when a status transition is detected. While the sidebar is suppressed, intra-status progress updates (still PROCESSING, progress 30%→70%) produce heartbeats but no reload, so the sidebar shows no movement until a status boundary is crossed. SummaryCard rendering is status-driven, so user-visible impact is limited, but if progress freshness is intended in the sidebar this path won't deliver it.

Coverage / blind spots

• The 3 new tests cover suppression, unmount-resume, and reload-on-change — but none exercise the P1 failure mode: heartbeat latches the flag and then stops without an unmount, with no assertion of recovery. A regression test that sets the flag, advances past the reference window, and asserts the sidebar resumes polling would have caught this.
• Not verified at runtime: whether ChatSummaryPanel (sidebar) and SummaryListPage are ever simultaneously mounted in production. The fix is only load-bearing when they coexist; the PR's own premise (and the pre-existing SummaryDetailPage coordination) assumes they do, which is also the precondition for the P1 to fire.

3. Overall verdict

CHANGES_REQUESTED — Spec ✅ but one P1 (a reachable polling freeze that leaves active sidebar tasks stale indefinitely). The fix is well-scoped: the sibling SummaryDetailPage already shows the correct self-heal/TTL pattern, or move to per-task suppression.

4. Suggested direction

• Mirror SummaryDetailPage's lastEventTime + TTL release inside maybeStartPoll()/the tick, or suppress only the heartbeat's taskIds instead of latching a component-wide flag (preferred — it also fixes the partial-overlap freeze cleanly).
• Align the overlap set in handleStatusChange_ with getActiveTaskIds().
• Add a test for "heartbeat then silence (no unmount) → sidebar resumes after the recovery window."