Fix critical bugs in string truncation and context window lookup - #1193
Closed
pavankumar-vh wants to merge 1 commit into
Closed
Fix critical bugs in string truncation and context window lookup#1193pavankumar-vh wants to merge 1 commit into
pavankumar-vh wants to merge 1 commit into
Conversation
Bug Fixes: 1. Fix context window lookup in base-chat.ts: Handle missing/undefined model correctly - Change: → - Prevents unnecessary lookup and improves clarity 2. Fix critical bug in truncateStringWithMessage: Prevent negative slice indices - Added Math.max(0, ...) guards to prevent negative slice lengths - Fixes potential runtime errors when maxLength < message length - Applies to all truncation modes (START, END, MIDDLE) 3. Add comprehensive tests for truncateStringWithMessage - Added 9 test cases covering edge cases - Tests for negative/zero available length scenarios - Tests for all truncation modes (START, END, MIDDLE) - Tests for custom messages and empty strings All changes are in approved contribution areas (agents/, common/) and improve code safety.
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.
Overview
Fix two critical bugs that could cause runtime errors and improve code safety.
Bug Fixes
1. Fix context window lookup in base-chat.ts
Issue: The code used
CONTEXT_WINDOWS[model ?? '']which converts undefined model to empty string and unnecessarily performs a lookup.Fix: Changed to
(model && CONTEXT_WINDOWS[model]) ?? DEFAULT_CONTEXT_WINDOWwhich:2. Fix critical bug in truncateStringWithMessage
Issue: When
maxLengthis smaller than the message length, the function calculates negative slice indices, causing unexpected behavior.Fix: Added
Math.max(0, ...)guards to prevent negative slice lengths:Math.max(0, maxLength - suffix.length)Math.max(0, maxLength - prefix.length)Math.max(0, Math.floor((maxLength - middle.length) / 2))This prevents potential runtime errors when truncating strings with very small maxLength values.
3. Add comprehensive tests for truncateStringWithMessage
Added 9 test cases covering:
Files Changed
agents/base-chat.ts- Improved context window lookup logiccommon/src/util/string.ts- Added safety guards to prevent negative slice indicescommon/src/util/__tests__/string.test.ts- Added comprehensive test coverageScope
This change only touches
agents/andcommon/which are approved contribution areas per the Contributing Guide.