feat(agent-runtime): expose per-message/tool/run timing via eggExt namespace#457
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughAdds millisecond-granular timing fields ( ChangesMillisecond-granular timing and persistedAtMs message stamping
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
core/agent-runtime/src/AgentRuntime.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. core/agent-runtime/src/AgentStoreUtils.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. core/agent-runtime/src/MessageConverter.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces millisecond-granular timing fields (startedAtMs, completedAtMs, durationMs, and apiDurationMs) across runs and messages, along with a server-side persistedAtMs timestamp for stored messages. The changes include helper functions, message extraction logic, updated types, and comprehensive unit tests. The review feedback highlights three key improvements: defensively guarding against null or undefined messages in MessageConverter, preventing negative run durations due to potential system clock drift in RunBuilder, and ensuring arrays are not incorrectly processed as objects in OSSAgentStore.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![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)
core/types/agent-runtime/AgentMessage.ts (1)
128-139:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
duration_api_mstoSDKResultMessageto close the cross-file contract gap.
MessageConverter.extractApiDurationMsconsumesresult.duration_api_ms, butSDKResultMessagedoesn’t declare it. That forces unsafe casts and makes this new timing path type-unsafe across files.Suggested patch
export interface SDKResultMessage extends AgentMessageExtensions { type: 'result'; subtype: string; + /** Pure model API duration in ms, when provided by the SDK result payload. */ + duration_api_ms?: number; usage?: { input_tokens?: number; output_tokens?: number;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/types/agent-runtime/AgentMessage.ts` around lines 128 - 139, The SDKResultMessage interface is missing the duration_api_ms property that is being consumed by MessageConverter.extractApiDurationMs, creating a type-safety gap. Add an optional duration_api_ms field with a number type to the SDKResultMessage interface definition to properly declare this contract and eliminate unsafe casting across files.
🧹 Nitpick comments (1)
core/agent-runtime/test/MessageConverter.test.ts (1)
129-133: ⚡ Quick winExtend this suite with NaN/Infinity/negative duration cases.
Given timing is now a surfaced contract, add edge-case assertions so invalid numeric values are ignored consistently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/agent-runtime/test/MessageConverter.test.ts` around lines 129 - 133, Add additional test cases to the MessageConverter test suite to cover edge cases for invalid duration values. After the existing test for non-numeric duration_api_ms in the 'should ignore non-numeric duration_api_ms' test block, add separate test cases that verify MessageConverter.extractApiDurationMs returns undefined for NaN, Infinity, and negative numeric values passed as duration_api_ms. Each test should follow the same assertion pattern as the existing test, passing the respective invalid value and asserting the result is undefined, ensuring all invalid numeric and non-numeric duration values are handled consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/agent-runtime/src/MessageConverter.ts`:
- Around line 47-50: The type check for duration_api_ms (the variable d) is
insufficient because typeof d === 'number' accepts NaN, Infinity, and negative
values. Replace the condition that checks `typeof d === 'number'` in the
duration_api_ms validation block with a stricter check using Number.isFinite(d)
&& d >= 0 to ensure only valid, finite, non-negative numbers are accepted and
accumulated into the total.
In `@core/agent-runtime/test/OSSAgentStore.test.ts`:
- Line 115: The linter requires spaces inside array brackets. In the
appendMessages call, change the array argument from [original] to [ original ]
by adding a space after the opening bracket and before the closing bracket to
satisfy the array-bracket spacing requirement.
---
Outside diff comments:
In `@core/types/agent-runtime/AgentMessage.ts`:
- Around line 128-139: The SDKResultMessage interface is missing the
duration_api_ms property that is being consumed by
MessageConverter.extractApiDurationMs, creating a type-safety gap. Add an
optional duration_api_ms field with a number type to the SDKResultMessage
interface definition to properly declare this contract and eliminate unsafe
casting across files.
---
Nitpick comments:
In `@core/agent-runtime/test/MessageConverter.test.ts`:
- Around line 129-133: Add additional test cases to the MessageConverter test
suite to cover edge cases for invalid duration values. After the existing test
for non-numeric duration_api_ms in the 'should ignore non-numeric
duration_api_ms' test block, add separate test cases that verify
MessageConverter.extractApiDurationMs returns undefined for NaN, Infinity, and
negative numeric values passed as duration_api_ms. Each test should follow the
same assertion pattern as the existing test, passing the respective invalid
value and asserting the result is undefined, ensuring all invalid numeric and
non-numeric duration values are handled consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4da5cd6e-2e42-4e60-8fa7-7317c8c09d41
📒 Files selected for processing (11)
core/agent-runtime/src/AgentRuntime.tscore/agent-runtime/src/AgentStoreUtils.tscore/agent-runtime/src/MessageConverter.tscore/agent-runtime/src/OSSAgentStore.tscore/agent-runtime/src/RunBuilder.tscore/agent-runtime/test/MessageConverter.test.tscore/agent-runtime/test/OSSAgentStore.test.tscore/agent-runtime/test/RunBuilder.test.tscore/types/agent-runtime/AgentMessage.tscore/types/agent-runtime/AgentRuntime.tscore/types/agent-runtime/AgentStore.ts
jerryliang64
force-pushed
the
feat-agent-message-timing
branch
from
e98073b to
8eae36b
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/agent-runtime/test/OSSAgentStore.test.ts (1)
115-115:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix array-bracket spacing to unblock CI.
The linter requires spaces inside array brackets per the project's eslint configuration.
🔧 Proposed fix
- await store.appendMessages(thread.id, [original]); + await store.appendMessages(thread.id, [ original ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/agent-runtime/test/OSSAgentStore.test.ts` at line 115, The array bracket spacing in the store.appendMessages call does not conform to the project's eslint configuration which requires spaces inside array brackets. Modify the array parameter in the appendMessages method call to add spaces inside the brackets around the original element, changing [original] to [ original ].Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@core/agent-runtime/test/OSSAgentStore.test.ts`:
- Line 115: The array bracket spacing in the store.appendMessages call does not
conform to the project's eslint configuration which requires spaces inside array
brackets. Modify the array parameter in the appendMessages method call to add
spaces inside the brackets around the original element, changing [original] to [
original ].
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8a6d4ad-221b-4f7c-9d84-218e2be853d2
📒 Files selected for processing (11)
core/agent-runtime/src/AgentRuntime.tscore/agent-runtime/src/AgentStoreUtils.tscore/agent-runtime/src/MessageConverter.tscore/agent-runtime/src/OSSAgentStore.tscore/agent-runtime/src/RunBuilder.tscore/agent-runtime/test/MessageConverter.test.tscore/agent-runtime/test/OSSAgentStore.test.tscore/agent-runtime/test/RunBuilder.test.tscore/types/agent-runtime/AgentMessage.tscore/types/agent-runtime/AgentRuntime.tscore/types/agent-runtime/AgentStore.ts
✅ Files skipped from review due to trivial changes (1)
- core/types/agent-runtime/AgentRuntime.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- core/agent-runtime/src/AgentStoreUtils.ts
- core/agent-runtime/test/MessageConverter.test.ts
- core/agent-runtime/test/RunBuilder.test.ts
- core/agent-runtime/src/OSSAgentStore.ts
- core/agent-runtime/src/MessageConverter.ts
- core/types/agent-runtime/AgentMessage.ts
jerryliang64
force-pushed
the
feat-agent-message-timing
branch
from
8eae36b to
6ed3a1e
Compare
…pace Expose conversation timing the Claude Agent SDK does not provide, kept in a single additive `eggExt` namespace so it never collides with SDK message fields (the SDK `message` payload is untouched). - types: add MessageTiming / ToolTiming / EggExt; every AgentMessage member carries an optional `eggExt`. RunRecord/RunObject gain ms-granular timing (startedAtMs/completedAtMs/durationMs/apiDurationMs); legacy seconds fields retained for back-compat. - RunBuilder: record ms timing, derive durationMs = completedAtMs - startedAtMs (null when restored without startedAtMs); seconds fields derived from the same ms reading to avoid cross-second drift. - MessageConverter.extractApiDurationMs: sum result.duration_api_ms. - AgentRuntime: pass apiDurationMs into rb.complete at all three completion sites. - OSSAgentStore.appendMessages: stamp a server-side eggExt.persistedAtMs on each persisted message (shallow-copied; never overwrites executor-stamped timing). - tests: cover ms timing, extractApiDurationMs, persistedAtMs stamping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jerryliang64
force-pushed
the
feat-agent-message-timing
branch
from
6ed3a1e to
3a5fe13
Compare
2 participants
Why
Session data lacks timing: per-message latency, per-tool start/end/duration, and ms-granular run timing. The Claude Agent SDK only reports a whole-conversation total on the
resultmessage — individual assistant/user/tool_use messages carry no time at all. This PR adds the storage/contract-layer fields to carry that timing (the actual stamping happens executor-side).What
All framework extension fields live under a single top-level
eggExtnamespace so they never collide with Claude Agent SDK message fields (the SDKmessagepayload is untouched). Time-field naming: absolute epoch-ms points end inAtMs, durations/intervals inMs; existing seconds-level run fields are kept for back-compat.MessageTiming/ToolTiming/EggExt; everyAgentMessagemember carries an optionaleggExt.RunRecord/RunObjectgain ms timing (startedAtMs/completedAtMs/durationMs/apiDurationMs).durationMs(null when restored withoutstartedAtMs); seconds fields are derived from the same ms reading to avoid cross-second drift.result.duration_api_ms.eggExt.persistedAtMson each persisted message (shallow-copied; never overwrites executor-stampedtiming).Compatibility
All new fields are optional; seconds-level run fields unchanged; old consumers ignore
eggExt.getThreadstill returns only user/assistant by default, and theeggExt(timing/toolTimings) on those is visible alongside them.Test
New coverage for ms timing,
extractApiDurationMs, andpersistedAtMsstamping; full agent-runtime suite green (200 passing).🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
apiDurationMs, now captured and exposed in run snapshots.persistedAtMsat append time (without mutating the original inputs). Message shapes now support optional timing metadata.Tests
apiDurationMsextraction, message stamping (including no input mutation), and RunBuilder millisecond timing restoration/snapshotting.