-
Notifications
You must be signed in to change notification settings - Fork 5
fix: address runtime review follow-ups #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| using System.Collections.Concurrent; | ||
| using SharpClaw.Code.Protocol.Models; | ||
|
|
||
| namespace SharpClaw.Code.Runtime.Context; | ||
|
|
||
| /// <summary> | ||
| /// Caches assembled conversation history per session so follow-up turns can reuse | ||
| /// in-process transcript state without rereading the full event log. | ||
| /// </summary> | ||
| internal static class ConversationHistoryCache | ||
| { | ||
| internal const int MaxHistoryTokenBudget = 100_000; | ||
| private const int MaxCacheEntries = 100; | ||
| private static readonly ConcurrentDictionary<string, CacheEntry> Cache = new(StringComparer.Ordinal); | ||
|
|
||
| public static bool TryGet( | ||
| string workspaceRoot, | ||
| string sessionId, | ||
| int completedTurnSequence, | ||
| out IReadOnlyList<ChatMessage> history) | ||
| { | ||
| if (completedTurnSequence >= 0 | ||
| && Cache.TryGetValue(CreateKey(workspaceRoot, sessionId), out var entry) | ||
| && entry.CompletedTurnSequence == completedTurnSequence) | ||
| { | ||
| history = entry.History; | ||
| return true; | ||
| } | ||
|
|
||
| history = []; | ||
| return false; | ||
| } | ||
|
|
||
| public static void Store( | ||
| string workspaceRoot, | ||
| string sessionId, | ||
| int completedTurnSequence, | ||
| IReadOnlyList<ChatMessage> history) | ||
| { | ||
| Cache[CreateKey(workspaceRoot, sessionId)] = new CacheEntry(completedTurnSequence, [.. history]); | ||
| EvictOverflow(); | ||
| } | ||
|
|
||
| public static void StoreCompletedTurn(string workspaceRoot, string sessionId, ConversationTurn completedTurn) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(completedTurn); | ||
| if (completedTurn.SequenceNumber <= 0 || string.IsNullOrWhiteSpace(completedTurn.Output)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var previousSequence = completedTurn.SequenceNumber - 1; | ||
| IReadOnlyList<ChatMessage> priorHistory = []; | ||
| if (previousSequence > 0 | ||
| && !TryGet(workspaceRoot, sessionId, previousSequence, out priorHistory)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var updatedHistory = priorHistory | ||
| .Concat( | ||
| [ | ||
| CreateMessage("user", completedTurn.Input), | ||
| CreateMessage("assistant", completedTurn.Output), | ||
| ]) | ||
| .ToArray(); | ||
|
|
||
| Store( | ||
| workspaceRoot, | ||
| sessionId, | ||
| completedTurn.SequenceNumber, | ||
| ContextWindowManager.Truncate(updatedHistory, MaxHistoryTokenBudget)); | ||
| } | ||
|
|
||
| private static ChatMessage CreateMessage(string role, string text) | ||
| => new(role, [new ContentBlock(ContentBlockKind.Text, text, null, null, null, null)]); | ||
|
|
||
| private static string CreateKey(string workspaceRoot, string sessionId) | ||
| => $"{workspaceRoot}\u0000{sessionId}"; | ||
|
|
||
| private static void EvictOverflow() | ||
| { | ||
| if (Cache.Count <= MaxCacheEntries) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var overflowKeys = Cache.Keys | ||
| .OrderBy(static key => key, StringComparer.Ordinal) | ||
| .Take(Cache.Count - MaxCacheEntries) | ||
| .ToArray(); | ||
|
|
||
| foreach (var key in overflowKeys) | ||
| { | ||
| Cache.TryRemove(key, out _); | ||
| } | ||
| } | ||
|
|
||
| private sealed record CacheEntry(int CompletedTurnSequence, ChatMessage[] History); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -162,15 +162,19 @@ public async Task<PromptExecutionContext> AssembleAsync( | |||||||||||||
| // NOTE: This reads the full event log per turn, which scales linearly with session length. | ||||||||||||||
| // For long-running sessions, consider persisting a compacted message history in session metadata | ||||||||||||||
| // to avoid re-reading and re-assembling the full log on every prompt. | ||||||||||||||
|
||||||||||||||
| // NOTE: This reads the full event log per turn, which scales linearly with session length. | |
| // For long-running sessions, consider persisting a compacted message history in session metadata | |
| // to avoid re-reading and re-assembling the full log on every prompt. | |
| // Prefer the cached history for the previous turn when available; on a cache miss, | |
| // fall back to reading the full event log and re-assembling the history for caching. | |
| // The fallback path still scales linearly with session length for long-running sessions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EvictOverflow() evicts entries by lexicographic key order, which is unrelated to insertion time or recency and can even immediately evict the just-stored entry if its key sorts early. Consider tracking insertion/last-access (e.g., timestamp in CacheEntry or a separate queue) and evicting oldest/least-recently-used sessions instead.