Store timing in metadata #282409
Store timing in metadata #282409
Conversation
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
|
There was a problem hiding this comment.
Pull request overview
This PR ports a fix from issue #281758 by adding a timing field to chat session metadata. The change centralizes timing information (start and end times) in the metadata rather than computing it on-demand, which improves consistency and simplifies code in the sessions provider.
Key Changes
- Introduced
IChatSessionTiminginterface withstartTimeand optionalendTimefields - Added
timingproperty toIChatModelinterface and implemented a getter inChatModelthat computes timing from the session's timestamp and last response - Added
timingtoIChatDetail(required) andIChatSessionEntryMetadata(optional for backward compatibility)
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/common/chatService.ts | Added IChatSessionTiming interface and timing field to IChatDetail |
| src/vs/workbench/contrib/chat/common/chatModel.ts | Added timing property to IChatModel interface and implemented getter that computes timing from timestamp and last response |
| src/vs/workbench/contrib/chat/common/chatSessionStore.ts | Added optional timing field to IChatSessionEntryMetadata and updated getSessionMetadata to populate timing from ChatModel or legacy data |
| src/vs/workbench/contrib/chat/common/chatServiceImpl.ts | Added fallback logic to provide default timing for old data that doesn't have timing stored |
| src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsProvider.ts | Simplified by using timing from metadata instead of computing it locally |
| src/vs/workbench/contrib/chat/browser/actions/chatActions.ts | Added timing field when creating mock chat details |
| src/vs/workbench/contrib/chat/test/common/mockChatModel.ts | Added timing property to MockChatModel |
| src/vs/workbench/contrib/chat/test/browser/localAgentSessionsProvider.test.ts | Updated all test cases to include timing field in IChatDetail objects |
| isActive: true, | ||
| lastResponseState: ResponseModelState.Complete | ||
| lastResponseState: ResponseModelState.Complete, | ||
| timing: { startTime: 0, endTime: 1 }, |
There was a problem hiding this comment.
[nitpick] Inconsistent trailing comma: The timing field has a trailing comma here, but most other test cases in this file don't have one (e.g., lines 325, 349, 375, 383). For consistency, either remove the trailing comma here or add it to all other instances.
| timing: { startTime: 0, endTime: 1 }, | |
| timing: { startTime: 0, endTime: 1 } |
| isActive: true, | ||
| lastResponseState: ResponseModelState.Complete | ||
| lastResponseState: ResponseModelState.Complete, | ||
| timing: { startTime: 0, endTime: 1 }, |
There was a problem hiding this comment.
[nitpick] Inconsistent trailing comma: The timing field has a trailing comma here, but most other test cases in this file don't have one (e.g., lines 325, 349, 375, 383). For consistency, either remove the trailing comma here or add it to all other instances.
| timing: { startTime: 0, endTime: 1 }, | |
| timing: { startTime: 0, endTime: 1 } |
| isActive: true, | ||
| lastResponseState: ResponseModelState.Complete | ||
| lastResponseState: ResponseModelState.Complete, | ||
| timing: { startTime: 0, endTime: 1 }, |
There was a problem hiding this comment.
[nitpick] Inconsistent trailing comma: The timing field has a trailing comma here, but most other test cases in this file don't have one (e.g., lines 325, 349, 375, 383). For consistency, either remove the trailing comma here or add it to all other instances.
| timing: { startTime: 0, endTime: 1 }, | |
| timing: { startTime: 0, endTime: 1 } |
Port fix for #281758 to main
Sorry for pinging the entire team because I ended up with the wrong base branch somehow