test: cover chatd compaction usage decision (AIGOV-585) - #28053
Draft
johnstcn wants to merge 1 commit into
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds test coverage for the three compaction-decision functions in chatd that had zero tests:
latestPromptUsage,shouldCompactPromptUsage, andcontextTokensFromUsage.AIGOV-585 hypothesized that chatd's token counting logic was incorrect — that it compared a cumulative sum of prompt tokens across all agentic-loop steps against the context window. The tests disprove this:
latestPromptUsagereturns the last persisted assistant message's usage, not a sum. The actual bug was in the aibridge streaming interceptor, which summed usage across SSE chunks and persisted inflated values (fixed inad100452d4).What's tested
TestLatestPromptUsage— pins that the compaction path reads the last step's usage (5,400), not a cumulative sum across steps (15,600). If someone wiresTotalUsageinto the compaction path as the issue suggested, this fails.TestShouldCompactPromptUsage— covers the threshold decision with the inflated value from the issue (417,012 → compacts), the correct value (6,000 → doesn't compact), cache token counting, and both disable guards (threshold=100, contextLimit=0).Plan / investigation notes
chatloop.go:993setsresult.usage = part.Usagefrom the per-stepStreamPartTypeFinishevent, not the accumulatedTotalUsagefromagent.go:544. chatd never calls fantasy'sAgentinterface.TotalUsageaccumulation inagent.go:544is only used for cost attribution, not context occupancy.ad100452d4fixed the real bug inaibridge/intercept/chatcompletions/streaming.go(cross-chunk usage summation for vLLM-style backends).dbMessageandwithUsagehelpers frommessage_conversion_test.go(same package).Generated by Coder Agents