fix(core): keep compactString within its limit when the marker does not fit (#7872)

* fix(core): keep compactString within its limit when the marker does not fit

The compaction marker embeds the original length, so it runs 60-80
characters on its own. `contentBudget` was clamped to 0 when the limit was
smaller than that, but the marker was then appended regardless:

  compactStringForRecording('x'.repeat(70), 60) -> 79 chars
  compactStringForHistory('x'.repeat(64), 63)   -> 64 chars
  compactStringForHistory('x'.repeat(100), 50)  -> 65 chars

The first returns more characters than the input it was asked to compact,
which is the opposite of what a compaction function is for. Both entry
points are exported and take a caller-supplied limit.

When the marker cannot fit alongside any content there is nothing useful to
announce, so hard-truncate the head instead of explaining the truncation at
greater length than the truncated text. The marker is unchanged wherever the
limit leaves room for it.

* test(core): fix the purpose ternary and cover surrogates on the guard path

Two review findings on the compaction tests.

The `it.each` over both purposes called `compactStringForRecording` in both
branches, so the two `history` rows silently re-ran the recording case. The
history marker is 65 characters against recording's 79, so the two purposes
take different content budgets and the history budget was never exercised.

The `marker.length >= limit` guard also had no surrogate coverage: it slices
without a marker, so it has a boundary of its own, and both existing
surrogate-aware tests run at the default limit and take the head+marker+tail
path instead. Added four rows over both purposes at limits 9 and 8. Replacing
`safeHeadEnd(value, limit)` with a bare `limit` fails the two odd-limit rows
and leaves the even-limit controls green, so the coverage is load-bearing.

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
This commit is contained in:
chinesepowered 2026-07-31 01:46:11 -07:00 committed by GitHub
parent 90052f25ae
commit d9f5b2e288
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 91 additions and 0 deletions

View file

@ -381,3 +381,82 @@ describe('toolResultDisplayCompaction', () => {
expect(compactedTeam.teamName).toContain('truncated from');
});
});
describe('compactString limit', () => {
// The compaction marker embeds the original length, so it is 60-80
// characters on its own. It used to be appended whatever the limit was,
// which meant a small caller-supplied limit got back more than it asked
// for -- and sometimes more than the string it was given.
it.each([
['recording' as const, 70, 60],
['history' as const, 64, 63],
['history' as const, 100, 50],
['recording' as const, 200, 10],
['history' as const, 40, 0],
])(
'keeps %s output within bounds for input %d at limit %d',
(purpose, inputLength, limit) => {
const value = 'x'.repeat(inputLength);
const compact =
purpose === 'recording'
? compactStringForRecording(value, limit)
: compactStringForHistory(value, limit);
expect(compact.length).toBeLessThanOrEqual(limit);
// Compacting must never hand back more characters than it was given.
expect(compact.length).toBeLessThanOrEqual(value.length);
},
);
// Guards against over-correcting: when the limit does leave room for the
// marker, the marker must still be there. These pass before and after.
it.each([
['recording' as const, 5000, 500],
['history' as const, 5000, 200],
['history' as const, 5000, 120],
])(
'still explains the truncation for %s at input %d, limit %d',
(purpose, inputLength, limit) => {
const value = 'x'.repeat(inputLength);
const compact =
purpose === 'recording'
? compactStringForRecording(value, limit)
: compactStringForHistory(value, limit);
expect(compact.length).toBeLessThanOrEqual(limit);
expect(compact).toContain('truncated');
},
);
// The `marker.length >= limit` path slices without a marker, so it has a
// boundary of its own to get right. The two surrogate-aware tests above both
// run at the default limit and take the head+marker+tail path, so neither
// reaches this one.
it.each([
['history' as const, 9],
['history' as const, 8],
['recording' as const, 9],
['recording' as const, 8],
])(
'does not split a surrogate pair when the marker does not fit, for %s at limit %d',
(purpose, limit) => {
const value = '😀'.repeat(40);
const compact =
purpose === 'recording'
? compactStringForRecording(value, limit)
: compactStringForHistory(value, limit);
expect(compact.length).toBeLessThanOrEqual(limit);
expect(hasUnpairedSurrogate(compact)).toBe(false);
// A whole number of pairs survived, so the cut backed off to a boundary
// rather than landing between a high and low surrogate.
expect(compact.length % 2).toBe(0);
// Confirms this really is the marker-does-not-fit path.
expect(compact).not.toContain('truncated');
},
);
it('returns a short string untouched regardless of the marker length', () => {
expect(compactStringForHistory('short', 1000)).toBe('short');
});
});

View file

@ -84,6 +84,18 @@ function compactString(
}
const marker = buildStringCompactionMarker(value, purpose);
// The marker is 60-80 characters and was appended whatever the limit was,
// so a caller-supplied limit below that got back more than it asked for --
// and for a limit under the input length, more characters than it passed
// in. `compactStringForRecording('x'.repeat(70), 60)` returned 79. When the
// marker cannot fit alongside any content there is nothing to announce, so
// hard-truncate instead of explaining the truncation at greater length than
// the truncated text.
if (marker.length >= limit) {
return copyString(value.slice(0, safeHeadEnd(value, Math.max(0, limit))));
}
const contentBudget = Math.max(0, limit - marker.length);
const headLength = Math.ceil(contentBudget * 0.6);
const tailLength = contentBudget - headLength;