fix(runtime-host): page Session traces at the source - #3133
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review. 📝 WalkthroughWhat problem this solvesThe Runtime Host previously loaded complete Session traces before display. Large or corrupt Sessions could exceed live Host evidence limits and fail inspection. This PR paginates traces at the Storage source. Each page uses evidence, turn, and result budgets. Opaque cursors support continuation. Oversized and unreadable evidence produces partial coverage instead of failure. The Desktop loads earlier pages on demand. Usage and cache summaries load independently from the timeline. Source of truthThis PR extends the existing Storage and usage sources of truth. It does not create a parallel trace dataset. The Runtime Host reads paginated Durable usage writes publish session-scoped invalidations. The Runtime Host coalesces these events. The Desktop consumes them through the current Host epoch. Scope and complexityThis is the smallest coherent solution for bounded inspection:
Cursor validation, refresh guards, refresh coalescers, coverage tracking, session scoping, and migration logic are necessary for continuation, refresh races, corrupt evidence, usage isolation, and mixed-version boundaries. Deletion and simplification opportunitiesThe obsolete complete-trace loader, revision/offset pagination, timeline filter state, filter models, aggregate trace totals, and related styles were removed. No further deletion is evident from the supplied diff. The added tests cover pagination, refresh races, unreadable and oversized evidence, composite identity, usage scoping, invalidation, migrations, protocol validation, and stale retry supersession. Risks and validationUser-visible behavior changes include:
Public contracts changed in the preload bridge, Runtime Host inspection protocol, Storage interfaces, usage query types, and session-domain notifications. The Runtime Host compatibility epoch changed from 22 to 23. The SQLite usage schema changed from version 3 to 4 and adds session-scoped indexes. Final test, build, lint, typecheck, and Storybook results remain unverified because no direct check output was provided. Review-relevant risks
The person performing the merge reviews the final diff. A maintainer makes the final determination. WalkthroughThe session inspector now uses session-scoped usage summaries and cursor-based trace pages. Runtime usage changes propagate through continuity and IPC notifications. The desktop UI displays cost, cache-hit rate, coverage status, timestamps, and earlier trace pages. ChangesSession inspector data flow
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🔵 Low · up to The PR makes Session traces paged and keeps summaries Session-wide, with focused tests and builds passing. It is mergeable with owner awareness that one test fixture depends on the node:sqlite runtime and storage-internal names, which could cause unrelated test failures if those interfaces change. Sequence Diagram(s)sequenceDiagram
participant UsageWriter
participant RuntimeHost
participant InspectorBridge
participant UseSessionTrace
participant InspectorPanel
UsageWriter->>RuntimeHost: publish session usage change
RuntimeHost->>InspectorBridge: usage:changed(sessionId)
InspectorBridge->>UseSessionTrace: notify usage change
UseSessionTrace->>InspectorBridge: request summary or trace page
InspectorBridge-->>UseSessionTrace: return summary or nextCursor
UseSessionTrace->>InspectorPanel: provide inspector snapshot
<f_fixed_issue_severity>Low</f_fixed_issue_severity> 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
bc35fc6 to
8096203
Compare
There was a problem hiding this comment.
Pull request overview
This PR reworks Session inspection to page trace data at the Storage source (instead of materializing entire Sessions in the Runtime Host), while keeping Session-wide usage/cost summaries independent of timeline pagination. It also introduces Session-scoped usage invalidations so Desktop can refresh usage estimates from the Usage authority rather than inferring from Session events.
Changes:
- Add Session-scoped indexing/querying for usage + model-call ledgers (including schema migration/backfill) and publish Session usage-change notifications after durable writes.
- Introduce cursor-based, bounded Session trace paging in Runtime Host and propagate a new
usagesession domain for continuity invalidations. - Update Desktop Inspector to load only a bounded trace window initially, append earlier pages on demand, and fetch Session-wide usage summary independently.
Reviewed changes
Copilot reviewed 44 out of 45 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/storage/src/usage-stores.ts | Adds Session-scoped model-call paging and a Session usage-change subscription API on the usage writer facade. |
| packages/storage/src/sqlite-usage-store.ts | Stores session_id for legacy LLM rows, clamps cache-read tokens, and scopes reads by (session_id, ts) where applicable. |
| packages/storage/src/sqlite-usage-schema.ts | Bumps schema version and migrates/backfills session_id columns + supporting indexes. |
| packages/storage/src/model-call-ledger.ts | Adds optional Session scoping for model-call reads and pending reprojection queries; persists session_id. |
| packages/storage/src/execution-stores.ts | Plumbs a new AgentRun paging API through execution stores. |
| packages/storage/src/agent-run-store.ts | Implements listSessionRunsPage with stable (created_at, run_id) cursor semantics. |
| packages/storage/src/tests/usage-stores.test.ts | Adds coverage for Session usage-change publication and legacy summary clamping/scoping behavior. |
| packages/storage/src/tests/sqlite-usage-schema.test.ts | Verifies migration backfills Session identity + creates indexes. |
| packages/storage/src/tests/sqlite-core-execution-store.test.ts | Tests stable AgentRun paging order and cursor behavior. |
| packages/storage/src/tests/model-call-ledger.test.ts | Ensures Session-scoped ledger reads exclude other Sessions (including corrupt rows). |
| packages/runtime/src/session-trace-projection.ts | Moves trace identity to (runId, turnId) and introduces oversizedRuns coverage. |
| packages/runtime/src/tests/session-trace-projection.test.ts | Adds tests for per-run turn identity separation and partial coverage classification. |
| packages/runtime-host/src/server/execution-inspect-coordinator.ts | Replaces revision/offset paging with cursor-based, bounded Session trace pages assembled from run pages. |
| packages/runtime-host/src/server/execution-composition.ts | Subscribes to usage-change notifications and emits usage session-domain invalidations. |
| packages/runtime-host/src/server/canonical-usage-reader.ts | Scopes pending repairs and attempt reads by sessionId where requested. |
| packages/runtime-host/src/protocol/usage-pricing.ts | Allows sessionId in usage query decoding. |
| packages/runtime-host/src/protocol/session-continuity.ts | Adds usage to the set of Session domains. |
| packages/runtime-host/src/protocol/index.ts | Advances compatibility epoch to 23. |
| packages/runtime-host/src/protocol/execution-inspect.ts | Updates Session trace inspect protocol to use an opaque cursor and nextCursor (removing revision/offset). |
| packages/runtime-host/src/tests/usage-pricing-protocol.test.ts | Adds test ensuring Session summary doesn’t repair/report other Sessions’ pending projections. |
| packages/runtime-host/src/tests/session-continuity-coordinator.test.ts | Extends domain invalidation coalescing tests to include usage. |
| packages/runtime-host/src/tests/protocol.test.ts | Adjusts imports/order due to protocol constant changes. |
| packages/runtime-host/src/tests/execution-inspect-protocol.test.ts | Updates protocol tests for cursor-based paging and expanded coverage fields. |
| packages/runtime-host/src/tests/execution-inspect-coordinator.test.ts | Adds end-to-end tests for paging, oversized runs/results, corrupt evidence handling, and cursor validation. |
| packages/core/src/usage-stats/types.ts | Adds optional sessionId to usage queries. |
| packages/core/src/session-trace.ts | Introduces (runId, turnId) identity helpers and adds oversizedRuns; adds trace/coverage merge helpers. |
| packages/core/src/model-call-usage-projection.ts | Adds Session filtering + clamps cache-read tokens via exported helper. |
| packages/core/src/tests/session-trace.test.ts | Tests coverage merge semantics and multi-page trace merge behavior. |
| packages/core/src/tests/model-call-usage-projection.test.ts | Tests cache-read clamping and Session scoping in selection/projection. |
| docs/astryx-surface-file-inventory.md | Updates Astryx surface inventory to reflect Inspector component usage changes. |
| apps/desktop/stories/session-workbar.stories.tsx | Updates story fixtures for new trace identity/coverage and adds a “load more history” story path. |
| apps/desktop/src/renderer/use-session-trace.ts | Refactors trace loading into paged windows + independent summary/context reads and refresh signals. |
| apps/desktop/src/renderer/styles/chat-detail.css | Removes CSS tied to Inspector search/filter UI that was deleted. |
| apps/desktop/src/renderer/session-trace-refresh.ts | Generalizes refresh coalescing for authority invalidations and keeps trace-specific event coalescer. |
| apps/desktop/src/renderer/session-inspector-panel.tsx | Removes in-memory search/filter UI, adds “Load earlier records”, and switches turn labeling to stable timestamps. |
| apps/desktop/src/renderer/session-inspector-panel-model.ts | Updates panel model to carry run identity + startedAt and to surface oversizedRuns. |
| apps/desktop/src/renderer/session-inspector-overview-model.ts | Moves cache hit rate + cost estimate to be derived from Session-wide usage summary (not paged trace). |
| apps/desktop/src/renderer/session-inspector-filter.ts | Removes the Inspector timeline filter implementation (no longer supported with paged data). |
| apps/desktop/src/renderer/locales/conversation-copy.ts | Updates copy for paged timeline/usage summary states and new coverage wording/fields. |
| apps/desktop/src/preload/preload.ts | Changes Inspector trace API to return { trace, nextCursor }; adds usage summary call and usage-change subscription. |
| apps/desktop/src/preload/bridge-contract.d.ts | Extends bridge contract with trace-page + Session usage summary types and usage-change subscription. |
| apps/desktop/src/main/runtime-host-session-domains-ipc-main.ts | Routes usage domain invalidations to a usage:changed renderer event and includes it in resync. |
| apps/desktop/src/main/tests/use-session-trace.test.ts | Adds tests for usage-only refresh, page-depth rebuild on refresh, cursor stability, and summary failure behavior. |
| apps/desktop/src/main/tests/session-inspector-panel-model.test.ts | Adds tests for cost-estimate semantics and availability heuristics; updates coverage expectations. |
| apps/desktop/src/main/tests/runtime-host-session-domains-ipc-main.test.ts | Verifies usage:changed dispatch and resync behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![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
🧹 Nitpick comments (7)
packages/core/src/session-trace.ts (1)
676-699: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAdd a
turnIdtiebreak to the page merge comparator.The comparator compares
startedAtthenrunId. Two turns of the same run that sharestartedAtkeep insertion order, so the merged order depends on which page arrived first.orderedTurnIdentitiesinpackages/runtime/src/session-trace-projection.ts(lines 494-501) already breaks the same tie byrunIdthenturnId. Aligning both comparators makes the merged order independent of page arrival order.♻️ Proposed comparator alignment
const ordered = [...turns.values()].sort( - (left, right) => left.startedAt - right.startedAt || left.runId.localeCompare(right.runId), + (left, right) => + left.startedAt - right.startedAt || + left.runId.localeCompare(right.runId) || + left.turnId.localeCompare(right.turnId), );packages/runtime-host/src/__tests__/execution-inspect-coordinator.test.ts (2)
296-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the continuation cursor instead of returning early.
Line 301 returns when
first.result.nextCursoris falsy. If pagination stops emitting a cursor for fractionalcreatedAtvalues, this test passes without checking anything, which is the exact regression it exists to catch. Assert the cursor, then continue.💚 Proposed assertion
assert.equal(first.ok, true); - if (!first.ok || first.result.kind !== 'session_trace_page' || !first.result.nextCursor) - return; + if (!first.ok || first.result.kind !== 'session_trace_page') return; + assert.ok(first.result.nextCursor, 'a 17th run must leave a continuation cursor');As per path instructions: "Flag tests that duplicate existing coverage, assert implementation details, or do not protect observable behavior."
Source: Path instructions
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the corruption update changes one row.
Capture
run()and assertchanges === 1so storage schema drift fails with an explicit fixture error.packages/core/src/__tests__/session-trace.test.ts (1)
26-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the untested
nonefold direction and the two page-merge rejections.The table omits
['absent', 'none', 'absent'], so thenext.modelCalls === 'none'branch inmergeDisjointTraceCoverage(packages/core/src/session-trace.tsline 648) is never executed. One extra row closes that.
mergeSessionTracesalso guarantees two rejections that no test exercises: an empty page list, and pages that disagree onsessionIdorschemaVersion. Both are stated contracts and both are cheap to assert.💚 Proposed additions
const cases = [ ['none', 'absent', 'absent'], + ['absent', 'none', 'absent'], ['absent', 'absent', 'absent'], ['no_known_gap', 'no_known_gap', 'no_known_gap'], ['absent', 'no_known_gap', 'partial'], ['partial', 'no_known_gap', 'partial'], ] as const;assert.equal(merged.totals.inputTokens, 3); + + assert.throws(() => mergeSessionTraces([]), /At least one Session trace page/); + assert.throws( + () => mergeSessionTraces([page('run-1', 1, 1), { ...page('run-2', 2, 2), sessionId: 'other' }]), + /same Session/, + ); });Also applies to: 66-76
apps/desktop/src/preload/preload.ts (1)
798-807: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winWrap
loadSessionUsageSummaryinbridgeResultso the declaredResultcontract always holds.
inspector.traceandinspector.contextconvert a thrown error into{ ok: false, error }.inspector.summaryreturns the rawipcRenderer.invokepromise cast toResult<DesktopSessionUsageSummary>. IfruntimeHostSessionRefthrows, or theusage:summaryhandler rejects, the returned promise rejects instead of resolving to aResult. The declared return type then does not describe the runtime behavior.
use-session-trace.tscurrently passes a rejection handler, so the UI still recovers. The contract inconsistency remains.♻️ Uniform Result envelope
-async function loadSessionUsageSummary( - sessionId: string, -): Promise<Result<DesktopSessionUsageSummary>> { - const session = await runtimeHostSessionRef(sessionId); - return ipcRenderer.invoke( - 'usage:summary', - session.scope, - { range: 'all', sessionId: session.sessionId }, - ) as Promise<Result<DesktopSessionUsageSummary>>; -} +function loadSessionUsageSummary( + sessionId: string, +): Promise<Result<DesktopSessionUsageSummary>> { + return bridgeResult(async () => { + const session = await runtimeHostSessionRef(sessionId); + const result = await ipcRenderer.invoke( + 'usage:summary', + session.scope, + { range: 'all', sessionId: session.sessionId }, + ) as Result<DesktopSessionUsageSummary>; + if (!result.ok) throw new Error(result.error.message); + return result.data; + }, 'INSPECTOR_SUMMARY_FAILED'); +}Also applies to: 2281-2283
apps/desktop/src/main/__tests__/use-session-trace.test.ts (1)
183-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
tracePageproducesstartedAt: NaNfor non-numeric run ids, so the ordering assertions rely on the merge tie-break.Line 188 computes
Number(runId.replace(/\D/g, '')). For'run-z','run-m', and'run-n'the digit set is empty, soNumber('')is0… no:runId.replace(/\D/g,'')yields''andNumber('')is0. For these ids the value is0, so every turn sharesstartedAt: 0andmergeSessionTracesfalls back torunId.localeCompare. The expected orders['run-m','run-z']and['run-n','run-z']therefore assert the tie-break, not chronological ordering.Give the fixture an explicit
startedAtso the test states the ordering it means.♻️ Explicit timestamps in the fixture
function tracePage( sessionId: string, runId: string, nextCursor: string | null, + startedAt = Number(runId.replace(/\D/g, '')) || 0, ): DesktopSessionTracePage { - const startedAt = Number(runId.replace(/\D/g, '')); return {Also applies to: 400-440
apps/desktop/src/main/__tests__/session-inspector-panel-model.test.ts (1)
159-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the cast so the assertion still protects the
InspectorTurnRowcontract.
InspectorTurnRowdeclaresstartedAt: number. The cast to{ startedAt?: number } | undefinedmakes the test compile even ifstartedAtis removed from the model, which is the field this test exists to protect.♻️ Assert the typed field directly
- const turn = deriveInspectorPanelModel(trace).turns[0]; - assert.equal((turn as { startedAt?: number } | undefined)?.startedAt, 1); + const turn = deriveInspectorPanelModel(trace).turns[0]; + assert.ok(turn); + assert.equal(turn.startedAt, 1);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e684a90-c382-4a33-8c4c-e884bcd01081
📒 Files selected for processing (45)
apps/desktop/src/main/__tests__/runtime-host-session-domains-ipc-main.test.tsapps/desktop/src/main/__tests__/session-inspector-panel-model.test.tsapps/desktop/src/main/__tests__/use-session-trace.test.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/locales/conversation-copy.tsapps/desktop/src/renderer/session-inspector-filter.tsapps/desktop/src/renderer/session-inspector-overview-model.tsapps/desktop/src/renderer/session-inspector-panel-model.tsapps/desktop/src/renderer/session-inspector-panel.tsxapps/desktop/src/renderer/session-trace-refresh.tsapps/desktop/src/renderer/styles/chat-detail.cssapps/desktop/src/renderer/use-session-trace.tsapps/desktop/stories/session-workbar.stories.tsxdocs/astryx-surface-file-inventory.mdpackages/core/src/__tests__/model-call-usage-projection.test.tspackages/core/src/__tests__/session-trace.test.tspackages/core/src/model-call-usage-projection.tspackages/core/src/session-trace.tspackages/core/src/usage-stats/types.tspackages/runtime-host/src/__tests__/execution-inspect-coordinator.test.tspackages/runtime-host/src/__tests__/execution-inspect-protocol.test.tspackages/runtime-host/src/__tests__/protocol.test.tspackages/runtime-host/src/__tests__/session-continuity-coordinator.test.tspackages/runtime-host/src/__tests__/usage-pricing-protocol.test.tspackages/runtime-host/src/protocol/execution-inspect.tspackages/runtime-host/src/protocol/index.tspackages/runtime-host/src/protocol/session-continuity.tspackages/runtime-host/src/protocol/usage-pricing.tspackages/runtime-host/src/server/canonical-usage-reader.tspackages/runtime-host/src/server/execution-composition.tspackages/runtime-host/src/server/execution-inspect-coordinator.tspackages/runtime/src/__tests__/session-trace-projection.test.tspackages/runtime/src/session-trace-projection.tspackages/storage/src/__tests__/model-call-ledger.test.tspackages/storage/src/__tests__/sqlite-core-execution-store.test.tspackages/storage/src/__tests__/sqlite-usage-schema.test.tspackages/storage/src/__tests__/usage-stores.test.tspackages/storage/src/agent-run-store.tspackages/storage/src/execution-stores.tspackages/storage/src/model-call-ledger.tspackages/storage/src/sqlite-usage-schema.tspackages/storage/src/sqlite-usage-store.tspackages/storage/src/usage-stores.ts
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/styles/chat-detail.css
- apps/desktop/src/renderer/session-inspector-filter.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
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: 366d7faa-58e8-4deb-a552-6d3defde1b32
📒 Files selected for processing (8)
apps/desktop/src/main/__tests__/use-session-trace.test.tsapps/desktop/src/renderer/use-session-trace.tspackages/core/src/__tests__/session-trace.test.tspackages/core/src/session-trace.tspackages/storage/src/__tests__/sqlite-core-execution-store.test.tspackages/storage/src/__tests__/usage-stores.test.tspackages/storage/src/agent-run-store.tspackages/storage/src/sqlite-usage-store.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/core/src/tests/session-trace.test.ts
- packages/storage/src/tests/sqlite-core-execution-store.test.ts
- packages/storage/src/tests/usage-stores.test.ts
- packages/storage/src/sqlite-usage-store.ts
- packages/core/src/session-trace.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/desktop/src/renderer/session-inspector-panel.tsx:639
formatTurnStartedAtallocates a newIntl.DateTimeFormaton every row render. For long traces this can become a measurable hotspot; consider caching the formatter per-locale (similar tonumberFormatter) and only formatting theDateper call.
function formatTurnStartedAt(startedAt: number, locale: UiLocale): string {
const date = new Date(startedAt);
if (!Number.isFinite(date.getTime())) return '—';
return new Intl.DateTimeFormat(uiLocaleToIntlLocale(locale), {
dateStyle: 'short',
|
/agentic_review Automated request by Codex on behalf of @Astro-Han to verify the newly installed Qodo OSS review integration. |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/storage/src/sqlite-usage-schema.ts:88
ensureColumnbuilds SQL by interpolatingtable/columndirectly into the statement. Even though current callers pass constants, this helper is now a footgun (and potential injection vector) if reused with non-validated identifiers later. Consider validatingtable/columnagainst a strict identifier regex before composing SQL.
9d276d1 to
55e0516
Compare
|
Heads-up on a cross-PR collision — not a review comment on your change.
The trap is that this does not conflict. All three branches write the same text to that line, so git's three-way merge takes it silently; only the adjacent comment block conflicts, and keeping both comments is the natural resolution. Each PR's own Please re-check against (Posted with Claude Code (Opus 5) assistance; the epoch values were read from each branch head.) |
1f50987 to
f20354b
Compare
|
Rebased onto the latest New head:
All passed. @hqhq1025, the previous approval was necessarily dismissed by the rebased head; please re-approve this exact head when convenient. Once the fresh required CI is green, the contributor has authorized squash merge. Posted by Codex on behalf of the contributor. |
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Re-review of f20354b
No new code finding was introduced by the latest revision. The range-diff against the previously reviewed head shows the Session paging, Usage authority, repair checkpoint, invalidation, and Desktop state commits are patch-equivalent after the rebase. The earlier repair-intent race remains fixed by the transactional AgentRun high-water and applied-through checkpoint model.
Problem and mechanism: this PR correctly moves long-Session trace pagination to the Storage source while keeping Session-wide Usage independent from the visible timeline. Composite (runId, turnId) identity, bounded continuation pages, explicit incomplete coverage, and Session-scoped Usage invalidation remain the right ownership boundaries.
First principles, optimality, and deletion: the final architecture derives repair lag from durable authority state instead of maintaining a second marker authority, which is both simpler and safer. I found no additional production code or low-value test that must be removed, and no deeper refactor is required.
Current merge gate: not ready to merge only because current main advanced after this head and GitHub now reports a content conflict in packages/runtime-host/src/protocol/index.ts and packages/runtime-host/src/__tests__/protocol.test.ts. Both branches independently consumed compatibility epoch 34. Rebase onto current main, preserve the epoch-34 ScheduledTask entry, move this PR's Session-trace protocol change to epoch 35, and update the exact epoch regression accordingly. The new protocol epoch guard is expected to reject a same-number resolution.
After that rebase, this needs a short exact-head check of the two conflict resolutions and CI rather than another full architectural review.
Verification: required CI is green on f20354b; npm run build:test and git diff --check pass locally; the range-diff confirms all 19 previously reviewed follow-up commits are equivalent apart from the base-driven epoch renumbering.
Read the latest trace page without materializing the full Session, expose stable older-history cursors, and keep the Inspector's usage estimate scoped to the complete Session. Generated-by: Codex
Degrade Session trace pages that exceed result or turn limits into explicit unreadable coverage while preserving their continuation cursor. Validate opaque cursor payloads before they reach Storage so malformed input is reported as invalid_request. Generated-by: Codex
Keep Session trace pages keyed by their request cursor, reconnect refreshed head pages to the existing tail, and derive totals and coverage only from the connected disjoint window. Separate Session lifetime from head refresh revisions so earlier-page reads survive live updates, and remove stale Session summaries after a failed refresh. Generated-by: Codex
Route Inspector summaries through the existing Host-scoped usage IPC instead of widening the generic renderer allowlist. Clamp cache-read tokens per canonical and legacy record so one malformed attempt cannot inflate the full-Session cache rate. Generated-by: Codex
Cover Storage keyset ordering and append stability directly, replace the 2,800-write aggregate evidence fixture with two projection-ignored bounded records, and remove the literal compatibility-epoch assertion that only repeated a production constant. Generated-by: Codex
Delete the unused overview trace input, panel-level trace totals, dead duration copy, and search-era comments and classes now that the complete Session summary and paged timeline have distinct owners. Generated-by: Codex
Provide Session usage summary fixtures through the Storybook inspector bridge so trace stories exercise the same scoped IPC contract as the desktop renderer. Generated-by: Codex
Rebuild the visible trace prefix from the source on refresh, fill Host pages by the existing evidence budgets, and keep continuation cursors valid for every accepted AgentRun timestamp. Push Session identity into the Usage indexes so summaries, repair, and provenance are scoped before decoding, while keeping timeline and summary refresh policies independent. Remove window-relative turn numbering and move pure page merging to Core. Generated-by: Codex
Keep Session usage refreshes on the Usage authority, distinguish oversized evidence from unreadable records, and use composite Run/Turn identities throughout trace pagination. Generated-by: Codex
Append one earlier page per user request, keep Usage invalidations live across Host epochs, and preserve known unreadable evidence in trace coverage. Generated-by: Codex
Validate pagination cursors, make trace ordering deterministic, keep usage bucket cache accounting consistent, and settle superseded loading state. Generated-by: Codex
Generated-by: Codex
Publish Session usage changes only after durable usage mutations succeed, including pending-reprojection marker creation and removal. Generated-by: Codex
Keep the mainline pricing-key regression fixture valid after Session trace coverage gains oversized-run accounting. Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Replace the independent reprojection marker lifecycle with a durable per-run checkpoint derived from the canonical AgentRun event sequence. Normal accounting and query-time repair now share one bounded catch-up path, preserving unreadable evidence without allowing projection failures to advance progress. Generated-by: Codex
Maintain a rebuildable per-run model-call sequence index in the same transaction as the canonical AgentRun append. Backfill existing runs during migration so Usage catch-up discovers lagging projections without rescanning the complete event history. Generated-by: Codex
f20354b to
58de960
Compare
|
Hi @hqhq1025, @M4n5ter, and @likun666661 — could one of you please review the current head This branch has been rebased onto current Current-head validation is complete:
All existing review threads are resolved. An approval from a committer other than the author is the final merge gate. Posted by Codex on behalf of the contributor. |
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Re-review of 58de960
No actionable findings remain.
Problem and mechanism: the PR fixes unbounded Session inspection by paging AgentRuns at the Storage source, while keeping Session-wide Usage independent from the visible timeline and refreshing it through Session-scoped invalidations. Composite (runId, turnId) identity, bounded continuation pages, explicit incomplete coverage, and the transactional authority high-water/applied checkpoint model remain the correct ownership boundaries.
Latest rebase: the range-diff confirms all 20 PR commits are patch-equivalent to the previously reviewed revision except for the required protocol conflict resolution. Current main owns compatibility epoch 34 for backend-free ScheduledTask templates; this revision preserves that history and advances Session trace pagination and Usage wire changes to epoch 35. The exact epoch assertion and merge-result guard agree with that ordering.
First principles, optimality, deletion, and tests: deriving projection lag from durable authority sequence minus the applied checkpoint remains simpler and safer than a second repair-marker authority. I found no additional production code or low-value test that should be deleted, and no deeper refactor is required.
Merge verdict: ready to merge. Required CI is green, GitHub reports the head mergeable, git diff --check passes, the protocol epoch guard reports 34 -> 35, and all eight epoch-guard tests pass. The previously documented corrupt persisted trace-row behavior and bounded catch-up latency remain non-blocking residual risks.
Summary
session_idbefore decoding, so the summary remains Session-wide while the visible timeline stays bounded.usagedomain, and Desktop subscribes synchronously through the current Host epoch instead of inferring cost changes from Session events.(runId, turnId)as the trace identity throughout projection, coverage, merge, and rendering. Distinguish oversized evidence from corrupt/unreadable evidence, and treat either known gap as partial rather than claiming the backend records no call details.This changes user-visible Inspector behavior: long Sessions open normally, summary figures remain Session-wide estimates, each click loads one bounded earlier page, and incomplete accounting is never presented as a known zero or a complete timeline.
Screenshots
The same 944-record, 1.18 MB Session is shown before and after the change.
Additional Storybook verification covers the continuation control above the ascending timeline, one-page loading, stable timestamp labels, and explicit incomplete coverage. A clean isolated run produced no console warnings or errors.
Verification
npm --workspace @maka/core run buildnpm --workspace @maka/runtime run buildnpm --workspace @maka/storage run buildnpm --workspace @maka/runtime-host run buildnpm --workspace @maka/desktop run build:mainnpm --workspace @maka/desktop run typechecknpm run astryx:surface-inventorygit diff --check origin/main...HEADTraceMoreHistory: click-to-load appends the older page and a fresh isolated page reports no console warnings or errors.Review
AI use
Select exactly one:
Tool(s) and scope: Codex diagnosed the failure, shaped and authored the implementation and tests, adjudicated adversarial reviews, resolved the latest-main rebase, and performed focused and visual verification. Claude Fable provided earlier read-only architecture and code reviews. The human contributor must review the final diff, screenshots, and commit messages before merge.
Checklist
Does this PR entail a change in behavior?