feat(runtime): admit cross-epoch graph results - #2992
Conversation
dfdbaaf to
86d33c5
Compare
|
Warning Review limit reached
Next review available in: 12 minutes Limit details: You’ve used all 3 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
🚧 Files skipped from review as they are similar to previous changes (14)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughProblem solvedThis PR enables Agent Graph work to consume selected, committed results from completed earlier epochs of the same root Session. It adds typed Source of truthThe PR extends the existing schedule, epoch store, runtime coordinator, protocol, read-model, IPC, preload, and TUI paths. It does not create a parallel execution path. Historical results use existing epoch bindings and Solution scope and complexityThe change is the smallest coherent solution shown by the supplied diff summary. Persisted provenance, graph identity checks, historical lookup, recovery validation, protocol support, and client presentation are required for safe cross-epoch inputs. No clear deletion or simplification preserves the same behavior and regression coverage. The added fixtures and tests cover storage, runtime, runtime-host, desktop, and CLI boundaries. Validation performedThe supplied changes add coverage for:
Required check status remains unverified because direct check results were not provided. Complexity deltaThe PR adds historical epoch and graph-ID lookup, paginated discovery, selected-result resolution state, graph-specific query branches, optional fields across public boundaries, new epoch-listing APIs, refresh scheduling, and test fixtures across five packages and the desktop application. It removes no existing authority or state. It reuses the existing schedule, epoch, record, and snapshot models. Public surface and test-maintenance burden increase. The increase is bounded and justified by cross-epoch reuse, provenance, authorization, recovery, history inspection, and stale-refresh protection. Review-relevant risks
Material changes in these areas require independent human review under repository policy. The person performing the merge reviews the final diff. A maintainer makes the final determination. WalkthroughAgent Graph history now supports paginated epoch discovery, graph-specific snapshots, historical result selection, and read-only desktop and CLI views. Runtime Host, IPC, preload, storage, and runtime layers carry graph identity through these flows. ChangesAgent Graph history
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🔵 Low · up to This PR enables later graph epochs to consume selected results from earlier finished epochs. It is mergeable with owner awareness of two bounded issues: the history command can show an inaccurate mode-change message, and repeated failed historical-source lookups can add unnecessary reconciliation work. Sequence Diagram(s)sequenceDiagram
participant AgentGraphPanel
participant MakaBridge
participant RuntimeHost
participant AgentGraphCoordinator
AgentGraphPanel->>MakaBridge: list epochs and request graph snapshot
MakaBridge->>RuntimeHost: query epochs and graph snapshot
RuntimeHost->>AgentGraphCoordinator: read graph-specific data
AgentGraphCoordinator-->>RuntimeHost: return epoch metadata and snapshot
RuntimeHost-->>AgentGraphPanel: return read-only historical graph
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoAdmit selected results across Agent Graph epochs
AI Description
Diagram
High-Level Assessment
Files changed (40)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/pi-tui-runner.ts (1)
2337-2345: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the turn-running block message to cover the new
historycommand.
editor.onSubmit's turn-running gate (unchanged in this diff) rejects every non-statusGraph command with "Cannot change or start Graph Mode while a turn is running."historyis a read-only inspection, not a mode change, so this message misdescribes the block for the new command added here.✏️ Suggested wording fix
- if (graphCommand.kind === 'status') { + if (graphCommand.kind === 'status' || graphCommand.kind === 'history') { showGraphStatus(); } else { state.entries.push({ kind: 'notice', level: 'error', - text: 'Cannot change or start Graph Mode while a turn is running.', + text: 'Cannot change Graph Mode while a turn is running.', }); requestRender(); }Note: routing
historytoshowGraphStatus()above is only a placeholder; if history viewing should stay blocked mid-turn, keep the block but correct the message text to something like "Graph history is unavailable while a turn is running."
🧹 Nitpick comments (4)
packages/core/src/agent-graph-epoch.ts (1)
25-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOptional: use the core epoch contract.
packages/runtime/src/stream-graph-coordinator.tsredeclaresAgentGraphEpochPageandAgentGraphEpochStoreat lines 31-51. Import the exported core types and delete the local declarations. This prevents pagination contract drift between runtime and storage.Disposition: optional.
As per path instructions, consolidate duplicated authority and reuse the closest existing seam.Also applies to: 48-49
Source: Path instructions
packages/runtime/src/__tests__/stream-graph-coordinator.test.ts (1)
48-239: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the paginated historical cursor.
This test asserts
nextHistoricalBeforeEpoch === nullfor a two-epoch root, so the continuation path never runs.#listHistoricalSelectedResultsuseslimit: 1andbeforeEpoch: Math.min(beforeEpoch ?? current.epoch, current.epoch). Two behaviors stay untested:
- A root with three or more epochs returns a non-null
nextHistoricalBeforeEpoch, and passing it back throughhistorical_before_epochadvances to the next earlier epoch without repeating or skipping one.- An earlier epoch with no
finishyields emptyresultswhile still returning a usable cursor.The clamping and the
epoch < beforeEpochstorage semantics are the parts most likely to regress into a stalled or looping cursor.Disposition: optional. Behavior looks correct by inspection; the test would protect the cursor contract.
Source: Path instructions
packages/runtime/src/__tests__/stream-graph-schedule-reconcile.test.ts (1)
29-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd negative-path coverage for the resolver guards.
resolveSelectedResultInputsis injected here, so the new fail-closed checks inresolveSelectedResultRecordsare cheap to exercise. None is covered today:
- A resolver returning fewer records than requested must reject with "incomplete result set".
- A resolver returning a record whose
graphIdorrecordIddiffers from the request must reject with "mismatched record".- Two resolved records sharing one
recordIdmust reject as ambiguous.- A resolved record whose
recordIdalready exists in the current graph must reject as a collision (source line 201).Each case is one stub variant plus one
assert.rejects.Disposition: optional. The guards protect against a compromised or buggy Runtime Host resolver, so they deserve tests.
Source: Path instructions
apps/desktop/src/renderer/agent-graph-panel.tsx (1)
177-222: 🚀 Performance & Scalability | 🔵 TrivialConsider parallelizing
listEpochsandgetSnapshot.This effect mounts for every session shown in the chat surface, not only Graph-mode sessions (see
app-shell.tsxline 3084). The refresh now chainslistEpochsthengetSnapshot({ graphId })sequentially, adding a second IPC round trip to every session render where a singlegetSnapshotcall sufficed before.
getSnapshot(rootSessionId)without agraphIdalready resolves the current graph on the host side. WhenfollowCurrentRef.currentistrue,listEpochsandgetSnapshot(rootSessionId)(nographId) do not depend on each other and can run concurrently; reconcilenext.graphIdagainstnextEpochsafterward instead of awaitinglistEpochsfirst. Disposition: optional — the result is correct today, but the extra latency lands on every session switch, not just Graph-mode sessions.As per path instructions: "Choose remedies in this order: delete an unnecessary path, consolidate duplicated authority, reuse the closest existing seam, make the smallest local correction, then add new behavior only when the earlier options cannot satisfy the invariant."
♻️ Sketch of a parallel fetch
task = window.maka.graphs - .listEpochs(props.rootSessionId) - .then(async (nextEpochs) => { - const current = nextEpochs.find((entry) => entry.current) ?? nextEpochs[0]; - const selected = followCurrentRef.current - ? current - : nextEpochs.find((entry) => entry.graphId === selectedGraphIdRef.current); - const graphId = (selected ?? current)?.graphId; - if (!graphId) throw new Error('Agent graph epoch directory is empty'); - selectedGraphIdRef.current = graphId; - const next = await window.maka.graphs.getSnapshot(props.rootSessionId, { graphId }); - return { next, nextEpochs }; - }) + .then(async () => { + const knownGraphId = !followCurrentRef.current ? selectedGraphIdRef.current : undefined; + const [nextEpochs, next] = await Promise.all([ + window.maka.graphs.listEpochs(props.rootSessionId), + window.maka.graphs.getSnapshot( + props.rootSessionId, + knownGraphId ? { graphId: knownGraphId } : undefined, + ), + ]); + selectedGraphIdRef.current = next.graphId; + return { next, nextEpochs }; + })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e1bbd0a1-b1bb-40d3-ba5c-33b653944a6c
📒 Files selected for processing (40)
apps/desktop/src/main/__tests__/agent-graph-panel.test.tsapps/desktop/src/main/__tests__/runtime-host-session-domains-ipc-main.test.tsapps/desktop/src/main/runtime-host-client.tsapps/desktop/src/main/runtime-host-session-domains-ipc-main.tsapps/desktop/src/preload/bridge-contract.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/agent-graph-panel.tsxapps/desktop/src/renderer/app-shell.tsxapps/desktop/src/renderer/locales/shell-copy.tsapps/desktop/src/renderer/styles/agent-graph.csspackages/cli/src/__tests__/pi-tui-runner.test.tspackages/cli/src/pi-tui-runner.tspackages/cli/src/runtime-host-tui-command.tspackages/cli/src/runtime-host-tui-context.tspackages/core/src/__tests__/agent-graph-schedule.test.tspackages/core/src/__tests__/graph-command.test.tspackages/core/src/agent-graph-epoch.tspackages/core/src/agent-graph-schedule.tspackages/core/src/graph-command.tspackages/runtime-host/src/__tests__/agent-graph-coordinator.test.tspackages/runtime-host/src/__tests__/agent-graph-protocol.test.tspackages/runtime-host/src/__tests__/agent-graph-reader.test.tspackages/runtime-host/src/__tests__/agent-graph-two-client-uds.test.tspackages/runtime-host/src/client/agent-graph-reader.tspackages/runtime-host/src/client/index.tspackages/runtime-host/src/protocol/agent-graph.tspackages/runtime-host/src/protocol/operations.tspackages/runtime-host/src/server/agent-graph-coordinator.tspackages/runtime/src/__tests__/stream-graph-coordinator.test.tspackages/runtime/src/__tests__/stream-graph-handoff.test.tspackages/runtime/src/__tests__/stream-graph-schedule-reconcile.test.tspackages/runtime/src/__tests__/stream-graph-supervisor-tools.test.tspackages/runtime/src/graph-mode.tspackages/runtime/src/stream-graph-coordinator.tspackages/runtime/src/stream-graph-handoff.tspackages/runtime/src/stream-graph-read-model.tspackages/runtime/src/stream-graph-schedule-reconcile.tspackages/runtime/src/stream-graph-supervisor-tools.tspackages/storage/src/__tests__/agent-graph-epochs.test.tspackages/storage/src/sqlite-session-metadata-store.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.
Code Review by Qodo
1.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 11dc6a9a-36be-4946-a5a4-692974480edf
📒 Files selected for processing (2)
packages/runtime/src/__tests__/stream-graph-schedule-reconcile.test.tspackages/runtime/src/stream-graph-schedule-reconcile.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed the updated head 3428c3caf5a9dfe5e154c1972378a940effc07ef, including the one-commit delta, its new reconciliation tests, the complete stack comparison, current CI, and all review threads.
The latest commit correctly addresses the two CodeRabbit findings: historical result resolution is cached per reconciliation, failures are isolated by source graph, and healthy sources can continue when another source is unavailable. I found no new P0–P2 in that delta.
The branch still has one P2 stack-consistency problem. It remains diverged from #2991’s current head a520f2c1, behind the two commits that fixed atomic epoch-page observation and bounded history collection. Consequently, this head still reads the current epoch and page through separate storage observations. A rollover between those reads can return epoch N+1 while reporting N as current, causing the first-page protocol invariant to reject a valid history request.
Please rebase this branch onto the latest #2991 head rather than recreating those fixes locally. That is the smallest and cleanest correction and will also remove the stale >2,048-epoch behavior.
Non-blocking contract follow-up: the Runtime Host decoder still allows 64 current inputs plus 64 selected historical inputs, while the durable schedule contract caps their combined count at 64. Aligning the decoder and adding one rejection test would close that P3 boundary.
CI is currently red on E2E and the PR has no approval. Recommendation: rebase onto the corrected #2991 stack, rerun CI, then re-review the resulting head.
Disclosure: Codex performed the read-only latest-head, stack, lifecycle, protocol, CI, and feedback analysis. The human contributor remains responsible for verifying the evidence and deciding whether to merge.
00b062d to
58b3895
Compare
|
Both follow-ups are on
Runtime 2885 green, runtime-host 962 green, biome clean. CI is re-running on the rebased head. |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The latest commits close most of the historical-result contract: the source graph is explicit, root/epoch/closed-graph/selected-result checks are fail-closed, and failed lookups are isolated and cached during reconciliation. One ownership boundary is still lost after authorization, however: historical tuples are flattened into a global record-id namespace shared by every scheduled work item.
The simplest first-principles fix is to keep two authorities throughout reconciliation: current-graph records keyed by recordId, and historical records keyed by each work's (sourceGraphId, resultId) selections. A global map may still detect collisions, but it must not authorize or render another work's inputs. This preserves provenance without adding a compatibility path.
Review performed with Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I reproduced the failure against the latest head and verified live CI.
中文评论
最新提交已经补齐大部分历史结果契约：source graph 显式携带，root/epoch/closed graph/selected result 均 fail-closed，失败查询也在 reconciliation 中隔离并缓存。但授权之后仍丢失了一层所有权：历史 tuple 被压平成所有 work 共用的全局 record-id namespace。
更符合第一性原理的最小修复是让 reconciliation 始终保留两个权威：当前图记录按 recordId 管理；历史记录按每个 work 明确选择的 (sourceGraphId, resultId) 管理。全局 map 可以继续用于碰撞检测，但不能授权或渲染其他 work 的输入。这样无需兼容分支即可保留 provenance。
本次审查使用了 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具；我已在最新 head 上复现该失败，并复核实时 CI。
… observation listGraphEpochPage resolved the current epoch and the page rows in two separate reads, so a rollover between them produced a first page whose current marker was not on the first row, which the protocol rejects — Desktop and TUI history failed to open during an epoch transition. The epoch-page storage seam now returns the current epoch observed by the same read transaction as the rows, and the coordinator only falls back to the legacy virtual identity when the store holds no durable rows.
- The shared epoch collector no longer throws away a valid directory when more pages exist past the 64-page bound; it returns a clearly truncated directory, and Desktop/TUI surface the cap instead of failing to open history for Sessions with more than 2,048 graph runs. - Desktop resumes following the current epoch when the explicitly selected historical epoch leaves the bounded directory, instead of staying pinned on the fallback graph. - The TUI stops the history picker flow when it shut down while page reads were in flight.
22a4ebe to
ebedf52
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Cross-epoch result admission belongs in the existing graph authority, and the PR correctly binds results to durable epoch identities instead of introducing another runtime state source. Two inherited history-surface gaps remain on this head. The smallest path is to rebase onto the corrected #2991 history foundation, then keep the selected graph identity as explicit UI state so controls cannot act through a stale snapshot during loading.
storybook is still running on the current head.
AI-assisted review disclosure: Codex verified the findings against the current head and traced epoch authority, root ownership, Desktop selection/refresh fencing, and root-scoped stop/dismiss actions. Two independent reviewer-agent passes and an OpenCode Go DeepSeek V4 Flash (high) adversarial pass were used as inputs. No local tests were run.
中文复核
跨 epoch result admission 应继续由现有 Graph 权威负责，这个 PR 正确地把结果绑定到持久 epoch identity，没有建立第二份 runtime 状态源。当前 head 还继承了历史浏览层的两个缺口。最小方案是先 rebase 到修正后的 #2991 foundation，再把 selected graph identity 作为明确 UI state，使加载期间的 control 不会通过旧 snapshot 作用到当前 Graph。当前 storybook 仍在运行。
本次为 AI 辅助审查：Codex 在最新 head 上追踪 epoch authority、root ownership、Desktop selection/refresh fencing 与 root-scoped stop/dismiss；另使用两次独立 reviewer 及一次 OpenCode Go DeepSeek V4 Flash（high）对抗审查。未运行本地测试。
ebedf52 to
0020615
Compare
Generated-by: Maka
0020615 to
31b9faf
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
This is now a coherent cross-epoch admission design. Historical results remain explicitly selected, same-root, earlier-epoch, source-finished, and RuntimeEvent-backed; (sourceGraphId, resultId) provenance survives authorization and reconciliation; current and historical records stay separated; and resolver failures defer only the affected source. Runtime Host and the durable schedule remain the existing authorities rather than introducing a parallel result store.
The test suite covers the important adversarial boundaries: cross-root and current/future epoch rejection, finish membership, recovery/reconciliation, provenance collisions, resolver failure caching, protocol limits, and read-only Desktop/TUI history. I found no concrete P0–P3 on the exact current head. Live required checks are green and merge state is clean.
Reviewed with Codex using three independent reviewer agents and OpenCode Go DeepSeek V4 Flash (high); I verified the exact head, current-main integration, authority boundaries, persistence/protocol contracts, and live CI.
中文
当前方案已经形成一致的跨 epoch admission 设计。历史结果必须显式选择、同 root、来自更早 epoch、source 已 finish，并由 RuntimeEvent 记录支撑；(sourceGraphId, resultId) provenance 在授权和 reconciliation 中都被保留；当前与历史 records 不混用；resolver 失败只延后受影响 source。Runtime Host 与 durable schedule 继续作为既有权威，没有新增并行结果存储。
测试覆盖了关键对抗边界：跨 root、当前/未来 epoch 拒绝、finish membership、恢复/reconciliation、provenance 冲突、resolver 失败缓存、协议上限，以及 Desktop/TUI 历史只读。当前精确 head 未发现具体 P0–P3；实时必需检查全绿，merge state clean。
本次由 Codex 配合三个独立 reviewer agent，以及 OpenCode Go DeepSeek V4 Flash（high）审查；我核验了精确 head、与当前 main 的集成、权威边界、持久化/协议契约和实时 CI。
2 participants
Summary
selected_result_inputswith the source graph identity for scheduled workHistorical results cross the epoch boundary as immutable data only. Runtime Host authorization verifies the root, epoch ordering, finished schedule selection, and exact RuntimeEvent-backed record before the schedule is committed; recovery performs the same resolution again. Input counts and discovery pages are bounded, and ambiguous current/historical record identities fail closed.
This completes the cross-epoch result-input slice of #2588 after the historical-read slice.
Depends on #2991
Refs #2588
This is intentionally opened as a Draft during the pre-Incubator stabilization window. It is not requesting merge until feature work resumes.
Verification
npm run build:testnpm run typecheckgit diff --checkChecklist
Does this PR entail a change in behavior?
中文说明
摘要
selected_result_inputsfinish中明确选择的已提交结果跨 epoch 传递的只有不可变数据，不包含旧图的控制权。Runtime Host 会在 schedule 提交前验证 root、epoch 顺序、旧图的
finish选择以及由 RuntimeEvent 支撑的精确 record；崩溃恢复时再次验证。输入数量与发现页都有上限，当前图和历史图的 record identity 若出现歧义会 fail closed。这是 #2588 的跨 epoch 结果输入部分，接续前一项历史读取改动。
本 PR 在进入孵化器前的稳定期内以 Draft 打开；在 feature 合并恢复前不请求进入
main。AI disclosure / AI 披露
This PR was implemented by Codex under me2seeks’s direction and review. / 本 PR 由 Codex 在 me2seeks 的指导与审核下完成。