From 89034f6422c3a2f66a1021da6cf8797d989444ed Mon Sep 17 00:00:00 2001 From: Stephen Kirby <58410745+stirby@users.noreply.github.com> Date: Thu, 7 May 2026 12:09:11 -0500 Subject: [PATCH] test(coderd/database): cover step message ID boundaries (#24690) Closes #24091 Adds `TestDeleteChatDebugDataAfterMessageIDStepLevelFieldBoundariesAndNulls`, which complements the existing triggered-runs test for `DeleteChatDebugDataAfterMessageID` with boundary and NULL coverage for step-level message IDs. The existing `TestDeleteChatDebugDataAfterMessageIDIncludesTriggeredRuns` already exercises the `step.assistant_message_id > @message_id` deletion path. This test focuses on: - Strict greater-than behavior at the cutoff for assistant and history-tip step message IDs. - Step-level assistant and history-tip message ID combinations. - SQL NULL behavior for step-level message IDs. - A mixed-step run where one matching step deletes the whole run and cascades every step. | Scenario | assistant_message_id | history_tip_message_id | Expected | |----------|----------------------|------------------------|----------| | Assistant above cutoff, history tip NULL | cutoff + 5 | NULL | Deleted | | Assistant above cutoff, history tip below cutoff | cutoff + 20 | cutoff - 3 | Deleted | | Assistant below cutoff, history tip NULL | cutoff - 3 | NULL | Preserved | | Assistant at cutoff boundary, history tip NULL | cutoff | NULL | Preserved | | Assistant NULL, history tip above cutoff | NULL | cutoff + 2 | Deleted | | Assistant NULL, history tip at cutoff boundary | NULL | cutoff | Preserved | | Both step message IDs NULL | NULL | NULL | Preserved | > Generated by Coder Agents
Review notes - Run-level message IDs are below the cutoff to isolate step-level selection. - The assistant-above-cutoff scenario includes a second nonmatching step to cover mixed-step deletion. - The test uses unique model and chat names for isolation. - `go test -v ./coderd/database -run TestDeleteChatDebugDataAfterMessageID -count=1` passes.
--- coderd/database/querier_test.go | 263 ++++++++++++++++++++++++++++++++ 1 file changed, 263 insertions(+) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 7a1a503534..f58f428a51 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -11741,6 +11741,269 @@ func TestDeleteChatDebugDataAfterMessageIDIncludesTriggeredRuns(t *testing.T) { require.Equal(t, unaffectedStep.ID, remainingSteps[0].ID) } +// TestDeleteChatDebugDataAfterMessageIDStepLevelFieldBoundariesAndNulls +// verifies that DeleteChatDebugDataAfterMessageID handles step-level +// field boundaries and NULL combinations when run-level message IDs are +// below the cutoff. This complements the triggered-runs test with extra +// coverage for strict step-level comparisons and SQL NULL behavior. +func TestDeleteChatDebugDataAfterMessageIDStepLevelFieldBoundariesAndNulls(t *testing.T) { + t.Parallel() + + store, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitMedium) + + org := dbgen.Organization(t, store, database.Organization{}) + user := dbgen.User(t, store, database.User{}) + + providerName := "openai" + modelName := "debug-model-step-boundaries-" + uuid.NewString() + + _, err := store.InsertChatProvider(ctx, database.InsertChatProviderParams{ + Provider: providerName, + DisplayName: "Debug Provider", + APIKey: "test-key", + Enabled: true, + CentralApiKeyEnabled: true, + }) + require.NoError(t, err) + + modelCfg, err := store.InsertChatModelConfig(ctx, database.InsertChatModelConfigParams{ + Provider: providerName, + Model: modelName, + DisplayName: "Debug Model", + CreatedBy: uuid.NullUUID{UUID: user.ID, Valid: true}, + UpdatedBy: uuid.NullUUID{UUID: user.ID, Valid: true}, + Enabled: true, + IsDefault: true, + ContextLimit: 128000, + CompressionThreshold: 80, + Options: json.RawMessage(`{}`), + }) + require.NoError(t, err) + + chat, err := store.InsertChat(ctx, database.InsertChatParams{ + OrganizationID: org.ID, + Status: database.ChatStatusWaiting, + ClientType: database.ChatClientTypeUi, + OwnerID: user.ID, + LastModelConfigID: modelCfg.ID, + Title: "chat-debug-step-boundaries-" + uuid.NewString(), + }) + require.NoError(t, err) + + const cutoff int64 = 100 + + // insertRunBelowRunLevelCutoff creates a run whose run-level message + // IDs cannot match the deletion query. The step-level fields decide + // whether the run is deleted. + insertRunBelowRunLevelCutoff := func(t *testing.T) database.ChatDebugRun { + t.Helper() + run, runErr := store.InsertChatDebugRun(ctx, database.InsertChatDebugRunParams{ + ChatID: chat.ID, + ModelConfigID: uuid.NullUUID{UUID: modelCfg.ID, Valid: true}, + TriggerMessageID: sql.NullInt64{Int64: cutoff - 10, Valid: true}, + HistoryTipMessageID: sql.NullInt64{Int64: cutoff - 10, Valid: true}, + Kind: "chat_turn", + Status: "in_progress", + Provider: sql.NullString{String: providerName, Valid: true}, + Model: sql.NullString{String: modelName, Valid: true}, + }) + require.NoError(t, runErr) + return run + } + + // assistantAboveWithNullHistoryTipRun is deleted only through the + // step.assistant_message_id clause. + assistantAboveWithNullHistoryTipRun := insertRunBelowRunLevelCutoff(t) + _, err = store.InsertChatDebugStep(ctx, database.InsertChatDebugStepParams{ + RunID: assistantAboveWithNullHistoryTipRun.ID, + ChatID: chat.ID, + StepNumber: 1, + Operation: "stream", + Status: "completed", + AssistantMessageID: sql.NullInt64{Int64: cutoff + 5, Valid: true}, + // HistoryTipMessageID intentionally omitted (NULL). + }) + require.NoError(t, err) + + // Add a nonmatching step to verify that one matching step is enough + // to delete the run and cascade all of its steps. + _, err = store.InsertChatDebugStep(ctx, database.InsertChatDebugStepParams{ + RunID: assistantAboveWithNullHistoryTipRun.ID, + ChatID: chat.ID, + StepNumber: 2, + Operation: "stream", + Status: "completed", + AssistantMessageID: sql.NullInt64{Int64: cutoff - 5, Valid: true}, + // HistoryTipMessageID intentionally omitted (NULL). + }) + require.NoError(t, err) + + // assistantAboveWithHistoryTipBelowRun is deleted through the + // step.assistant_message_id clause while the step history tip stays + // below the cutoff. + assistantAboveWithHistoryTipBelowRun := insertRunBelowRunLevelCutoff(t) + _, err = store.InsertChatDebugStep(ctx, database.InsertChatDebugStepParams{ + RunID: assistantAboveWithHistoryTipBelowRun.ID, + ChatID: chat.ID, + StepNumber: 1, + Operation: "stream", + Status: "completed", + AssistantMessageID: sql.NullInt64{Int64: cutoff + 20, Valid: true}, + HistoryTipMessageID: sql.NullInt64{Int64: cutoff - 3, Valid: true}, + }) + require.NoError(t, err) + + // assistantBelowWithNullHistoryTipRun survives because its step + // assistant_message_id is below the cutoff and step history tip is + // NULL. + assistantBelowWithNullHistoryTipRun := insertRunBelowRunLevelCutoff(t) + assistantBelowWithNullHistoryTipStep, err := store.InsertChatDebugStep(ctx, database.InsertChatDebugStepParams{ + RunID: assistantBelowWithNullHistoryTipRun.ID, + ChatID: chat.ID, + StepNumber: 1, + Operation: "stream", + Status: "completed", + AssistantMessageID: sql.NullInt64{Int64: cutoff - 3, Valid: true}, + }) + require.NoError(t, err) + + // assistantAtBoundaryWithNullHistoryTipRun survives because the + // query uses strict greater-than, not greater-than-or-equal. + assistantAtBoundaryWithNullHistoryTipRun := insertRunBelowRunLevelCutoff(t) + assistantAtBoundaryWithNullHistoryTipStep, err := store.InsertChatDebugStep(ctx, database.InsertChatDebugStepParams{ + RunID: assistantAtBoundaryWithNullHistoryTipRun.ID, + ChatID: chat.ID, + StepNumber: 1, + Operation: "stream", + Status: "completed", + AssistantMessageID: sql.NullInt64{Int64: cutoff, Valid: true}, + }) + require.NoError(t, err) + + // historyTipAboveWithNullAssistantRun is deleted through the + // step.history_tip_message_id clause while assistant_message_id is + // NULL. + historyTipAboveWithNullAssistantRun := insertRunBelowRunLevelCutoff(t) + _, err = store.InsertChatDebugStep(ctx, database.InsertChatDebugStepParams{ + RunID: historyTipAboveWithNullAssistantRun.ID, + ChatID: chat.ID, + StepNumber: 1, + Operation: "stream", + Status: "completed", + HistoryTipMessageID: sql.NullInt64{Int64: cutoff + 2, Valid: true}, + // AssistantMessageID intentionally omitted (NULL). + }) + require.NoError(t, err) + + // historyTipAtBoundaryWithNullAssistantRun survives because the + // step history tip uses strict greater-than, not greater-than-or-equal. + historyTipAtBoundaryWithNullAssistantRun := insertRunBelowRunLevelCutoff(t) + historyTipAtBoundaryWithNullAssistantStep, err := store.InsertChatDebugStep(ctx, database.InsertChatDebugStepParams{ + RunID: historyTipAtBoundaryWithNullAssistantRun.ID, + ChatID: chat.ID, + StepNumber: 1, + Operation: "stream", + Status: "completed", + HistoryTipMessageID: sql.NullInt64{Int64: cutoff, Valid: true}, + // AssistantMessageID intentionally omitted (NULL). + }) + require.NoError(t, err) + + // bothStepMessageIDsNullRun survives because NULL > N evaluates to + // NULL, not TRUE, in SQL. + bothStepMessageIDsNullRun := insertRunBelowRunLevelCutoff(t) + bothStepMessageIDsNullStep, err := store.InsertChatDebugStep(ctx, database.InsertChatDebugStepParams{ + RunID: bothStepMessageIDsNullRun.ID, + ChatID: chat.ID, + StepNumber: 1, + Operation: "stream", + Status: "completed", + // Both message IDs intentionally omitted (NULL). + }) + require.NoError(t, err) + + deletedRows, err := store.DeleteChatDebugDataAfterMessageID(ctx, database.DeleteChatDebugDataAfterMessageIDParams{ + ChatID: chat.ID, + MessageID: cutoff, + StartedBefore: time.Now().Add(time.Minute), + }) + require.NoError(t, err) + require.EqualValues(t, 3, deletedRows) + + _, err = store.GetChatDebugRunByID(ctx, assistantAboveWithNullHistoryTipRun.ID) + require.ErrorIs(t, err, sql.ErrNoRows, + "assistant above cutoff with NULL history tip must be deleted") + + _, err = store.GetChatDebugRunByID(ctx, assistantAboveWithHistoryTipBelowRun.ID) + require.ErrorIs(t, err, sql.ErrNoRows, + "assistant above cutoff with history tip below cutoff must be deleted") + + _, err = store.GetChatDebugRunByID(ctx, historyTipAboveWithNullAssistantRun.ID) + require.ErrorIs(t, err, sql.ErrNoRows, + "NULL assistant with history tip above cutoff must be deleted") + + for _, deletedRun := range []struct { + name string + id uuid.UUID + }{ + {name: "assistant above cutoff with NULL history tip", id: assistantAboveWithNullHistoryTipRun.ID}, + {name: "assistant above cutoff with history tip below cutoff", id: assistantAboveWithHistoryTipBelowRun.ID}, + {name: "NULL assistant with history tip above cutoff", id: historyTipAboveWithNullAssistantRun.ID}, + } { + steps, stepsErr := store.GetChatDebugStepsByRunID(ctx, deletedRun.id) + require.NoError(t, stepsErr, "%s: get cascaded steps", deletedRun.name) + require.Empty(t, steps, "%s: deleted run steps must cascade", deletedRun.name) + } + + remainingAssistantBelowRun, err := store.GetChatDebugRunByID(ctx, assistantBelowWithNullHistoryTipRun.ID) + require.NoError(t, err) + require.Equal(t, assistantBelowWithNullHistoryTipRun.ID, remainingAssistantBelowRun.ID, + "assistant below cutoff with NULL history tip must survive") + + remainingAssistantAtBoundaryRun, err := store.GetChatDebugRunByID(ctx, assistantAtBoundaryWithNullHistoryTipRun.ID) + require.NoError(t, err) + require.Equal(t, assistantAtBoundaryWithNullHistoryTipRun.ID, remainingAssistantAtBoundaryRun.ID, + "assistant at cutoff boundary with NULL history tip must survive") + + remainingHistoryTipAtBoundaryRun, err := store.GetChatDebugRunByID(ctx, historyTipAtBoundaryWithNullAssistantRun.ID) + require.NoError(t, err) + require.Equal(t, historyTipAtBoundaryWithNullAssistantRun.ID, remainingHistoryTipAtBoundaryRun.ID, + "history tip at cutoff boundary with NULL assistant must survive") + + remainingBothStepMessageIDsNullRun, err := store.GetChatDebugRunByID(ctx, bothStepMessageIDsNullRun.ID) + require.NoError(t, err) + require.Equal(t, bothStepMessageIDsNullRun.ID, remainingBothStepMessageIDsNullRun.ID, + "both step message IDs NULL must survive") + + assistantBelowSteps, err := store.GetChatDebugStepsByRunID(ctx, assistantBelowWithNullHistoryTipRun.ID) + require.NoError(t, err) + require.Len(t, assistantBelowSteps, 1) + require.Equal(t, assistantBelowWithNullHistoryTipStep.ID, assistantBelowSteps[0].ID) + + assistantAtBoundarySteps, err := store.GetChatDebugStepsByRunID(ctx, assistantAtBoundaryWithNullHistoryTipRun.ID) + require.NoError(t, err) + require.Len(t, assistantAtBoundarySteps, 1) + require.Equal(t, assistantAtBoundaryWithNullHistoryTipStep.ID, assistantAtBoundarySteps[0].ID) + + historyTipAtBoundarySteps, err := store.GetChatDebugStepsByRunID(ctx, historyTipAtBoundaryWithNullAssistantRun.ID) + require.NoError(t, err) + require.Len(t, historyTipAtBoundarySteps, 1) + require.Equal(t, historyTipAtBoundaryWithNullAssistantStep.ID, historyTipAtBoundarySteps[0].ID) + + bothStepMessageIDsNullSteps, err := store.GetChatDebugStepsByRunID(ctx, bothStepMessageIDsNullRun.ID) + require.NoError(t, err) + require.Len(t, bothStepMessageIDsNullSteps, 1) + require.Equal(t, bothStepMessageIDsNullStep.ID, bothStepMessageIDsNullSteps[0].ID) + + remaining, err := store.GetChatDebugRunsByChatID(ctx, database.GetChatDebugRunsByChatIDParams{ + ChatID: chat.ID, + LimitVal: 100, + }) + require.NoError(t, err) + require.Len(t, remaining, 4) +} + func TestFinalizeStaleChatDebugRows(t *testing.T) { t.Parallel()