From d4d01f9272bc397e3f43a5ca32fc3c2bc403507f Mon Sep 17 00:00:00 2001 From: mark42 Date: Sat, 22 Aug 2026 20:31:40 +0900 Subject: [PATCH] fix(prompt): stop invalidating provider prompt caches on every turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The system prompt led with a second-resolution wall-clock timestamp, so its first token block differed on every request. Prompt caching is prefix-matched, so nothing ever matched: the cache_control marker on the Anthropic system block wrote a fresh entry each turn and never read one, and OpenAI/Gemini automatic prefix caching never engaged either. The whole static body — identity, planning, formatting, long-running command rules, workspace bootstrap — was re-charged at write rates every turn. Move the clock to a "## Current Time" block at the very end of the prompt and truncate it to the minute, so a burst of turns shares one prefix. Keep it in the system prompt rather than the first user message: decorating the outgoing user turn would either pollute persisted history or break message-prefix matching that LP-002 is about to rely on. The builder's tail was not the assembled prompt's tail — the chat assembler appends the memory rule, skills, session override, style, goal, and critic *after* the builder's output, so per-turn recall sat mid-prompt with static text behind it. BuildResult now exposes StaticPrompt and DynamicTail separately and the assembler emits the tail last, with the ordering invariant documented on BuildResultFor. Anthropic gets one cache breakpoint on the system block, so a volatile tail anywhere inside it is still all-or-nothing. Emit system messages as one text block each and mark only the first — callers order them stable-first, so the tail now falls outside the cached prefix. A single system message behaves exactly as before. Also fix a prompt-identity bug the audit turned up: the chat memory cache stored the fully assembled prompt, and the prefetch goroutine builds without the session's work dirs or current dir. Once its entry landed in the cache, the next turn's prompt lost "## Working Directories" entirely. The cache now holds only the recall payload and the prompt is always rebuilt from live options, so a cache hit and a cache miss are byte-identical by construction. Closes #920 Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 5 + internal/llm/anthropic.go | 38 +++-- internal/llm/anthropic_test.go | 77 +++++++++ internal/prompt/builder.go | 93 ++++++++++- internal/prompt/builder_test.go | 148 +++++++++++++++++- internal/tarsserver/handler_chat.go | 72 +++++++-- internal/tarsserver/handler_chat_context.go | 6 +- internal/tarsserver/handler_chat_pipeline.go | 2 +- internal/tarsserver/handler_chat_prefetch.go | 6 +- .../handler_chat_prompt_cache_test.go | 140 +++++++++++++++++ internal/tarsserver/memory_cache.go | 52 +++++- internal/tarsserver/memory_cache_test.go | 45 ++++-- 12 files changed, 622 insertions(+), 62 deletions(-) create mode 100644 internal/tarsserver/handler_chat_prompt_cache_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 41321727..b46d88da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ The format is based on Keep a Changelog and the project follows Semantic Version - **`antigravity-cli` provider** — Reuses a locally installed `agy` CLI the way `claude-code-cli` reuses `claude`, via `agy --print --output-format stream-json`. The CLI owns its Google login in the system keyring, so the kind takes no `api_key` and no `base_url` and no credential passes through TARS. Multi-turn resumes through the stream's `conversation_id` (`--conversation`), `ReasoningEffort` maps to `--effort`, and a `json_schema` response format maps to `--json-schema`. Tools remain the CLI's own and are reported on `ChatResponse.ProviderExecutedTools` for audit only, never re-dispatched through TARS' registry. `AGY_CLI_MODE` accepts only `accept-edits` or `plan`; there is deliberately no path to `--dangerously-skip-permissions`. Requires Antigravity CLI 1.1.12 or newer. +### Fixed + +- **프롬프트 캐시가 매 턴 무효화되던 문제 (#920)** — 시스템 프롬프트 첫 줄이 초 단위 wall-clock 타임스탬프라 어떤 프로바이더의 prefix 캐시에도 걸리지 않았고, 정적 본문 전체가 매 턴 write 요금으로 재과금됐다. 시각은 이제 프롬프트 맨 뒤 `## Current Time` 블록으로 내려가 분 단위로 truncate되고, `## Prior Context` 회상도 skills/style/goal/critic 등 세션 고정 섹션 **뒤**로 재배치된다. Anthropic 요청은 system을 블록 배열로 보내 `cache_control` breakpoint를 안정 블록에만 찍으므로 동적 tail이 캐시 prefix를 깨지 않는다. 정렬 규칙은 `prompt.BuildResultFor`에 불변식으로 문서화했다. +- **메모리 prefetch가 세션 작업 디렉터리를 지우던 문제 (#920)** — 채팅 메모리 캐시가 조립된 프롬프트를 통째로 저장하는 바람에, work dir을 모르는 prefetch 고루틴의 결과가 캐시에 올라가면 그 다음 턴 프롬프트에서 `## Working Directories` 섹션이 통째로 사라졌다. 이제 캐시는 회상 payload만 보관하고 프롬프트는 항상 live 옵션으로 다시 조립한다 — 캐시 hit과 miss가 구조적으로 동일한 프롬프트를 낸다. + ## [0.35.0] - 2026-08-03 ### Added diff --git a/internal/llm/anthropic.go b/internal/llm/anthropic.go index a4e67754..a34758ce 100644 --- a/internal/llm/anthropic.go +++ b/internal/llm/anthropic.go @@ -110,15 +110,7 @@ func (c *AnthropicClient) buildChatRequest(messages []ChatMessage, opts ChatOpti } } if len(systemMessages) > 0 { - reqBody["system"] = []map[string]any{ - { - "type": "text", - "text": strings.Join(systemMessages, "\n"), - "cache_control": map[string]any{ - "type": "ephemeral", - }, - }, - } + reqBody["system"] = toAnthropicSystemBlocks(systemMessages) } if tools := toAnthropicTools(opts.Tools); len(tools) > 0 { tools[len(tools)-1].CacheControl = map[string]any{"type": "ephemeral"} @@ -133,6 +125,34 @@ func (c *AnthropicClient) buildChatRequest(messages []ChatMessage, opts ChatOpti return reqBody } +// toAnthropicSystemBlocks renders the collected system messages as one text +// block each and marks the cacheable prefix. +// +// The breakpoint goes on the FIRST block. Callers order their system messages +// stable-first, volatile-last (see prompt.BuildResultFor's ordering +// invariant), so the first block is the turn-stable region and everything +// after it — per-turn recall, the clock, drained critic feedback — stays +// outside the cached prefix. Anthropic's cache is prefix-matched at the +// breakpoint, so a marker placed after volatile text would write a fresh entry +// every turn and never read one. +// +// A single system message keeps the previous behavior exactly: one block, +// cached in full. +func toAnthropicSystemBlocks(systemMessages []string) []map[string]any { + blocks := make([]map[string]any, 0, len(systemMessages)) + for i, text := range systemMessages { + block := map[string]any{ + "type": "text", + "text": text, + } + if i == 0 { + block["cache_control"] = map[string]any{"type": "ephemeral"} + } + blocks = append(blocks, block) + } + return blocks +} + func (c *AnthropicClient) chatNonStreamingResponse(body io.Reader) (ChatResponse, error) { respBody, err := io.ReadAll(body) if err != nil { diff --git a/internal/llm/anthropic_test.go b/internal/llm/anthropic_test.go index d5d2b065..84161145 100644 --- a/internal/llm/anthropic_test.go +++ b/internal/llm/anthropic_test.go @@ -351,3 +351,80 @@ func TestAnthropicChat_StreamThinkingDelta(t *testing.T) { t.Fatalf("Content got %q", resp.Message.Content) } } + +// A system prompt split into a stable head and a volatile tail must keep the +// cache breakpoint on the head. Marking the tail instead writes a fresh cache +// entry every turn and reads none — the LP-001 failure mode. +func TestToAnthropicSystemBlocks_BreakpointStaysOnStablePrefix(t *testing.T) { + blocks := toAnthropicSystemBlocks([]string{"static prefix", "## Current Time\n\n2026-08-22T10:23:00Z"}) + if len(blocks) != 2 { + t.Fatalf("expected one block per system message, got %d", len(blocks)) + } + if blocks[0]["text"] != "static prefix" { + t.Fatalf("expected stable prefix first, got %+v", blocks[0]["text"]) + } + if _, ok := blocks[0]["cache_control"]; !ok { + t.Fatal("expected cache_control on the stable prefix block") + } + if _, ok := blocks[1]["cache_control"]; ok { + t.Fatal("expected no cache_control on the volatile tail block") + } +} + +func TestToAnthropicSystemBlocks_SingleMessageKeepsWholePromptCached(t *testing.T) { + blocks := toAnthropicSystemBlocks([]string{"only block"}) + if len(blocks) != 1 { + t.Fatalf("expected a single block, got %d", len(blocks)) + } + if _, ok := blocks[0]["cache_control"]; !ok { + t.Fatal("expected cache_control on the only system block") + } +} + +func TestAnthropicChat_EmitsSystemTailOutsideCachedPrefix(t *testing.T) { + var captured map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := json.NewDecoder(r.Body).Decode(&captured); err != nil { + t.Fatalf("decode request: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"content":[{"type":"text","text":"ok"}]}`)) + })) + defer srv.Close() + + client, err := NewAnthropicClient(srv.URL, "k", "claude-3-5-haiku-latest", 0) + if err != nil { + t.Fatalf("new client: %v", err) + } + _, err = client.Chat(context.Background(), []ChatMessage{ + {Role: "system", Content: "stable body"}, + {Role: "system", Content: "## Current Time\n\nCurrent time: 2026-08-22T10:23:00Z"}, + {Role: "user", Content: "hi"}, + }, ChatOptions{}) + if err != nil { + t.Fatalf("chat: %v", err) + } + + systemRaw, ok := captured["system"].([]any) + if !ok || len(systemRaw) != 2 { + t.Fatalf("expected two system blocks, got %+v", captured["system"]) + } + head, ok := systemRaw[0].(map[string]any) + if !ok || head["text"] != "stable body" { + t.Fatalf("unexpected head block: %+v", systemRaw[0]) + } + if _, ok := head["cache_control"]; !ok { + t.Fatalf("expected cache_control on head block, got %+v", head) + } + tail, ok := systemRaw[1].(map[string]any) + if !ok { + t.Fatalf("invalid tail block: %+v", systemRaw[1]) + } + tailText, _ := tail["text"].(string) + if !strings.Contains(tailText, "Current time:") { + t.Fatalf("expected the clock in the tail block, got %+v", tail["text"]) + } + if _, ok := tail["cache_control"]; ok { + t.Fatalf("expected no cache_control on tail block, got %+v", tail) + } +} diff --git a/internal/prompt/builder.go b/internal/prompt/builder.go index b172a56f..54ec99ac 100644 --- a/internal/prompt/builder.go +++ b/internal/prompt/builder.go @@ -27,6 +27,22 @@ type BuildOptions struct { StaticBudgetTokens int RelevantBudgetTokens int TotalBudgetTokens int + // PresetRelevant short-circuits the "## Prior Context" recall. When set, + // the builder reuses this payload verbatim instead of querying + // MemorySearcher. Callers that cache recall across turns must use this + // rather than caching the assembled prompt: the static region depends on + // live options (WorkDirs, CurrentDir, PlanClarifyMode, workspace files), + // so replaying a stored prompt ships a stale one and defeats the very + // provider prompt cache it was meant to help. + PresetRelevant *PresetRelevantMemory +} + +// PresetRelevantMemory is a previously computed "## Prior Context" section, +// handed back to the builder so a repeat semantic search can be skipped. +type PresetRelevantMemory struct { + Section string + Items []RelevantMemoryItem + Tokens int } // RelevantMemoryItem is the structured form of one line injected into the @@ -39,8 +55,15 @@ type RelevantMemoryItem struct { } // BuildResult captures prompt assembly output and budget usage. +// +// Prompt is StaticPrompt+DynamicTail. The two are also exposed separately so +// the chat assembler can keep appending its own static sections (skills, +// session style, goal, critic) *before* the dynamic tail — see the ordering +// invariant on BuildResultFor. type BuildResult struct { Prompt string + StaticPrompt string + DynamicTail string StaticTokens int RelevantTokens int RelevantMemoryCount int @@ -57,18 +80,31 @@ func Build(opts BuildOptions) string { // BuildResultFor assembles a system prompt and returns budget usage details. // +// ORDERING INVARIANT: static sections first, dynamic sections last. +// +// Provider prompt caching is prefix-matched — Anthropic against an explicit +// cache_control breakpoint, OpenAI and Gemini automatically. Anything that +// changes between two turns of the same session therefore has to sit behind +// everything that does not, or the cacheable prefix ends at the first byte +// that moved. Until LP-001 the wall-clock timestamp was the prompt's *first* +// line, so no prefix ever matched and the entire static body was re-charged +// at write rates on every single turn. +// +// Static (in order): Response Formatting, Planning, Long-running Commands, +// workspace bootstrap sections, Working Directories. +// Dynamic (BuildResult.DynamicTail, appended last): "## Prior Context" +// recall, then "## Current Time". +// // The identity line (\"You are TARS, a personal AI assistant.\") was // removed from the hardcoded header in ID-002(a). It now lives in the // workspace IDENTITY.md default content, which is loaded as the // \"## Identity\" bootstrap section below — that lets users override // their assistant’s identity without recompiling. Response Formatting -// rules and the dynamic \"Current time\" line stay in code: they describe -// runtime constraints, not user-tunable persona. +// rules and the dynamic time line stay in code: they describe runtime +// constraints, not user-tunable persona. func BuildResultFor(opts BuildOptions) BuildResult { var b strings.Builder - b.WriteString(fmt.Sprintf("Current time: %s\n", time.Now().UTC().Format(time.RFC3339))) - b.WriteString("\n") b.WriteString("## Response Formatting\n\n") b.WriteString("Always use rich Markdown in your responses:\n") b.WriteString("- Use headings, bold, lists, and tables to structure information clearly.\n") @@ -129,7 +165,11 @@ func BuildResultFor(opts BuildOptions) BuildResult { if totalBudgetTokens <= 0 { totalBudgetTokens = defaultTotalBudgetTokens } - totalTokens := estimateTokens(b.String()) + // The clock block is rendered last but charged here: it is always emitted, + // so reserving it up front keeps the static sections clamped inside the + // total budget exactly as they were when the timestamp led the prompt. + timeSection := currentTimeSection() + totalTokens := estimateTokens(b.String()) + estimateTokens(timeSection) remainingTotalTokens := max(0, totalBudgetTokens-totalTokens) remainingStaticTokens := opts.StaticBudgetTokens @@ -196,29 +236,49 @@ func BuildResultFor(opts BuildOptions) BuildResult { remainingTotalTokens -= sectionTokens } + // Everything written above is static for the lifetime of a session (it + // only moves when a workspace bootstrap file or a session setting + // changes). Everything below is per-turn and must stay behind it. + staticPrompt := b.String() + relevantTokens := 0 relevantCount := 0 relevantBudgetTokens := 0 relevantSection := "" var relevantItems []RelevantMemoryItem usedTokens := 0 + var tail strings.Builder if !opts.SubAgent { relevantBudgetTokens = opts.RelevantBudgetTokens if relevantBudgetTokens <= 0 { relevantBudgetTokens = defaultRelevantBudgetTokens } relevantBudgetTokens = min(relevantBudgetTokens, remainingTotalTokens) - relevantSection, relevantItems, usedTokens = buildRelevantMemorySection(opts, relevantBudgetTokens) + if opts.PresetRelevant != nil { + relevantSection = opts.PresetRelevant.Section + relevantItems = append([]RelevantMemoryItem(nil), opts.PresetRelevant.Items...) + usedTokens = opts.PresetRelevant.Tokens + } else { + relevantSection, relevantItems, usedTokens = buildRelevantMemorySection(opts, relevantBudgetTokens) + } if relevantSection != "" { - b.WriteString(relevantSection) + tail.WriteString(relevantSection) relevantTokens = usedTokens relevantCount = len(relevantItems) totalTokens += usedTokens } } + // Recall changes with the user's query; the clock changes on its own. The + // clock goes last so that re-running an identical query inside the same + // minute still matches through the recall block. + tail.WriteString(timeSection) + + dynamicTail := tail.String() return BuildResult{ - Prompt: b.String(), + Prompt: staticPrompt + dynamicTail, + StaticPrompt: staticPrompt, + DynamicTail: dynamicTail, StaticTokens: staticTokens, RelevantTokens: relevantTokens, RelevantMemoryCount: relevantCount, @@ -229,6 +289,23 @@ func BuildResultFor(opts BuildOptions) BuildResult { } } +// timeNow is a seam so tests can build the prompt at two different wall-clock +// instants and compare the resulting prefixes. +var timeNow = time.Now + +// currentTimeSection renders the dynamic clock block that closes every prompt. +// +// The timestamp is truncated to the minute rather than the second. Second +// resolution is more precision than any assistant answer needs, and it would +// re-break the prefix on every retry, tool-loop restart, or rapid follow-up — +// which is exactly the burst where a cache hit is worth the most. +func currentTimeSection() string { + return fmt.Sprintf( + "\n## Current Time\n\nCurrent time: %s\n", + timeNow().UTC().Truncate(time.Minute).Format(time.RFC3339), + ) +} + func readBootstrapSection(workspaceDir string, section bootstrapSection) string { parts := make([]string, 0, len(section.files)) for _, name := range section.files { diff --git a/internal/prompt/builder_test.go b/internal/prompt/builder_test.go index 1db9503d..80811814 100644 --- a/internal/prompt/builder_test.go +++ b/internal/prompt/builder_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) func TestBuild(t *testing.T) { @@ -275,9 +276,10 @@ func TestBuildResult_PrioritizesHigherOrderStaticSections(t *testing.T) { } } - // Floor must stay above the hardcoded header (Current time + Response - // Formatting + Planning + Long-running Commands). Each addition to - // the always-on header forces a bump here; 850 accommodates the + // Floor must stay above the always-on scaffolding (Response Formatting + + // Planning + Long-running Commands, plus the Current Time tail, which is + // charged up front even though it renders last). Each addition to + // that scaffolding forces a bump here; 850 accommodates the // post-Phase-2 layout while still keeping the prioritization // assertion meaningful (USER fits, IDENTITY/TOOLS get clamped). result := BuildResultFor(BuildOptions{ @@ -304,11 +306,11 @@ func TestBuildResult_ClampsRelevantMemoryToRemainingTotalBudget(t *testing.T) { } // Budget here is a stress test for the clamping logic, not a target - // for production. The total floor must accommodate the hardcoded - // header (Current time + Response Formatting + Planning + Long-running - // Commands ≈ ~430 tokens) plus the static USER section, otherwise + // for production. The total floor must accommodate the always-on + // scaffolding (Response Formatting + Planning + Long-running Commands + + // Current Time ≈ ~430 tokens) plus the static USER section, otherwise // relevant memory has nothing left to clamp. 850 keeps the assertion - // meaningful with headroom for future header tweaks. + // meaningful with headroom for future tweaks. result := BuildResultFor(BuildOptions{ WorkspaceDir: root, Query: "what coffee do i prefer?", @@ -324,3 +326,135 @@ func TestBuildResult_ClampsRelevantMemoryToRemainingTotalBudget(t *testing.T) { t.Fatalf("expected relevant memory to fit remaining budget, got static=%d relevant=%d", result.StaticTokens, result.RelevantTokens) } } + +// withFixedTime pins the builder's clock for the duration of the test. +func withFixedTime(t *testing.T, at time.Time) { + t.Helper() + prev := timeNow + timeNow = func() time.Time { return at } + t.Cleanup(func() { timeNow = prev }) +} + +// LP-001: the cacheable region must not move when the clock does. Two builds +// hours apart have to agree byte-for-byte through the end of the static +// sections, or every provider's prefix cache misses on every turn. +func TestBuildResult_StaticPrefixSurvivesClockChange(t *testing.T) { + root := t.TempDir() + for name, content := range map[string]string{ + "IDENTITY.md": "# IDENTITY.md\n\nName: TARS", + "USER.md": "# USER.md\n\nName: Alice", + } { + if err := os.WriteFile(filepath.Join(root, name), []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + + withFixedTime(t, time.Date(2026, 8, 22, 10, 23, 45, 0, time.UTC)) + first := BuildResultFor(BuildOptions{WorkspaceDir: root}) + + withFixedTime(t, time.Date(2026, 8, 22, 17, 4, 9, 0, time.UTC)) + second := BuildResultFor(BuildOptions{WorkspaceDir: root}) + + if first.StaticPrompt != second.StaticPrompt { + t.Fatalf("static region changed with the clock:\nfirst=%q\nsecond=%q", first.StaticPrompt, second.StaticPrompt) + } + if first.DynamicTail == second.DynamicTail { + t.Fatal("expected the dynamic tail to carry the clock change") + } + if !strings.HasPrefix(first.Prompt, first.StaticPrompt) { + t.Fatal("expected Prompt to lead with StaticPrompt") + } + if first.Prompt != first.StaticPrompt+first.DynamicTail { + t.Fatal("expected Prompt to be StaticPrompt+DynamicTail") + } + if strings.Contains(first.StaticPrompt, "Current time:") { + t.Fatalf("expected no timestamp in the static region, got %q", first.StaticPrompt) + } +} + +// The clock still has to reach the model — just from the tail. +func TestBuildResult_KeepsCurrentTimeInTail(t *testing.T) { + root := t.TempDir() + withFixedTime(t, time.Date(2026, 8, 22, 10, 23, 45, 0, time.UTC)) + + result := BuildResultFor(BuildOptions{WorkspaceDir: root}) + + // Truncated to the minute so a burst of turns shares one prefix. + const want = "Current time: 2026-08-22T10:23:00Z" + if !strings.Contains(result.Prompt, want) { + t.Fatalf("expected %q in prompt, got %q", want, result.Prompt) + } + if !strings.Contains(result.DynamicTail, want) { + t.Fatalf("expected the clock in the dynamic tail, got %q", result.DynamicTail) + } +} + +func TestBuildResult_SubAgentPromptStillCarriesTime(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "AGENTS.md"), []byte("# AGENTS.md\n\nrules"), 0o644); err != nil { + t.Fatalf("write AGENTS.md: %v", err) + } + withFixedTime(t, time.Date(2026, 8, 22, 10, 23, 45, 0, time.UTC)) + + result := BuildResultFor(BuildOptions{WorkspaceDir: root, SubAgent: true}) + + if !strings.Contains(result.Prompt, "Current time: 2026-08-22T10:23:00Z") { + t.Fatalf("expected sub-agent prompt to carry the clock, got %q", result.Prompt) + } +} + +// Callers that cache recall must be able to replay it without re-running the +// search and without touching the static region. +func TestBuildResult_PresetRelevantReplacesSearch(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "MEMORY.md"), []byte("User prefers black coffee.\n"), 0o644); err != nil { + t.Fatalf("write MEMORY.md: %v", err) + } + withFixedTime(t, time.Date(2026, 8, 22, 10, 23, 45, 0, time.UTC)) + + live := BuildResultFor(BuildOptions{WorkspaceDir: root, Query: "what coffee do i prefer?"}) + if live.RelevantSection == "" { + t.Fatal("expected the live path to produce a prior-context section") + } + + // No Query and no searcher: the preset alone must reproduce the prompt. + replayed := BuildResultFor(BuildOptions{ + WorkspaceDir: root, + PresetRelevant: &PresetRelevantMemory{ + Section: live.RelevantSection, + Items: live.RelevantMemoryItems, + Tokens: live.RelevantTokens, + }, + }) + + if replayed.Prompt != live.Prompt { + t.Fatalf("replayed prompt differs:\nlive=%q\nreplayed=%q", live.Prompt, replayed.Prompt) + } + if replayed.RelevantMemoryCount != live.RelevantMemoryCount { + t.Fatalf("expected %d recalled items, got %d", live.RelevantMemoryCount, replayed.RelevantMemoryCount) + } +} + +// The tail must close the prompt: recall first, clock last, so an identical +// query re-run inside the same minute matches all the way through. +func TestBuildResult_DynamicTailOrdersRecallBeforeClock(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "MEMORY.md"), []byte("User prefers black coffee.\n"), 0o644); err != nil { + t.Fatalf("write MEMORY.md: %v", err) + } + withFixedTime(t, time.Date(2026, 8, 22, 10, 23, 45, 0, time.UTC)) + + result := BuildResultFor(BuildOptions{WorkspaceDir: root, Query: "what coffee do i prefer?"}) + + recallAt := strings.Index(result.DynamicTail, "## Prior Context") + clockAt := strings.Index(result.DynamicTail, "## Current Time") + if recallAt < 0 || clockAt < 0 { + t.Fatalf("expected both dynamic sections in the tail, got %q", result.DynamicTail) + } + if recallAt > clockAt { + t.Fatalf("expected recall before the clock, got %q", result.DynamicTail) + } + if !strings.HasSuffix(result.Prompt, result.DynamicTail) { + t.Fatal("expected the dynamic tail to close the prompt") + } +} diff --git a/internal/tarsserver/handler_chat.go b/internal/tarsserver/handler_chat.go index 87bf8781..02375f87 100644 --- a/internal/tarsserver/handler_chat.go +++ b/internal/tarsserver/handler_chat.go @@ -69,7 +69,14 @@ func prepareChatContext(workspaceDir, userMessage string) (systemPrompt string, } type preparedChatContext struct { - SystemPrompt string + // SystemPrompt holds only the turn-stable region. Callers keep appending + // their own static sections to it and must emit SystemPromptTail last — + // see the ordering invariant on prompt.BuildResultFor. + SystemPrompt string + // SystemPromptTail is the per-turn region (prior-context recall, current + // time). It closes the assembled system prompt so everything ahead of it + // stays a matchable provider cache prefix. + SystemPromptTail string ToolChoice *llm.ToolChoice SystemPromptTokens int RelevantMemoryCount int @@ -91,7 +98,9 @@ func prepareChatContextWithExtensions( if err != nil { return "", nil, err } - return details.SystemPrompt, details.ToolChoice, nil + // Single-string callers (telegram, previews) get the dynamic tail folded + // back on at the end — same order the split assembler produces. + return details.SystemPrompt + details.SystemPromptTail, details.ToolChoice, nil } func prepareChatContextDetailsWithExtensions( @@ -120,25 +129,29 @@ func prepareChatContextDetailsWithCache( forceRelevantMemory := shouldForceMemoryToolCall(userMessage) extSnapshot = filterSkillSnapshotForProject(extSnapshot, workspaceDir) - // Cache-first strategy: check cache before expensive memory search - if cached, ok := cache.Get(userMessage, sessionID); ok { - return buildContextFromResult(workspaceDir, cached, extSnapshot, invokedSkill, forceRelevantMemory), nil - } - - memService := buildSemanticMemoryService(workspaceDir, semanticCfg) - buildResult := prompt.BuildResultFor(prompt.BuildOptions{ + buildOpts := prompt.BuildOptions{ WorkspaceDir: workspaceDir, WorkDirs: workDirs, CurrentDir: currentDir, PlanClarifyMode: planClarifyMode, Query: userMessage, SessionID: sessionID, - MemorySearcher: memService, ForceRelevantMemory: forceRelevantMemory, - }) + } + + // Cache-first strategy: reuse the recall payload to skip the expensive + // semantic search, but always reassemble the prompt from the live options + // so a cache hit and a cache miss produce byte-identical output. + if cached, ok := cache.Get(userMessage, sessionID); ok { + buildOpts.PresetRelevant = cached.Preset() + return buildContextFromResult(workspaceDir, prompt.BuildResultFor(buildOpts), extSnapshot, invokedSkill, forceRelevantMemory), nil + } + + buildOpts.MemorySearcher = buildSemanticMemoryService(workspaceDir, semanticCfg) + buildResult := prompt.BuildResultFor(buildOpts) // Populate cache with search result - cache.Put(userMessage, sessionID, buildResult) + cache.Put(userMessage, sessionID, memoryRecallFromResult(buildResult)) return buildContextFromResult(workspaceDir, buildResult, extSnapshot, invokedSkill, forceRelevantMemory), nil } @@ -150,7 +163,7 @@ func buildContextFromResult( invokedSkill *skill.Definition, forceRelevantMemory bool, ) preparedChatContext { - systemPrompt := buildResult.Prompt + systemPrompt := buildResult.StaticPrompt systemPrompt += "\n" + strings.TrimSpace(memoryToolSystemRule) + "\n" skillPrompt := skillPromptForChatContext(workspaceDir, extSnapshot) if strings.TrimSpace(skillPrompt) != "" { @@ -175,8 +188,9 @@ func buildContextFromResult( } return preparedChatContext{ SystemPrompt: systemPrompt, + SystemPromptTail: buildResult.DynamicTail, ToolChoice: toolChoice, - SystemPromptTokens: promptTokenEstimate(systemPrompt), + SystemPromptTokens: promptTokenEstimate(systemPrompt + buildResult.DynamicTail), RelevantMemoryCount: buildResult.RelevantMemoryCount, RelevantMemoryTokens: buildResult.RelevantTokens, RelevantMemorySection: buildResult.RelevantSection, @@ -252,14 +266,40 @@ func buildLLMMessages(systemPrompt string, history []session.Message, userMessag } func buildLLMMessagesWithBlocks(systemPrompt string, history []session.Message, userMessage string, contentBlocks []llm.ContentBlock) []llm.ChatMessage { - llmMessages := make([]llm.ChatMessage, 0, len(history)+2) + return buildLLMMessagesWithTail(systemPrompt, "", history, userMessage, contentBlocks) +} + +// buildLLMMessagesWithTail emits the turn-stable prompt and the per-turn tail +// as two adjacent system messages. Providers concatenate system messages in +// order, so the rendered prompt is unchanged — but keeping them separate lets +// the Anthropic client put its cache_control breakpoint at the end of the +// stable block instead of after the volatile one, where it could never hit. +// An empty tail collapses back to a single system message. +func buildLLMMessagesWithTail(systemPrompt, systemTail string, history []session.Message, userMessage string, contentBlocks []llm.ContentBlock) []llm.ChatMessage { + llmMessages := make([]llm.ChatMessage, 0, len(history)+3) llmMessages = append(llmMessages, llm.ChatMessage{Role: "system", Content: systemPrompt}) + if strings.TrimSpace(systemTail) != "" { + llmMessages = append(llmMessages, llm.ChatMessage{Role: "system", Content: systemTail}) + } llmMessages = append(llmMessages, buildLLMMessageHistory(history)...) msg := llm.ChatMessage{Role: "user", Content: userMessage, ContentBlocks: contentBlocks} llmMessages = append(llmMessages, msg) return llmMessages } +// systemPromptTokens estimates the whole system prompt, which the assembler +// may have split across a stable message and a per-turn tail. +func systemPromptTokens(msgs []llm.ChatMessage) int { + total := 0 + for _, msg := range msgs { + if !strings.EqualFold(strings.TrimSpace(msg.Role), "system") { + break + } + total += promptTokenEstimate(msg.Content) + } + return total +} + // insertSystemMessageBeforeUser inserts an extra system-role message // immediately before the final user message in msgs. If no user message is // found (defensive) the system message is appended to the end. The original @@ -1428,6 +1468,8 @@ func newChatAPIHandlerWithRuntimeConfig( } style := effectiveSessionStyle(tooling.StyleDefaults, sess.StyleControl) systemPrompt += formatSessionStylePrompt(style, sess.AutomationConsent) + // Mirror the live assembler: the per-turn tail closes the prompt. + systemPrompt += contextDetails.SystemPromptTail registry := buildChatToolRegistry( reqStore, "", sessionID, requestWorkspaceDir, previewPolicy, historySnapshot.Messages, chatHandlerDeps{ workspaceDir: workspaceDir, diff --git a/internal/tarsserver/handler_chat_context.go b/internal/tarsserver/handler_chat_context.go index 16a2a083..f90450ea 100644 --- a/internal/tarsserver/handler_chat_context.go +++ b/internal/tarsserver/handler_chat_context.go @@ -203,7 +203,11 @@ func buildSessionChatRunState( systemPrompt += hint } - llmMessages := buildLLMMessagesWithBlocks(systemPrompt, history, userMessage, contentBlocks) + // contextDetails.SystemPromptTail closes the assembled prompt: everything + // appended above (skills, override, style, goal, critic, mention hints) is + // stable for the session, so it belongs ahead of the per-turn recall and + // clock. See the ordering invariant on prompt.BuildResultFor. + llmMessages := buildLLMMessagesWithTail(systemPrompt, contextDetails.SystemPromptTail, history, userMessage, contentBlocks) // Drain any pending critic feedback queued by the async reviewer on a // previous turn. Injected as a system-role message right before the // current user message so the LLM treats it as authoritative direction. diff --git a/internal/tarsserver/handler_chat_pipeline.go b/internal/tarsserver/handler_chat_pipeline.go index 5ca49651..817a6ec6 100644 --- a/internal/tarsserver/handler_chat_pipeline.go +++ b/internal/tarsserver/handler_chat_pipeline.go @@ -108,7 +108,7 @@ func handleChatRequest(w http.ResponseWriter, r *http.Request, deps chatHandlerD // Emit context info for frontend monitoring stream.contextInfo(map[string]any{ - "system_prompt_tokens": promptTokenEstimate(state.llmMessages[0].Content), + "system_prompt_tokens": systemPromptTokens(state.llmMessages), "history_tokens": sumHistoryTokens(state.history), "history_messages": len(state.history), "tool_count": len(state.injectedSchemas), diff --git a/internal/tarsserver/handler_chat_prefetch.go b/internal/tarsserver/handler_chat_prefetch.go index 76752a4e..62d1a01d 100644 --- a/internal/tarsserver/handler_chat_prefetch.go +++ b/internal/tarsserver/handler_chat_prefetch.go @@ -89,7 +89,11 @@ func startMemoryPrefetchForNextTurn( ForceRelevantMemory: shouldForceMemoryToolCall(userMessage), }) if result.RelevantMemoryCount > 0 { - cache.Put(userMessage, sessionID, result) + // Only the recall payload is cached — the prompt this goroutine + // assembled is discarded. It was built without the session's work + // dirs or current dir, so storing it would hand the next turn a + // prompt missing whole sections; the live path rebuilds instead. + cache.Put(userMessage, sessionID, memoryRecallFromResult(result)) } }() } diff --git a/internal/tarsserver/handler_chat_prompt_cache_test.go b/internal/tarsserver/handler_chat_prompt_cache_test.go new file mode 100644 index 00000000..3ec93b50 --- /dev/null +++ b/internal/tarsserver/handler_chat_prompt_cache_test.go @@ -0,0 +1,140 @@ +package tarsserver + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/devlikebear/tars/internal/extensions" + "github.com/devlikebear/tars/internal/llm" + "github.com/devlikebear/tars/internal/memory" + "github.com/devlikebear/tars/internal/prompt" + "github.com/devlikebear/tars/internal/session" +) + +func writePromptCacheWorkspace(t *testing.T) string { + t.Helper() + root := t.TempDir() + files := map[string]string{ + "IDENTITY.md": "# IDENTITY.md\n\nName: TARS", + "USER.md": "# USER.md\n\nName: Alice", + "MEMORY.md": "User prefers black coffee.\n", + } + for name, content := range files { + if err := os.WriteFile(filepath.Join(root, name), []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + return root +} + +// A cache hit must assemble the same prompt as a cache miss. Caching the +// assembled prompt instead of the recall payload broke this: the prefetch +// goroutine builds without the session's work dirs, so replaying its prompt +// dropped "## Working Directories" and handed the provider a different prefix. +func TestPrepareChatContext_CacheHitRebuildsLiveStaticRegion(t *testing.T) { + root := writePromptCacheWorkspace(t) + query := "what coffee do i prefer?" + workDirs := []string{filepath.Join(root, "artifacts")} + currentDir := workDirs[0] + + // Simulate the prefetch goroutine, which knows nothing about work dirs. + prefetched := prompt.BuildResultFor(prompt.BuildOptions{ + WorkspaceDir: root, + Query: query, + }) + if prefetched.RelevantMemoryCount == 0 { + t.Fatal("expected the fixture workspace to produce recall") + } + if strings.Contains(prefetched.Prompt, "## Working Directories") { + t.Fatal("fixture invalid: prefetch prompt should lack the work-dir section") + } + + cache := newMemoryCache(time.Minute) + cache.Put(query, "sess1", memoryRecallFromResult(prefetched)) + + hit, err := prepareChatContextDetailsWithCache( + root, "sess1", query, extensions.Snapshot{}, nil, + cache, memory.SemanticConfig{}, workDirs, currentDir, "smart", + ) + if err != nil { + t.Fatalf("prepare on cache hit: %v", err) + } + if !strings.Contains(hit.SystemPrompt, "## Working Directories") { + t.Fatalf("cache hit dropped the work-dir section: %q", hit.SystemPrompt) + } + if hit.RelevantMemoryCount != prefetched.RelevantMemoryCount { + t.Fatalf("expected cached recall to be reused, got %d want %d", hit.RelevantMemoryCount, prefetched.RelevantMemoryCount) + } + + miss, err := prepareChatContextDetailsWithCache( + root, "sess2", query, extensions.Snapshot{}, nil, + newMemoryCache(time.Minute), memory.SemanticConfig{}, workDirs, currentDir, "smart", + ) + if err != nil { + t.Fatalf("prepare on cache miss: %v", err) + } + if hit.SystemPrompt != miss.SystemPrompt { + t.Fatalf("cache hit and miss disagree:\nhit=%q\nmiss=%q", hit.SystemPrompt, miss.SystemPrompt) + } +} + +// The per-turn region has to be handed over separately so the assembler can +// keep it behind its own static sections. +func TestPrepareChatContext_SplitsStaticPromptFromDynamicTail(t *testing.T) { + root := writePromptCacheWorkspace(t) + + details, err := prepareChatContextDetailsWithCache( + root, "sess1", "what coffee do i prefer?", extensions.Snapshot{}, nil, + newMemoryCache(time.Minute), memory.SemanticConfig{}, nil, "", "smart", + ) + if err != nil { + t.Fatalf("prepare: %v", err) + } + if strings.Contains(details.SystemPrompt, "Current time:") { + t.Fatalf("expected no clock in the stable region: %q", details.SystemPrompt) + } + if !strings.Contains(details.SystemPromptTail, "Current time:") { + t.Fatalf("expected the clock in the tail: %q", details.SystemPromptTail) + } + if !strings.Contains(details.SystemPromptTail, "## Prior Context") { + t.Fatalf("expected recall in the tail: %q", details.SystemPromptTail) + } + // The memory-tool rule and skill policy are session-stable and must stay + // ahead of the tail. + if !strings.Contains(details.SystemPrompt, "## Memory Tool Policy") { + t.Fatalf("expected the memory tool rule in the stable region: %q", details.SystemPrompt) + } +} + +func TestBuildLLMMessagesWithTail_EmitsTailAsTrailingSystemMessage(t *testing.T) { + history := []session.Message{{Role: "user", Content: "earlier"}, {Role: "assistant", Content: "reply"}} + + msgs := buildLLMMessagesWithTail("stable", "## Current Time\n\nCurrent time: 2026-08-22T10:23:00Z", history, "now", nil) + + if len(msgs) != 5 { + t.Fatalf("expected system+tail+2 history+user, got %d: %+v", len(msgs), msgs) + } + if msgs[0].Role != "system" || msgs[0].Content != "stable" { + t.Fatalf("expected the stable system message first, got %+v", msgs[0]) + } + if msgs[1].Role != "system" || !strings.Contains(msgs[1].Content, "Current time:") { + t.Fatalf("expected the tail as the second system message, got %+v", msgs[1]) + } + if msgs[len(msgs)-1].Role != "user" || msgs[len(msgs)-1].Content != "now" { + t.Fatalf("expected the user message last, got %+v", msgs[len(msgs)-1]) + } +} + +func TestBuildLLMMessagesWithTail_EmptyTailKeepsSingleSystemMessage(t *testing.T) { + msgs := buildLLMMessagesWithTail("stable", " ", nil, "now", []llm.ContentBlock{}) + + if len(msgs) != 2 { + t.Fatalf("expected system+user, got %d: %+v", len(msgs), msgs) + } + if msgs[0].Role != "system" || msgs[1].Role != "user" { + t.Fatalf("unexpected roles: %+v", msgs) + } +} diff --git a/internal/tarsserver/memory_cache.go b/internal/tarsserver/memory_cache.go index 09e08a0c..a30c995b 100644 --- a/internal/tarsserver/memory_cache.go +++ b/internal/tarsserver/memory_cache.go @@ -12,8 +12,44 @@ import ( const defaultMemoryCacheTTL = 5 * time.Minute +// memoryRecall is the only part of a prompt build worth caching across turns: +// the semantic search behind "## Prior Context". The assembled prompt itself is +// deliberately NOT cached — its static region depends on live inputs (session +// work dirs, current dir, plan-clarify mode, workspace bootstrap files) that +// the prefetch path does not carry, so replaying a stored prompt used to drop +// whole sections and hand the provider a different prefix on a cache hit than +// on a cache miss. Rebuilding from live options with this payload injected +// makes hit and miss byte-identical by construction. +type memoryRecall struct { + Section string + Items []prompt.RelevantMemoryItem + Tokens int + Count int + Budget int +} + +// Preset converts the cached recall into the builder's injection shape. +func (r memoryRecall) Preset() *prompt.PresetRelevantMemory { + return &prompt.PresetRelevantMemory{ + Section: r.Section, + Items: r.Items, + Tokens: r.Tokens, + } +} + +// memoryRecallFromResult extracts the cacheable recall payload from a build. +func memoryRecallFromResult(result prompt.BuildResult) memoryRecall { + return memoryRecall{ + Section: result.RelevantSection, + Items: append([]prompt.RelevantMemoryItem(nil), result.RelevantMemoryItems...), + Tokens: result.RelevantTokens, + Count: result.RelevantMemoryCount, + Budget: result.RelevantBudgetTokens, + } +} + type memoryCacheEntry struct { - Result prompt.BuildResult + Recall memoryRecall CreatedAt time.Time } @@ -33,34 +69,34 @@ func newMemoryCache(ttl time.Duration) *memoryCache { } } -func (c *memoryCache) Get(query, sessionID string) (prompt.BuildResult, bool) { +func (c *memoryCache) Get(query, sessionID string) (memoryRecall, bool) { if c == nil { - return prompt.BuildResult{}, false + return memoryRecall{}, false } key := memoryCacheKey(query, sessionID) c.mu.RLock() entry, ok := c.entries[key] c.mu.RUnlock() if !ok { - return prompt.BuildResult{}, false + return memoryRecall{}, false } if time.Since(entry.CreatedAt) > c.ttl { c.mu.Lock() delete(c.entries, key) c.mu.Unlock() - return prompt.BuildResult{}, false + return memoryRecall{}, false } - return entry.Result, true + return entry.Recall, true } -func (c *memoryCache) Put(query, sessionID string, result prompt.BuildResult) { +func (c *memoryCache) Put(query, sessionID string, recall memoryRecall) { if c == nil { return } key := memoryCacheKey(query, sessionID) c.mu.Lock() c.entries[key] = memoryCacheEntry{ - Result: result, + Recall: recall, CreatedAt: time.Now(), } c.mu.Unlock() diff --git a/internal/tarsserver/memory_cache_test.go b/internal/tarsserver/memory_cache_test.go index 3f19dd86..f3c8cc10 100644 --- a/internal/tarsserver/memory_cache_test.go +++ b/internal/tarsserver/memory_cache_test.go @@ -9,22 +9,43 @@ import ( func TestMemoryCache_PutGet(t *testing.T) { cache := newMemoryCache(5 * time.Minute) - result := prompt.BuildResult{ + cache.Put("coffee preference", "sess1", memoryRecallFromResult(prompt.BuildResult{ Prompt: "test prompt", + RelevantSection: "## Prior Context\n\n- likes coffee\n", RelevantMemoryCount: 3, RelevantTokens: 120, - } - cache.Put("coffee preference", "sess1", result) + })) got, ok := cache.Get("coffee preference", "sess1") if !ok { t.Fatal("expected cache hit") } - if got.RelevantMemoryCount != 3 { - t.Fatalf("expected 3 relevant memories, got %d", got.RelevantMemoryCount) + if got.Count != 3 { + t.Fatalf("expected 3 relevant memories, got %d", got.Count) + } + if got.Section != "## Prior Context\n\n- likes coffee\n" { + t.Fatalf("expected cached recall section, got %q", got.Section) + } + if got.Tokens != 120 { + t.Fatalf("expected 120 recall tokens, got %d", got.Tokens) + } +} + +// The cache deliberately stores only the recall payload: the assembled prompt +// depends on live session inputs the prefetch path never sees, so keeping it +// would let a cache hit ship a different prompt than a cache miss. +func TestMemoryCache_StoresRecallOnly(t *testing.T) { + recall := memoryRecallFromResult(prompt.BuildResult{ + Prompt: "assembled prompt that must not be reused", + RelevantSection: "## Prior Context\n\n- fact\n", + RelevantTokens: 10, + }) + preset := recall.Preset() + if preset.Section != "## Prior Context\n\n- fact\n" { + t.Fatalf("unexpected preset section %q", preset.Section) } - if got.Prompt != "test prompt" { - t.Fatalf("expected cached prompt, got %q", got.Prompt) + if preset.Tokens != 10 { + t.Fatalf("unexpected preset tokens %d", preset.Tokens) } } @@ -38,7 +59,7 @@ func TestMemoryCache_Miss(t *testing.T) { func TestMemoryCache_TTLExpiry(t *testing.T) { cache := newMemoryCache(50 * time.Millisecond) - cache.Put("query", "", prompt.BuildResult{RelevantMemoryCount: 1}) + cache.Put("query", "", memoryRecall{Count: 1}) // Should hit immediately _, ok := cache.Get("query", "") @@ -57,9 +78,9 @@ func TestMemoryCache_TTLExpiry(t *testing.T) { func TestMemoryCache_EvictExpired(t *testing.T) { cache := newMemoryCache(50 * time.Millisecond) - cache.Put("old", "", prompt.BuildResult{RelevantMemoryCount: 1}) + cache.Put("old", "", memoryRecall{Count: 1}) time.Sleep(60 * time.Millisecond) - cache.Put("new", "", prompt.BuildResult{RelevantMemoryCount: 2}) + cache.Put("new", "", memoryRecall{Count: 2}) // evictExpired was called by Put, old entry should be gone cache.mu.RLock() @@ -77,13 +98,13 @@ func TestMemoryCache_NilSafe(t *testing.T) { t.Fatal("expected miss on nil cache") } // Should not panic - cache.Put("query", "", prompt.BuildResult{}) + cache.Put("query", "", memoryRecall{}) cache.evictExpired() } func TestMemoryCache_CaseInsensitiveQuery(t *testing.T) { cache := newMemoryCache(5 * time.Minute) - cache.Put("Coffee Preference", "", prompt.BuildResult{RelevantMemoryCount: 1}) + cache.Put("Coffee Preference", "", memoryRecall{Count: 1}) _, ok := cache.Get("coffee preference", "") if !ok {