opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor(coderd/x/chatd): insert chats directly as pending on creation (#23888)

Browse Source 
Previously, `CreateChat` inserted the `chats` row with the DB default
status (`waiting`), then updated it to `pending` in the same transaction
via `setChatPendingWithStore`. This wasted two extra queries per chat
creation (`GetChatByID` + `UpdateChatStatus`) and rewrote the same row
immediately after inserting it.

Now `CreateChat` passes the status directly to `InsertChat`, so the row
is written once in its final create-time state. The
`setChatPendingWithStore` helper is removed entirely. `InsertChat` now
requires an explicit `status` parameter at all callsites instead of
relying on a DB column default.

## Motivation

On an experimental branch we're trialing firing all chatd notifications
from plpgsql triggers. The old two-step insert made that awkward: in an
`AFTER INSERT` trigger, `NEW` only contained the insert-time row
(`waiting`), not the final committed state (`pending`). To emit the
correct event payload the trigger had to be deferred and re-read the row
from `chats` at commit time.

With this change, `NEW` already contains the correct row to publish — no
deferred trigger, no extra `SELECT`, simpler and cheaper trigger logic.

That said, this seems like a worthwhile change regardless of the trigger
experiment: writing the final row state once removes unnecessary DB work
on every chat creation and makes the create path easier to reason about.
This commit is contained in:
Ethan
2026-04-02 14:13:51 +11:00
committed by GitHub
parent fc1e0beb3b
commit 7757cd8e08
12 changed files with 78 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/x/chatd/chatd.go
+5 -32
View File
@@ -851,7 +851,10 @@ func (p *Server) CreateChat(ctx context.Context, opts CreateOptions) (database.C
LastModelConfigID: opts.ModelConfigID,
Title: opts.Title,
Mode: opts.ChatMode,
MCPServerIDs: opts.MCPServerIDs,
// Chats created with an initial user message start pending.
// Waiting is reserved for idle chats with no pending work.
Status: database.ChatStatusPending,
MCPServerIDs: opts.MCPServerIDs,
Labels: pqtype.NullRawMessage{
RawMessage: labelsJSON,
Valid: true,
@@ -920,10 +923,7 @@ func (p *Server) CreateChat(ctx context.Context, opts CreateOptions) (database.C
return xerrors.Errorf("insert initial chat messages: %w", err)
}
chat, err = setChatPendingWithStore(ctx, tx, insertedChat.ID)
if err != nil {
return xerrors.Errorf("set chat pending: %w", err)
}
chat = insertedChat
if !chat.RootChatID.Valid && !chat.ParentChatID.Valid {
chat.RootChatID = uuid.NullUUID{UUID: chat.ID, Valid: true}
@@ -1997,33 +1997,6 @@ func (p *Server) RefreshStatus(ctx context.Context, chatID uuid.UUID) error {
return nil
}
func setChatPendingWithStore(
ctx context.Context,
store database.Store,
chatID uuid.UUID,
) (database.Chat, error) {
chat, err := store.GetChatByID(ctx, chatID)
if err != nil {
return database.Chat{}, xerrors.Errorf("get chat: %w", err)
}
if chat.Status == database.ChatStatusPending {
return chat, nil
}
updatedChat, err := store.UpdateChatStatus(ctx, database.UpdateChatStatusParams{
ID: chat.ID,
Status: database.ChatStatusPending,
WorkerID: uuid.NullUUID{},
StartedAt: sql.NullTime{},
HeartbeatAt: sql.NullTime{},
LastError: sql.NullString{},
})
if err != nil {
return database.Chat{}, xerrors.Errorf("set chat pending: %w", err)
}
return updatedChat, nil
}
func (p *Server) setChatWaiting(ctx context.Context, chatID uuid.UUID) (database.Chat, error) {
var updatedChat database.Chat
err := p.db.InTx(func(tx database.Store) error {
coderd/x/chatd/chatd_test.go
+7
View File
@@ -908,6 +908,7 @@ func TestCreateChatRejectsWhenUsageLimitReached(t *testing.T) {
require.NoError(t, err)
existingChat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
Title: "existing-limit-chat",
LastModelConfigID: model.ID,
@@ -1198,6 +1199,7 @@ func TestInterruptAutoPromotionIgnoresLaterUsageLimitIncrease(t *testing.T) {
require.NotNil(t, laterQueuedResult.QueuedMessage)
spendChat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{},
ParentChatID: uuid.NullUUID{},
@@ -1448,6 +1450,7 @@ func TestRecoverStaleChatsPeriodically(t *testing.T) {
// to running with a heartbeat in the past.
deadWorkerID := uuid.New()
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
Title: "stale-recovery-periodic",
LastModelConfigID: model.ID,
@@ -1493,6 +1496,7 @@ func TestRecoverStaleChatsPeriodically(t *testing.T) {
// This tests the periodic recovery, not just the startup one.
deadWorkerID2 := uuid.New()
chat2, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
Title: "stale-recovery-periodic-2",
LastModelConfigID: model.ID,
@@ -1531,6 +1535,7 @@ func TestNewReplicaRecoversStaleChatFromDeadReplica(t *testing.T) {
// heartbeat (well beyond the stale threshold).
deadReplicaID := uuid.New()
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
Title: "orphaned-chat",
LastModelConfigID: model.ID,
@@ -1573,6 +1578,7 @@ func TestWaitingChatsAreNotRecoveredAsStale(t *testing.T) {
// Create a chat in waiting status — this should NOT be touched
// by stale recovery.
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
Title: "waiting-chat",
LastModelConfigID: model.ID,
@@ -1615,6 +1621,7 @@ func TestUpdateChatStatusPersistsLastError(t *testing.T) {
user, model := seedChatDependencies(ctx, t, db)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
Title: "error-persisted",
LastModelConfigID: model.ID,
coderd/x/chatd/chatprompt/chatprompt_test.go
+2
View File
@@ -1482,6 +1482,7 @@ func TestNulEscapeRoundTrip(t *testing.T) {
require.NoError(t, err)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
LastModelConfigID: model.ID,
Title: "nul-roundtrip-test",
@@ -1978,6 +1979,7 @@ func TestMediaToolResultRoundTrip(t *testing.T) {
t.Helper()
chat, chatErr := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
LastModelConfigID: model.ID,
Title: "media-roundtrip-" + callID,
coderd/x/chatd/chattool/startworkspace_test.go
+7
View File
@@ -35,6 +35,7 @@ func TestStartWorkspace(t *testing.T) {
modelCfg := seedModelConfig(ctx, t, db, user.ID)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
LastModelConfigID: modelCfg.ID,
Title: "test-no-workspace",
@@ -77,6 +78,7 @@ func TestStartWorkspace(t *testing.T) {
ws := wsResp.Workspace
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
LastModelConfigID: modelCfg.ID,
@@ -155,6 +157,7 @@ func TestStartWorkspace(t *testing.T) {
require.NotEqual(t, uuid.Nil, preferredAgentID)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
LastModelConfigID: modelCfg.ID,
@@ -214,6 +217,7 @@ func TestStartWorkspace(t *testing.T) {
ws := wsResp.Workspace
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
LastModelConfigID: modelCfg.ID,
@@ -276,6 +280,7 @@ func TestStartWorkspace(t *testing.T) {
ws := wsResp.Workspace
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
LastModelConfigID: modelCfg.ID,
@@ -332,6 +337,7 @@ func TestStartWorkspace(t *testing.T) {
ws := wsResp.Workspace
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
LastModelConfigID: modelCfg.ID,
@@ -400,6 +406,7 @@ func TestStartWorkspace(t *testing.T) {
ws := wsResp.Workspace
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
LastModelConfigID: modelCfg.ID,
coderd/x/gitsync/worker_test.go
+1
View File
@@ -977,6 +977,7 @@ func TestWorker(t *testing.T) {
require.NoError(t, err)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
Status: database.ChatStatusWaiting,
OwnerID: user.ID,
LastModelConfigID: modelCfg.ID,
Title: "integration-test",