mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
fix: update the compaction message to be the "user" role (#22819)
## Bug
After compaction in the chat loop, the loop re-enters and calls the LLM
with a prompt that has **no non-system messages**. Anthropic (and most
providers) require at least one user/assistant/tool message, so the API
errors with empty messages.
## Root Cause
The compaction summary was stored as `role=system`. After compaction,
`GetChatMessagesForPromptByChatID` returns only:
- The compressed system summary (matched by the CTE)
- Original non-compressed system messages (system prompts)
All original user/assistant/tool messages are excluded (they predate the
summary). The compaction assistant/tool messages are `compressed=TRUE`
and don't match the main query's `compressed=FALSE` clauses.
So `ReloadMessages` returned only system messages. The Anthropic
provider moves system messages into a separate `system` field, leaving
the `messages` API field as `[]`.
## Fix
1. **Changed compaction summary from `role=system` to `role=user`** —
the summary now appears as a user message in the reloaded prompt, giving
the model valid conversational context to respond to.
2. **Simplified the CTE** — removed the `role = 'system'` check and
narrowed `visibility IN ('model', 'both')` to just `visibility =
'model'`. The summary is the only compressed message with
`visibility=model` (the assistant has `visibility=user`, the tool has
`visibility=both`), so the role check was redundant.
## Test
`PostRunCompactionReEntryIncludesUserSummary`: verifies the re-entry
prompt contains a user message (the compaction summary) after compaction
+ reload.
This commit is contained in:
Kyle Carberry
@@ -2582,7 +2582,7 @@ func (p *Server) persistChatContextSummary(
|
||||
_, txErr := tx.InsertChatMessage(ctx, database.InsertChatMessageParams{
|
||||
ChatID: chatID,
|
||||
ModelConfigID: uuid.NullUUID{UUID: modelConfigID, Valid: true},
|
||||
Role: string(fantasy.MessageRoleSystem),
|
||||
Role: string(fantasy.MessageRoleUser),
|
||||
Content: pqtype.NullRawMessage{
|
||||
RawMessage: systemContent,
|
||||
Valid: len(systemContent) > 0,
|
||||
|
||||
@@ -600,4 +600,117 @@ func TestRun_Compaction(t *testing.T) {
|
||||
// Two stream calls: one before compaction, one after re-entry.
|
||||
require.Equal(t, 2, streamCallCount)
|
||||
})
|
||||
|
||||
t.Run("PostRunCompactionReEntryIncludesUserSummary", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// After compaction the summary is stored as a user-role
|
||||
// message. When the loop re-enters, the reloaded prompt
|
||||
// must contain this user message so the LLM provider
|
||||
// receives a valid prompt (providers like Anthropic
|
||||
// require at least one non-system message).
|
||||
|
||||
var mu sync.Mutex
|
||||
var streamCallCount int
|
||||
var reEntryPrompt []fantasy.Message
|
||||
persistCompactionCalls := 0
|
||||
|
||||
const summaryText = "post-run compacted summary"
|
||||
|
||||
model := &loopTestModel{
|
||||
provider: "fake",
|
||||
streamFn: func(_ context.Context, call fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
mu.Lock()
|
||||
step := streamCallCount
|
||||
streamCallCount++
|
||||
mu.Unlock()
|
||||
|
||||
switch step {
|
||||
case 0:
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "initial response"},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"},
|
||||
{
|
||||
Type: fantasy.StreamPartTypeFinish,
|
||||
FinishReason: fantasy.FinishReasonStop,
|
||||
Usage: fantasy.Usage{
|
||||
InputTokens: 80,
|
||||
TotalTokens: 85,
|
||||
},
|
||||
},
|
||||
}), nil
|
||||
default:
|
||||
mu.Lock()
|
||||
reEntryPrompt = append([]fantasy.Message(nil), call.Prompt...)
|
||||
mu.Unlock()
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-2"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-2", Delta: "continued"},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-2"},
|
||||
{
|
||||
Type: fantasy.StreamPartTypeFinish,
|
||||
FinishReason: fantasy.FinishReasonStop,
|
||||
Usage: fantasy.Usage{
|
||||
InputTokens: 20,
|
||||
TotalTokens: 25,
|
||||
},
|
||||
},
|
||||
}), nil
|
||||
}
|
||||
},
|
||||
generateFn: func(_ context.Context, _ fantasy.Call) (*fantasy.Response, error) {
|
||||
return &fantasy.Response{
|
||||
Content: []fantasy.Content{
|
||||
fantasy.TextContent{Text: summaryText},
|
||||
},
|
||||
}, nil
|
||||
},
|
||||
}
|
||||
|
||||
// Simulate real post-compaction DB state: the summary is
|
||||
// a user-role message (the only non-system content).
|
||||
compactedMessages := []fantasy.Message{
|
||||
textMessage(fantasy.MessageRoleSystem, "system prompt"),
|
||||
textMessage(fantasy.MessageRoleUser, "Summary of earlier chat context:\n\ncompacted summary"),
|
||||
}
|
||||
|
||||
err := Run(context.Background(), RunOptions{
|
||||
Model: model,
|
||||
Messages: []fantasy.Message{
|
||||
textMessage(fantasy.MessageRoleUser, "hello"),
|
||||
},
|
||||
MaxSteps: 5,
|
||||
PersistStep: func(_ context.Context, _ PersistedStep) error {
|
||||
return nil
|
||||
},
|
||||
ContextLimitFallback: 100,
|
||||
Compaction: &CompactionOptions{
|
||||
ThresholdPercent: 70,
|
||||
SummaryPrompt: "summarize now",
|
||||
Persist: func(_ context.Context, _ CompactionResult) error {
|
||||
persistCompactionCalls++
|
||||
return nil
|
||||
},
|
||||
},
|
||||
ReloadMessages: func(_ context.Context) ([]fantasy.Message, error) {
|
||||
return compactedMessages, nil
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
require.GreaterOrEqual(t, persistCompactionCalls, 1)
|
||||
// Re-entry happened: stream was called at least twice.
|
||||
require.Equal(t, 2, streamCallCount)
|
||||
// The re-entry prompt must contain the user summary.
|
||||
require.NotEmpty(t, reEntryPrompt)
|
||||
hasUser := false
|
||||
for _, msg := range reEntryPrompt {
|
||||
if msg.Role == fantasy.MessageRoleUser {
|
||||
hasUser = true
|
||||
break
|
||||
}
|
||||
}
|
||||
require.True(t, hasUser, "re-entry prompt must contain a user message (the compaction summary)")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -8841,3 +8841,202 @@ func TestInsertWorkspaceAgentDevcontainers(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetChatMessagesForPromptByChatID(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// This test exercises a complex CTE query for prompt
|
||||
// reconstruction after compaction. It requires Postgres.
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
ctx := context.Background()
|
||||
|
||||
// Helper: create a chat model config (required FK for chats).
|
||||
user := dbgen.User(t, db, database.User{})
|
||||
|
||||
// A chat_providers row is required as a FK for model configs.
|
||||
_, err := db.InsertChatProvider(ctx, database.InsertChatProviderParams{
|
||||
Provider: "openai",
|
||||
DisplayName: "OpenAI",
|
||||
APIKey: "test-key",
|
||||
Enabled: true,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
modelCfg, err := db.InsertChatModelConfig(ctx, database.InsertChatModelConfigParams{
|
||||
Provider: "openai",
|
||||
Model: "test-model",
|
||||
DisplayName: "Test 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)
|
||||
|
||||
newChat := func(t *testing.T) database.Chat {
|
||||
t.Helper()
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OwnerID: user.ID,
|
||||
LastModelConfigID: modelCfg.ID,
|
||||
Title: "test-chat-" + uuid.NewString(),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
return chat
|
||||
}
|
||||
|
||||
insertMsg := func(
|
||||
t *testing.T,
|
||||
chatID uuid.UUID,
|
||||
role string,
|
||||
vis database.ChatMessageVisibility,
|
||||
compressed bool,
|
||||
content string,
|
||||
) database.ChatMessage {
|
||||
t.Helper()
|
||||
msg, err := db.InsertChatMessage(ctx, database.InsertChatMessageParams{
|
||||
ChatID: chatID,
|
||||
Role: role,
|
||||
Visibility: vis,
|
||||
Compressed: sql.NullBool{Bool: compressed, Valid: true},
|
||||
Content: pqtype.NullRawMessage{
|
||||
RawMessage: json.RawMessage(`"` + content + `"`),
|
||||
Valid: true,
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
return msg
|
||||
}
|
||||
|
||||
msgIDs := func(msgs []database.ChatMessage) []int64 {
|
||||
ids := make([]int64, len(msgs))
|
||||
for i, m := range msgs {
|
||||
ids[i] = m.ID
|
||||
}
|
||||
return ids
|
||||
}
|
||||
|
||||
t.Run("NoCompaction", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
chat := newChat(t)
|
||||
|
||||
sys := insertMsg(t, chat.ID, "system", database.ChatMessageVisibilityModel, false, "system prompt")
|
||||
usr := insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityBoth, false, "hello")
|
||||
ast := insertMsg(t, chat.ID, "assistant", database.ChatMessageVisibilityBoth, false, "hi there")
|
||||
|
||||
got, err := db.GetChatMessagesForPromptByChatID(ctx, chat.ID)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, []int64{sys.ID, usr.ID, ast.ID}, msgIDs(got))
|
||||
})
|
||||
|
||||
t.Run("UserOnlyVisibilityExcluded", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
chat := newChat(t)
|
||||
|
||||
// Messages with visibility=user should NOT appear in the
|
||||
// prompt (they are only for the UI).
|
||||
insertMsg(t, chat.ID, "system", database.ChatMessageVisibilityModel, false, "system prompt")
|
||||
insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityUser, false, "user-only msg")
|
||||
usr := insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityBoth, false, "hello")
|
||||
|
||||
got, err := db.GetChatMessagesForPromptByChatID(ctx, chat.ID)
|
||||
require.NoError(t, err)
|
||||
for _, m := range got {
|
||||
require.NotEqual(t, database.ChatMessageVisibilityUser, m.Visibility,
|
||||
"visibility=user messages should not appear in the prompt")
|
||||
}
|
||||
require.Contains(t, msgIDs(got), usr.ID)
|
||||
})
|
||||
|
||||
t.Run("AfterCompaction", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
chat := newChat(t)
|
||||
|
||||
// Pre-compaction conversation.
|
||||
sys := insertMsg(t, chat.ID, "system", database.ChatMessageVisibilityModel, false, "system prompt")
|
||||
preUser := insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityBoth, false, "old question")
|
||||
preAsst := insertMsg(t, chat.ID, "assistant", database.ChatMessageVisibilityBoth, false, "old answer")
|
||||
|
||||
// Compaction messages:
|
||||
// 1. Summary (role=user, visibility=model, compressed=true).
|
||||
summary := insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityModel, true, "compaction summary")
|
||||
// 2. Compressed assistant tool-call (visibility=user).
|
||||
insertMsg(t, chat.ID, "assistant", database.ChatMessageVisibilityUser, true, "tool call")
|
||||
// 3. Compressed tool result (visibility=both).
|
||||
insertMsg(t, chat.ID, "tool", database.ChatMessageVisibilityBoth, true, "tool result")
|
||||
|
||||
// Post-compaction messages.
|
||||
postUser := insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityBoth, false, "new question")
|
||||
postAsst := insertMsg(t, chat.ID, "assistant", database.ChatMessageVisibilityBoth, false, "new answer")
|
||||
|
||||
got, err := db.GetChatMessagesForPromptByChatID(ctx, chat.ID)
|
||||
require.NoError(t, err)
|
||||
|
||||
gotIDs := msgIDs(got)
|
||||
|
||||
// Must include: system prompt, summary, post-compaction.
|
||||
require.Contains(t, gotIDs, sys.ID, "system prompt must be included")
|
||||
require.Contains(t, gotIDs, summary.ID, "compaction summary must be included")
|
||||
require.Contains(t, gotIDs, postUser.ID, "post-compaction user msg must be included")
|
||||
require.Contains(t, gotIDs, postAsst.ID, "post-compaction assistant msg must be included")
|
||||
|
||||
// Must exclude: pre-compaction non-system messages.
|
||||
require.NotContains(t, gotIDs, preUser.ID, "pre-compaction user msg must be excluded")
|
||||
require.NotContains(t, gotIDs, preAsst.ID, "pre-compaction assistant msg must be excluded")
|
||||
|
||||
// Verify ordering.
|
||||
require.Equal(t, []int64{sys.ID, summary.ID, postUser.ID, postAsst.ID}, gotIDs)
|
||||
})
|
||||
|
||||
t.Run("AfterCompactionSummaryIsUserRole", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
chat := newChat(t)
|
||||
|
||||
// After compaction the summary must appear as role=user so
|
||||
// that LLM APIs (e.g. Anthropic) see at least one
|
||||
// non-system message in the prompt.
|
||||
insertMsg(t, chat.ID, "system", database.ChatMessageVisibilityModel, false, "system prompt")
|
||||
summary := insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityModel, true, "summary text")
|
||||
newUsr := insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityBoth, false, "new question")
|
||||
|
||||
got, err := db.GetChatMessagesForPromptByChatID(ctx, chat.ID)
|
||||
require.NoError(t, err)
|
||||
|
||||
hasNonSystem := false
|
||||
for _, m := range got {
|
||||
if m.Role != "system" {
|
||||
hasNonSystem = true
|
||||
break
|
||||
}
|
||||
}
|
||||
require.True(t, hasNonSystem,
|
||||
"prompt must contain at least one non-system message after compaction")
|
||||
require.Contains(t, msgIDs(got), summary.ID)
|
||||
require.Contains(t, msgIDs(got), newUsr.ID)
|
||||
})
|
||||
|
||||
t.Run("CompressedToolResultNotPickedAsSummary", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
chat := newChat(t)
|
||||
|
||||
// The CTE uses visibility='model' (exact match). If it
|
||||
// used IN ('model','both'), the compressed tool result
|
||||
// (visibility=both) would be picked as the "summary"
|
||||
// instead of the actual summary.
|
||||
insertMsg(t, chat.ID, "system", database.ChatMessageVisibilityModel, false, "system prompt")
|
||||
summary := insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityModel, true, "real summary")
|
||||
compressedTool := insertMsg(t, chat.ID, "tool", database.ChatMessageVisibilityBoth, true, "tool result")
|
||||
postUser := insertMsg(t, chat.ID, "user", database.ChatMessageVisibilityBoth, false, "follow-up")
|
||||
|
||||
got, err := db.GetChatMessagesForPromptByChatID(ctx, chat.ID)
|
||||
require.NoError(t, err)
|
||||
|
||||
gotIDs := msgIDs(got)
|
||||
require.Contains(t, gotIDs, summary.ID, "real summary must be included")
|
||||
require.NotContains(t, gotIDs, compressedTool.ID,
|
||||
"compressed tool result must not be included")
|
||||
require.Contains(t, gotIDs, postUser.ID)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -3321,9 +3321,8 @@ WITH latest_compressed_summary AS (
|
||||
chat_messages
|
||||
WHERE
|
||||
chat_id = $1::uuid
|
||||
AND role = 'system'
|
||||
AND visibility IN ('model', 'both')
|
||||
AND compressed = TRUE
|
||||
AND visibility = 'model'
|
||||
ORDER BY
|
||||
created_at DESC,
|
||||
id DESC
|
||||
|
||||
@@ -54,9 +54,8 @@ WITH latest_compressed_summary AS (
|
||||
chat_messages
|
||||
WHERE
|
||||
chat_id = @chat_id::uuid
|
||||
AND role = 'system'
|
||||
AND visibility IN ('model', 'both')
|
||||
AND compressed = TRUE
|
||||
AND visibility = 'model'
|
||||
ORDER BY
|
||||
created_at DESC,
|
||||
id DESC
|
||||
|
||||
Reference in New Issue
Block a user