Skip to content

feat(utils): add cache-aware chat history compression and compaction - #1303

Open
gugu8intel-i9 wants to merge 2 commits into
CodebuffAI:mainfrom
gugu8intel-i9:main
Open

feat(utils): add cache-aware chat history compression and compaction#1303
gugu8intel-i9 wants to merge 2 commits into
CodebuffAI:mainfrom
gugu8intel-i9:main

Conversation

@gugu8intel-i9

Copy link
Copy Markdown

Add src/utils/chat-compression.ts for context window compaction and history pruning. When conversations approach the limits of a model's token budget, this utility replaces a contiguous middle slice of message history with a structured synthetic summary, preserving critical conversational invariants.

Refactor comments and documentation in chat-compression.ts for clarity.
Refactor chat compression utility with improved type annotations and documentation. Update compression logic to enhance clarity and maintainability.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@codebuff-team

Copy link
Copy Markdown
Contributor

The instinct here is good — cache-aware compaction that avoids invalidating a hot prefix (via minFreshInputRatio/freshInputRatio) is a real, non-obvious problem, and the tool-pair boundary alignment (alignToPairBoundary) shows you understand why naive slicing breaks tool call/result pairing.

But as submitted this isn't reviewable as a port candidate:

  1. Line 8 literally says // ← verify this specifier on the import from @codebuff/common/util/tokens. That means you don't know if this compiles against the real package, and neither do we without checking. A PR shouldn't ship an admission that its own imports are unverified.
  2. Nothing calls compressChat/compressChatToFit/shouldCompress anywhere. This is 554 lines of new surface area with no call site, so there's no way to tell whether the token-budget math, the tail-boundary logic, or the interaction with lastUsage/totalTokens actually behaves correctly against real message shapes used elsewhere in the CLI.
  3. No tests. Given the amount of arithmetic here (trigger/target ratios, tail budget math, multi-pass convergence in compressChatToFit), this is exactly the kind of code where the repo would expect unit tests on findTailStart, measureContext, and the compress/no-op paths — regressions here would silently blow context budgets or thrash the cache in production.
  4. It duplicates functionality that likely already exists somewhere in the private tree for context management — worth checking before building a parallel implementation, since the two need to agree on message shape and usage semantics.

To make this portable: confirm the import, wire it into an actual call site (even a small one), add unit tests for the boundary/threshold logic, and drop the placeholder comment.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written labels Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants