Addressed nit improvements

This commit is contained in:
Dev Randalpura
2026-03-12 16:53:41 -05:00
parent 4c09d08187
commit d21a686640
2 changed files with 179 additions and 172 deletions

View File

@@ -46,7 +46,7 @@ export function SlicingMaxSizedBox<T>({
text = '...' + text.slice(-MAXIMUM_RESULT_DISPLAY_CHARACTERS);
}
}
if (maxLines) {
if (maxLines !== undefined) {
const hasTrailingNewline = text.endsWith('\n');
const contentText = hasTrailingNewline ? text.slice(0, -1) : text;
const lines = contentText.split('\n');
@@ -71,7 +71,7 @@ export function SlicingMaxSizedBox<T>({
};
}
if (Array.isArray(data) && !isAlternateBuffer && maxLines) {
if (Array.isArray(data) && !isAlternateBuffer && maxLines !== undefined) {
if (data.length > maxLines) {
// We will have a label from MaxSizedBox. Reserve space for it.
const targetLines = Math.max(1, maxLines - 1);

View File

@@ -18,184 +18,191 @@ import {
describe('toolLayoutUtils', () => {
describe('calculateToolContentMaxLines', () => {
it('returns undefined if availableTerminalHeight is undefined', () => {
const result = calculateToolContentMaxLines({
availableTerminalHeight: undefined,
isAlternateBuffer: false,
});
expect(result).toBeUndefined();
});
interface CalculateToolContentMaxLinesTestCase {
desc: string;
options: Parameters<typeof calculateToolContentMaxLines>[0];
expected: number | undefined;
}
it('returns maxLinesLimit if maxLinesLimit applies but availableTerminalHeight is undefined', () => {
const result = calculateToolContentMaxLines({
availableTerminalHeight: undefined,
isAlternateBuffer: false,
maxLinesLimit: 10,
});
expect(result).toBe(10);
});
const testCases: CalculateToolContentMaxLinesTestCase[] = [
{
desc: 'returns undefined if availableTerminalHeight is undefined',
options: {
availableTerminalHeight: undefined,
isAlternateBuffer: false,
},
expected: undefined,
},
{
desc: 'returns maxLinesLimit if maxLinesLimit applies but availableTerminalHeight is undefined',
options: {
availableTerminalHeight: undefined,
isAlternateBuffer: false,
maxLinesLimit: 10,
},
expected: 10,
},
{
desc: 'returns available space directly in constrained terminal (Standard mode)',
options: {
availableTerminalHeight: 2,
isAlternateBuffer: false,
},
expected: 0,
},
{
desc: 'returns available space directly in constrained terminal (ASB mode)',
options: {
availableTerminalHeight: 4,
isAlternateBuffer: true,
},
expected: 2,
},
{
desc: 'returns remaining space if sufficient space exists (Standard mode)',
options: {
availableTerminalHeight: 20,
isAlternateBuffer: false,
},
expected: 18,
},
{
desc: 'returns remaining space if sufficient space exists (ASB mode)',
options: {
availableTerminalHeight: 20,
isAlternateBuffer: true,
},
expected: 18,
},
];
it('returns available space directly in constrained terminal (Standard mode)', () => {
const availableTerminalHeight = 2; // Very small
const result = calculateToolContentMaxLines({
availableTerminalHeight,
isAlternateBuffer: false,
});
// Math.max(0, 2 - 2) = 0
expect(result).toBe(0);
});
it('returns available space directly in constrained terminal (ASB mode)', () => {
const availableTerminalHeight = 4; // Very small
const result = calculateToolContentMaxLines({
availableTerminalHeight,
isAlternateBuffer: true,
});
// Math.max(0, 4 - 2) = 2
expect(result).toBe(2);
});
it('returns remaining space if sufficient space exists (Standard mode)', () => {
const availableTerminalHeight = 20;
const result = calculateToolContentMaxLines({
availableTerminalHeight,
isAlternateBuffer: false,
});
// Math.max(0, 20 - 2) = 18
expect(result).toBe(18);
});
it('returns remaining space if sufficient space exists (ASB mode)', () => {
const availableTerminalHeight = 20;
const result = calculateToolContentMaxLines({
availableTerminalHeight,
isAlternateBuffer: true,
});
// Math.max(0, 20 - 2) = 18
expect(result).toBe(18);
it.each(testCases)('$desc', ({ options, expected }) => {
const result = calculateToolContentMaxLines(options);
expect(result).toBe(expected);
});
});
describe('calculateShellMaxLines', () => {
it('returns undefined when not constrained and is expandable', () => {
const result = calculateShellMaxLines({
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: 20,
constrainHeight: false,
isExpandable: true,
});
expect(result).toBeUndefined();
});
interface CalculateShellMaxLinesTestCase {
desc: string;
options: Parameters<typeof calculateShellMaxLines>[0];
expected: number | undefined;
}
it('returns ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD for ASB mode when availableTerminalHeight is undefined', () => {
const result = calculateShellMaxLines({
status: CoreToolCallStatus.Executing,
isAlternateBuffer: true,
isThisShellFocused: false,
availableTerminalHeight: undefined,
constrainHeight: true,
isExpandable: false,
});
expect(result).toBe(ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD);
});
const testCases: CalculateShellMaxLinesTestCase[] = [
{
desc: 'returns undefined when not constrained and is expandable',
options: {
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: 20,
constrainHeight: false,
isExpandable: true,
},
expected: undefined,
},
{
desc: 'returns ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD for ASB mode when availableTerminalHeight is undefined',
options: {
status: CoreToolCallStatus.Executing,
isAlternateBuffer: true,
isThisShellFocused: false,
availableTerminalHeight: undefined,
constrainHeight: true,
isExpandable: false,
},
expected: ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD,
},
{
desc: 'returns undefined for Standard mode when availableTerminalHeight is undefined',
options: {
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: undefined,
constrainHeight: true,
isExpandable: false,
},
expected: undefined,
},
{
desc: 'handles small availableTerminalHeight gracefully without overflow in Standard mode',
options: {
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: 2,
constrainHeight: true,
isExpandable: false,
},
expected: 0,
},
{
desc: 'handles small availableTerminalHeight gracefully without overflow in ASB mode',
options: {
status: CoreToolCallStatus.Executing,
isAlternateBuffer: true,
isThisShellFocused: false,
availableTerminalHeight: 6,
constrainHeight: true,
isExpandable: false,
},
expected: 4,
},
{
desc: 'handles negative availableTerminalHeight gracefully',
options: {
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: -5,
constrainHeight: true,
isExpandable: false,
},
expected: 0,
},
{
desc: 'returns maxLinesBasedOnHeight for focused ASB shells',
options: {
status: CoreToolCallStatus.Executing,
isAlternateBuffer: true,
isThisShellFocused: true,
availableTerminalHeight: 30,
constrainHeight: false,
isExpandable: false,
},
expected: 28,
},
{
desc: 'falls back to COMPLETED_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD for completed shells if space allows',
options: {
status: CoreToolCallStatus.Success,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: 100,
constrainHeight: true,
isExpandable: false,
},
expected: COMPLETED_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD,
},
{
desc: 'falls back to ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD for executing shells if space allows',
options: {
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: 100,
constrainHeight: true,
isExpandable: false,
},
expected: ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD,
},
];
it('returns undefined for Standard mode when availableTerminalHeight is undefined', () => {
const result = calculateShellMaxLines({
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: undefined,
constrainHeight: true,
isExpandable: false,
});
expect(result).toBeUndefined();
});
it('handles small availableTerminalHeight gracefully without overflow in Standard mode', () => {
const result = calculateShellMaxLines({
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: 2,
constrainHeight: true,
isExpandable: false,
});
// Math.max(0, 2 - 2) = 0
expect(result).toBe(0);
});
it('handles small availableTerminalHeight gracefully without overflow in ASB mode', () => {
const result = calculateShellMaxLines({
status: CoreToolCallStatus.Executing,
isAlternateBuffer: true,
isThisShellFocused: false,
availableTerminalHeight: 6,
constrainHeight: true,
isExpandable: false,
});
// Math.max(0, 6 - 2) = 4
expect(result).toBe(4);
});
it('handles negative availableTerminalHeight gracefully', () => {
const result = calculateShellMaxLines({
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: -5,
constrainHeight: true,
isExpandable: false,
});
expect(result).toBe(0);
});
it('returns maxLinesBasedOnHeight for focused ASB shells', () => {
const result = calculateShellMaxLines({
status: CoreToolCallStatus.Executing,
isAlternateBuffer: true,
isThisShellFocused: true,
availableTerminalHeight: 30,
constrainHeight: false,
isExpandable: false,
});
// 30 - 2 = 28
expect(result).toBe(28);
});
it('falls back to COMPLETED_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD for completed shells if space allows', () => {
const result = calculateShellMaxLines({
status: CoreToolCallStatus.Success,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: 100,
constrainHeight: true,
isExpandable: false,
});
expect(result).toBe(COMPLETED_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD);
});
it('falls back to ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD for executing shells if space allows', () => {
const result = calculateShellMaxLines({
status: CoreToolCallStatus.Executing,
isAlternateBuffer: false,
isThisShellFocused: false,
availableTerminalHeight: 100,
constrainHeight: true,
isExpandable: false,
});
expect(result).toBe(ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD);
it.each(testCases)('$desc', ({ options, expected }) => {
const result = calculateShellMaxLines(options);
expect(result).toBe(expected);
});
});
});