feat: improve token counting and add brevity pattern#7
Open
PatrickRuddiman wants to merge 3 commits intomainfrom
Open
feat: improve token counting and add brevity pattern#7PatrickRuddiman wants to merge 3 commits intomainfrom
PatrickRuddiman wants to merge 3 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements a new brevity pattern for handling context overflow scenarios, refactors token estimation to improve accuracy using TiktokenSharp, and enhances the chunk summarization logic in the OpenAIService.
- Introduces a new constant and pattern file for brevity summarization.
- Implements token estimation improvements through a dedicated TokenHelper class and integrates it into other services.
- Refactors OpenAIService to re-chunk messages when context tokens exceed the limit.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| patterns/brief_chunk_summary/system.md | Adds a new pattern file describing the brevity summarization process. |
| WriteCommit.csproj | Adds dependency to TiktokenSharp for enhanced token estimation. |
| Services/TokenHelper.cs | Implements token estimation using TiktokenSharp with fallback logic. |
| Services/SemanticCoherenceAnalyzer.cs | Refactors token estimation to use the new TokenHelper for better accuracy. |
| Services/OpenAIService.cs | Enhances chunk summarization logic with re-chunking of messages for context overflow. |
| README.md | Updates documentation to include usage of the brevity pattern. |
| Constants/PatternNames.cs | Adds a constant for the new brevity pattern. |
| Constants/FabricPatterns.cs | Adds a constant for the new brevity pattern. |
|
|
||
| var groupedSummaries = new List<string>(); | ||
| var currentGroup = new List<string>(); | ||
| var currentTokens = TokenHelper.EstimateTokens(systemPrompt, model); |
There was a problem hiding this comment.
Since systemPrompt remains unchanged throughout the method, consider caching its token estimate once rather than repeatedly recalculating it to improve maintainability.
Suggested change
| var currentTokens = TokenHelper.EstimateTokens(systemPrompt, model); | |
| var currentTokens = systemPromptTokens; |
Owner
Author
|
this may not be needed; ill do more testing |
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
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.
This pull request introduces a new brevity pattern to handle context overflow scenarios, improves token estimation accuracy, and refactors the codebase to support these changes. The most important updates include defining the brevity pattern, implementing token estimation using
TiktokenSharp, and enhancing the chunk summarization logic inOpenAIService.New Pattern for Context Overflow:
Constants/FabricPatterns.csandConstants/PatternNames.cs: Added a new constantBrevityPatternto define the pattern used for summarization when context overflow occurs. [1] [2]patterns/brief_chunk_summary/system.md: Introduced a new pattern file that outlines the purpose, steps, and output for the brevity summarization process.Token Estimation Improvements:
Services/TokenHelper.cs: Added a newTokenHelperclass usingTiktokenSharpfor accurate token estimation, with fallback logic for cases where encoding fails.WriteCommit.csproj: IncludedTiktokenSharpas a new dependency for token estimation.Services/SemanticCoherenceAnalyzer.cs: Updated theEstimateTokenCountmethod to useTokenHelperfor better accuracy.Enhanced Chunk Summarization Logic:
Services/OpenAIService.cs: Introduced logic to handle context overflow by re-chunking summaries and using the newBrevityPatternfor condensed summarization. Added a constantMaxContextTokensto define the token limit. [1] [2]Documentation Update:
README.md: Documented the newbrief_chunk_summarypattern and its usage for handling overflowing contexts.