mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
385146000b
Records reasoning start and end times on persisted reasoning `ChatMessagePart`s so reasoning duration can be computed for stored chats. Backend-only: no SSE changes and no frontend rendering ship in this PR. The `created_at` field on `ChatMessagePart` is extended to also be present on `reasoning` parts (it previously appeared only on `tool-call` and `tool-result`), and a new `completed_at` field is added for `reasoning` parts. ### How timestamps are recorded - `StreamPartTypeReasoningStart`: stamp `startedAt = dbtime.Now()` on the active reasoning state. - `StreamPartTypeReasoningEnd`: stamp `completedAt = dbtime.Now()` and append both into parallel `[]time.Time` slices on `stepResult`. - Persistence reads the slices in occurrence order (reasoning has no provider-side ID) and applies them to the matching `ChatMessagePart` via `buildAssistantPartsForPersist`. The first reasoning block's stamps go onto the first reasoning part, and so on. - `flushActiveState` flushes partial reasoning interrupted before `StreamPartTypeReasoningEnd` with `startedAt` from the active state and `completedAt = dbtime.Now()` at the interruption. ### Why two fields, not one? Tool calls and results are point events. The frontend computes their duration by subtracting the call's `created_at` from the result's `created_at`. Reasoning is one assistant part that brackets a span, so we record both endpoints on the part itself. ### Why not stamp in `PartFromContent`? Same rationale as #24101: `PartFromContent` is called during both SSE publishing and persistence. Stamping there would yield incorrect persistence-time timestamps for reasoning blocks that finished much earlier in the step. Instead we capture in the chatloop and apply during persistence. <details><summary>Implementation plan</summary> - `codersdk/chats.go`: extend `CreatedAt`'s `variants` to include `reasoning?`; add `CompletedAt *time.Time` with `variants:"reasoning?"`. - `coderd/x/chatd/chatloop/chatloop.go`: extend `reasoningState` with `startedAt`; extend `stepResult` and `PersistedStep` with parallel `[]time.Time` reasoning slices; stamp on `ReasoningStart`/`ReasoningEnd`; thread the slices through all `PersistStep` call sites including the interrupt-safe path; record partial reasoning in `flushActiveState`. - `coderd/x/chatd/attachments.go`: walk reasoning parts in occurrence order and apply `step.ReasoningStartedAt[i]` to `part.CreatedAt` and `step.ReasoningCompletedAt[i]` to `part.CompletedAt`. ### Tests - `codersdk/chats_test.go` round-trips `created_at` + `completed_at` on reasoning parts and verifies omission when absent and partial interrupted parts. - `coderd/x/chatd/chatprompt/chatprompt_test.go` asserts `PartFromContent(ReasoningContent{})` does NOT stamp timestamps. - `coderd/x/chatd/chatloop/chatloop_test.go` `TestRun_ReasoningTimestamps` drives a stream with two reasoning blocks and verifies parallel slices, monotonicity, ordering, non-zero values, and content-block ordering. `TestRun_InterruptedReasoningFlushesTimestamps` cancels mid-reasoning and verifies `flushActiveState` records a non-zero pair. - `coderd/x/chatd/attachments_test.go` covers `buildAssistantPartsForPersist` for normal interleaved reasoning, partial (zero `completed_at`), and missing slices. </details> > Generated by Coder Agents. Co-authored-by: Coder Agent <agent@coder.com>
248 lines
7.8 KiB
Go
248 lines
7.8 KiB
Go
package chatd //nolint:testpackage
|
|
|
|
import (
|
|
"context"
|
|
"testing"
|
|
"time"
|
|
|
|
"charm.land/fantasy"
|
|
"github.com/google/uuid"
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/coder/coder/v2/coderd/x/chatd/chatloop"
|
|
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
|
|
"github.com/coder/coder/v2/codersdk"
|
|
"github.com/coder/coder/v2/testutil"
|
|
)
|
|
|
|
func TestBuildAssistantPartsForPersist_PromotesToolAttachments(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
fileID := uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
|
|
response := chattool.WithAttachments(
|
|
fantasy.NewTextResponse(`{"ok":true}`),
|
|
chattool.AttachmentMetadata{
|
|
FileID: fileID,
|
|
MediaType: "image/png",
|
|
Name: "screenshot.png",
|
|
},
|
|
)
|
|
toolCallAt := time.Date(2026, time.April, 10, 0, 0, 0, 0, time.UTC)
|
|
|
|
parts := buildAssistantPartsForPersist(
|
|
context.Background(),
|
|
testutil.Logger(t),
|
|
[]fantasy.Content{fantasy.TextContent{Text: "Here is the screenshot."}},
|
|
[]fantasy.ToolResultContent{{
|
|
ToolCallID: "call-1",
|
|
ToolName: "computer",
|
|
ClientMetadata: response.Metadata,
|
|
ProviderExecuted: false,
|
|
}},
|
|
chatloop.PersistedStep{
|
|
ToolCallCreatedAt: map[string]time.Time{
|
|
"call-1": toolCallAt,
|
|
},
|
|
},
|
|
nil,
|
|
)
|
|
|
|
require.Len(t, parts, 2)
|
|
require.Equal(t, codersdk.ChatMessagePartTypeText, parts[0].Type)
|
|
require.Equal(t, "Here is the screenshot.", parts[0].Text)
|
|
require.Equal(t, codersdk.ChatMessagePartTypeFile, parts[1].Type)
|
|
require.True(t, parts[1].FileID.Valid)
|
|
require.Equal(t, fileID, parts[1].FileID.UUID)
|
|
require.Equal(t, "image/png", parts[1].MediaType)
|
|
require.Equal(t, "screenshot.png", parts[1].Name)
|
|
}
|
|
|
|
func TestBuildAssistantPartsForPersist_PromotesProposePlanAttachment(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
fileID := uuid.MustParse("bbbbbbbb-cccc-dddd-eeee-ffffffffffff")
|
|
response := chattool.WithAttachments(
|
|
fantasy.NewTextResponse(`{"ok":true,"kind":"plan"}`),
|
|
chattool.AttachmentMetadata{
|
|
FileID: fileID,
|
|
MediaType: "text/markdown",
|
|
Name: "PLAN.md",
|
|
},
|
|
)
|
|
|
|
parts := buildAssistantPartsForPersist(
|
|
context.Background(),
|
|
testutil.Logger(t),
|
|
[]fantasy.Content{fantasy.TextContent{Text: "Here is the proposed plan."}},
|
|
[]fantasy.ToolResultContent{{
|
|
ToolCallID: "call-plan",
|
|
ToolName: "propose_plan",
|
|
ClientMetadata: response.Metadata,
|
|
}},
|
|
chatloop.PersistedStep{},
|
|
nil,
|
|
)
|
|
|
|
require.Len(t, parts, 2)
|
|
require.Equal(t, codersdk.ChatMessagePartTypeText, parts[0].Type)
|
|
require.Equal(t, "Here is the proposed plan.", parts[0].Text)
|
|
require.Equal(t, codersdk.ChatMessagePartTypeFile, parts[1].Type)
|
|
require.True(t, parts[1].FileID.Valid)
|
|
require.Equal(t, fileID, parts[1].FileID.UUID)
|
|
require.Equal(t, "text/markdown", parts[1].MediaType)
|
|
require.Equal(t, "PLAN.md", parts[1].Name)
|
|
}
|
|
|
|
func TestBuildAssistantPartsForPersist_InvalidAttachmentMetadataSkipsOnlyBrokenResult(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
goodFileID := uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
|
|
goodResponse := chattool.WithAttachments(
|
|
fantasy.NewTextResponse(`{"ok":true}`),
|
|
chattool.AttachmentMetadata{
|
|
FileID: goodFileID,
|
|
MediaType: "image/png",
|
|
Name: "good.png",
|
|
},
|
|
)
|
|
|
|
parts := buildAssistantPartsForPersist(
|
|
context.Background(),
|
|
testutil.Logger(t),
|
|
[]fantasy.Content{fantasy.TextContent{Text: "Here are the results."}},
|
|
[]fantasy.ToolResultContent{
|
|
{
|
|
ToolCallID: "call-good",
|
|
ToolName: "computer",
|
|
ClientMetadata: goodResponse.Metadata,
|
|
},
|
|
{
|
|
ToolCallID: "call-bad",
|
|
ToolName: "attach_file",
|
|
ClientMetadata: `{"attachments":[{"file_id":"aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"}]}`,
|
|
},
|
|
},
|
|
chatloop.PersistedStep{},
|
|
nil,
|
|
)
|
|
|
|
require.Len(t, parts, 2)
|
|
require.Equal(t, codersdk.ChatMessagePartTypeText, parts[0].Type)
|
|
require.Equal(t, codersdk.ChatMessagePartTypeFile, parts[1].Type)
|
|
require.True(t, parts[1].FileID.Valid)
|
|
require.Equal(t, goodFileID, parts[1].FileID.UUID)
|
|
require.Equal(t, "image/png", parts[1].MediaType)
|
|
require.Equal(t, "good.png", parts[1].Name)
|
|
}
|
|
|
|
func TestBuildAssistantPartsForPersist_AppliesReasoningTimestamps(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
startedAt1 := time.Date(2026, time.April, 10, 12, 0, 0, 0, time.UTC)
|
|
completedAt1 := startedAt1.Add(500 * time.Millisecond)
|
|
startedAt2 := completedAt1.Add(time.Second)
|
|
completedAt2 := startedAt2.Add(750 * time.Millisecond)
|
|
|
|
// Interleave reasoning blocks with a text block to confirm the
|
|
// index walks reasoning content in occurrence order without
|
|
// being thrown off by non-reasoning entries.
|
|
parts := buildAssistantPartsForPersist(
|
|
context.Background(),
|
|
testutil.Logger(t),
|
|
[]fantasy.Content{
|
|
fantasy.ReasoningContent{Text: "first thought"},
|
|
fantasy.TextContent{Text: "intermission"},
|
|
fantasy.ReasoningContent{Text: "second thought"},
|
|
},
|
|
nil,
|
|
chatloop.PersistedStep{
|
|
ReasoningStartedAt: []time.Time{startedAt1, startedAt2},
|
|
ReasoningCompletedAt: []time.Time{completedAt1, completedAt2},
|
|
},
|
|
nil,
|
|
)
|
|
|
|
require.Len(t, parts, 3)
|
|
|
|
require.Equal(t, codersdk.ChatMessagePartTypeReasoning, parts[0].Type)
|
|
require.Equal(t, "first thought", parts[0].Text)
|
|
require.NotNil(t, parts[0].CreatedAt)
|
|
require.True(t, parts[0].CreatedAt.Equal(startedAt1),
|
|
"first reasoning part must use ReasoningStartedAt[0]")
|
|
require.NotNil(t, parts[0].CompletedAt)
|
|
require.True(t, parts[0].CompletedAt.Equal(completedAt1),
|
|
"first reasoning part must use ReasoningCompletedAt[0]")
|
|
|
|
require.Equal(t, codersdk.ChatMessagePartTypeText, parts[1].Type)
|
|
require.Nil(t, parts[1].CreatedAt,
|
|
"text part must not inherit reasoning timestamps")
|
|
require.Nil(t, parts[1].CompletedAt)
|
|
|
|
require.Equal(t, codersdk.ChatMessagePartTypeReasoning, parts[2].Type)
|
|
require.Equal(t, "second thought", parts[2].Text)
|
|
require.NotNil(t, parts[2].CreatedAt)
|
|
require.True(t, parts[2].CreatedAt.Equal(startedAt2),
|
|
"second reasoning part must use ReasoningStartedAt[1]")
|
|
require.NotNil(t, parts[2].CompletedAt)
|
|
require.True(t, parts[2].CompletedAt.Equal(completedAt2),
|
|
"second reasoning part must use ReasoningCompletedAt[1]")
|
|
}
|
|
|
|
func TestBuildAssistantPartsForPersist_PartialReasoningTimestamps(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
startedAt := time.Date(2026, time.April, 10, 12, 0, 0, 0, time.UTC)
|
|
|
|
// Tests the persistence helper when the parallel CompletedAt
|
|
// slot is zero-valued, ensuring it leaves CompletedAt nil rather
|
|
// than setting it to the Go zero time. No production code path
|
|
// currently emits a zero CompletedAt alongside a non-zero
|
|
// StartedAt (flushActiveState always stamps both with
|
|
// dbtime.Now()), so this is a defensive boundary test for the
|
|
// `variants:"reasoning?"` contract.
|
|
parts := buildAssistantPartsForPersist(
|
|
context.Background(),
|
|
testutil.Logger(t),
|
|
[]fantasy.Content{
|
|
fantasy.ReasoningContent{Text: "incomplete thought"},
|
|
},
|
|
nil,
|
|
chatloop.PersistedStep{
|
|
ReasoningStartedAt: []time.Time{startedAt},
|
|
ReasoningCompletedAt: []time.Time{{}},
|
|
},
|
|
nil,
|
|
)
|
|
|
|
require.Len(t, parts, 1)
|
|
require.Equal(t, codersdk.ChatMessagePartTypeReasoning, parts[0].Type)
|
|
require.NotNil(t, parts[0].CreatedAt)
|
|
require.True(t, parts[0].CreatedAt.Equal(startedAt))
|
|
require.Nil(t, parts[0].CompletedAt,
|
|
"zero-valued ReasoningCompletedAt must not produce a stamp")
|
|
}
|
|
|
|
func TestBuildAssistantPartsForPersist_MissingReasoningTimestamps(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// Legacy persisted steps and steps that never observed a
|
|
// reasoning block carry empty timestamp slices. The helper must
|
|
// leave CreatedAt and CompletedAt nil instead of panicking on
|
|
// the out-of-range index.
|
|
parts := buildAssistantPartsForPersist(
|
|
context.Background(),
|
|
testutil.Logger(t),
|
|
[]fantasy.Content{
|
|
fantasy.ReasoningContent{Text: "no timestamps recorded"},
|
|
},
|
|
nil,
|
|
chatloop.PersistedStep{},
|
|
nil,
|
|
)
|
|
|
|
require.Len(t, parts, 1)
|
|
require.Equal(t, codersdk.ChatMessagePartTypeReasoning, parts[0].Type)
|
|
require.Nil(t, parts[0].CreatedAt)
|
|
require.Nil(t, parts[0].CompletedAt)
|
|
}
|