diff --git a/src/core/transform.ts b/src/core/transform.ts index 8404cd0..6c93298 100644 --- a/src/core/transform.ts +++ b/src/core/transform.ts @@ -153,17 +153,48 @@ export function maxCharsPerImage(cols: number): number { return cols * LINES_PER_IMAGE; } -/** Returns true iff image-compressing a text block of `textLen` chars would - * actually save tokens vs leaving it as text. Used as the gate before every - * image-encoding decision in transformRequest. +/** Returns true iff image-compressing a text block would actually save tokens + * vs leaving it as text. Used as the gate before every image-encoding + * decision in transformRequest. + * + * Pass the **actual text string** when possible — the function will + * soft-wrap-count visual rows to match what `renderTextToPngs` will + * actually produce. Newline-heavy content (low fill ratio) renders to + * *more* images than the naive `chars / charsPerImage` estimate, and + * using the looser estimate lets net-losing compressions through. + * + * Passing a `number` falls back to the looser chars-only estimate for + * back-compat with existing unit tests; production transform call sites + * should always pass the string. * * `cols` defaults to `DEFAULTS.cols` (100) so existing callers and unit * tests that pass only `textLen` keep working byte-identically at the * current atlas. New call sites should pass `o.cols` so a runtime * `--cols` override flows into the break-even math too. */ -export function isCompressionProfitable(textLen: number, cols: number = DEFAULTS.cols): boolean { - const charsPerImage = maxCharsPerImage(cols); - const estImages = Math.max(1, Math.ceil(textLen / charsPerImage)); +export function isCompressionProfitable( + textOrLen: string | number, + cols: number = DEFAULTS.cols, + imageCountCap?: number, +): boolean { + let estImages: number; + let textLen: number; + if (typeof textOrLen === 'string') { + // Row-aware: matches renderTextToPngs() image budgeting exactly. + estImages = estimateImageCount(textOrLen, cols); + textLen = textOrLen.length; + } else { + // Looser chars-only estimate. Assumes lines fill width — wrong for + // newline-heavy code/logs but kept for back-compat. + const charsPerImage = maxCharsPerImage(cols); + estImages = Math.max(1, Math.ceil(textOrLen / charsPerImage)); + textLen = textOrLen; + } + // For code paths that truncate before rendering (tool_results), the + // actual image cost is bounded by the cap — text savings are still + // measured against the full pre-truncation length. + if (imageCountCap !== undefined && imageCountCap > 0) { + estImages = Math.min(estImages, imageCountCap); + } const imageTokensCost = estImages * TOKENS_PER_IMAGE; const textTokensEquivalent = textLen / CHARS_PER_TOKEN; return imageTokensCost < textTokensEquivalent; @@ -1116,7 +1147,10 @@ export async function transformRequest( // usually 25-30 KB so it always passes (1 image @ 2500 tokens < 25000/4 = // 6250 text-equivalent tokens), but the check guards against the edge // case where a tiny tool docs + tiny static slab combine to <10k chars. - if (!isCompressionProfitable(combined.length, o.cols)) { + // Pass the full text so the gate uses row-aware image-count math (matches + // renderTextToPngs exactly — newline-heavy content renders to more images + // than the naive chars/charsPerImage estimate). + if (!isCompressionProfitable(combined, o.cols)) { info.reason = `not_profitable (slab=${combined.length} chars)`; bumpPassthrough(info, 'not_profitable'); return { body, info }; @@ -1212,13 +1246,13 @@ export async function transformRequest( processedExisting.push(blk); continue; } - if (!isCompressionProfitable(textLen, o.cols)) { + const reminderText = (blk as TextBlock).text; + if (!isCompressionProfitable(reminderText, o.cols)) { // Above threshold but image cost ≥ text cost. Net loss to compress. bumpPassthrough(info, 'not_profitable'); processedExisting.push(blk); continue; } - const reminderText = (blk as TextBlock).text; const { blocks: imgs, droppedChars, droppedCodepoints: dcp } = await textToImageBlocks(reminderText, o.cols); for (const img of imgs) { @@ -1273,7 +1307,7 @@ export async function transformRequest( if (inner.length < o.minToolResultChars) { bumpPassthrough(info, 'below_threshold'); rewritten.push(blk); - } else if (!isCompressionProfitable(inner.length, o.cols)) { + } else if (!isCompressionProfitable(inner, o.cols, o.maxImagesPerToolResult)) { bumpPassthrough(info, 'not_profitable'); rewritten.push(blk); } else { @@ -1316,7 +1350,7 @@ export async function transformRequest( newInner.push(ib as TextBlock | ImageBlock); continue; } - if (!isCompressionProfitable(innerText.length, o.cols)) { + if (!isCompressionProfitable(innerText, o.cols, o.maxImagesPerToolResult)) { bumpPassthrough(info, 'not_profitable'); newInner.push(ib as TextBlock | ImageBlock); continue; diff --git a/tests/render.test.ts b/tests/render.test.ts index 1651045..9f04f37 100644 --- a/tests/render.test.ts +++ b/tests/render.test.ts @@ -980,7 +980,10 @@ describe('transform', () => { }); it('keeps as text outside the image so cache_control stays stable', async () => { - const staticSlab = 'claude.md ground truth.\n'.repeat(2200); + // Dense slab (long single line) so the row-aware break-even gate + // greenlights compression. Same total chars as the old short-line + // fixture but profitable: 1 image @ 2500 < 52800/4 = 13200 text. + const staticSlab = 'claude.md ground truth. '.repeat(2200); const envBlock = "\nWorking directory: /tmp/parityproj\nIs directory a git repo: Yes\nPlatform: darwin\nToday's date: 2026-05-18\n"; const sys = staticSlab + '\n' + envBlock; @@ -1025,7 +1028,7 @@ describe('transform', () => { it('puts cache_control on the image only, never on the dynamic tail', async () => { const sys = - 'claude.md\n'.repeat(5000) + + 'x'.repeat(40000) + '\nWorking directory: /tmp/x\n\n' + '\n[ ] do thing\n'; const body = new TextEncoder().encode( @@ -1133,7 +1136,7 @@ describe('transform', () => { // The whole token-savings story collapses if the renderer is non- // deterministic, because identical system prompts on consecutive turns // would produce different image bytes → 0% cache hit. Guard rail. - const sys = 'claude.md\n'.repeat(5000); + const sys = 'x'.repeat(40000); const body = new TextEncoder().encode( JSON.stringify({ model: 'claude', @@ -1238,7 +1241,7 @@ describe('transform', () => { JSON.stringify({ model: 'claude', messages: [{ role: 'user', content: 'hi' }], - system: 'claude.md\n'.repeat(5000), + system: 'x'.repeat(40000), }), ); const { body: outBytes } = await transformRequest(body); @@ -1268,7 +1271,7 @@ describe('transform', () => { ], }, ], - system: 'claude.md\n'.repeat(5000), + system: 'x'.repeat(40000), }), ); const { body: outBytes, info } = await transformRequest(body); @@ -1303,7 +1306,7 @@ describe('transform', () => { content: [{ type: 'text', text: shortReminder }], }, ], - system: 'claude.md\n'.repeat(5000), + system: 'x'.repeat(40000), }), ); const { body: outBytes, info } = await transformRequest(body); @@ -1335,7 +1338,7 @@ describe('transform', () => { ], }, ], - system: 'claude.md\n'.repeat(5000), + system: 'x'.repeat(40000), }), ); const { body: outBytes, info } = await transformRequest(body); @@ -1370,7 +1373,7 @@ describe('transform', () => { ], }, ], - system: 'claude.md\n'.repeat(5000), + system: 'x'.repeat(40000), }), ); const { body: outBytes, info } = await transformRequest(body); @@ -1534,6 +1537,42 @@ describe('transform', () => { expect(isCompressionProfitable(10_001, 20)).toBe(false); }); + it('isCompressionProfitable(string): row-aware → newline-heavy sparse content (~5500 chars/img) rejected as net-loss', () => { + // Regression for the -69% dashboard bug: the number-arg form estimates + // by chars/charsPerImage which assumes uniform line-fill. That assumes + // 14100 chars/image but renderTextToPngs actually packs ~141 visual + // rows/image — newline-heavy code/logs hit row cap WAY before char cap. + // + // 50000 chars of `x.md\n` is 5000 short lines → 5000 rows / 141 = 36 + // images. 36 * 2500 = 90000 image tokens vs 50000/4 = 12500 text tokens. + // Massive net loss. Number-arg form would incorrectly accept (50000 chars + // / 14100 chars-per-img = 4 imgs → 10000 < 12500 → "profitable"). + const sparse = 'x.md\n'.repeat(10_000); + expect(isCompressionProfitable(sparse, 100)).toBe(false); + expect(isCompressionProfitable(sparse.length, 100)).toBe(true); // back-compat: looser estimate + }); + + it('isCompressionProfitable(string): row-aware → dense single-line content packs full-width and profits', () => { + // Same 50000 chars but as ONE line wraps to 100-char rows → 500 rows / 141 + // = 4 images. 4 * 2500 = 10000 image tokens vs 50000/4 = 12500 text → + // profitable. Both forms agree on dense content. + const dense = 'x'.repeat(50_000); + expect(isCompressionProfitable(dense, 100)).toBe(true); + expect(isCompressionProfitable(dense.length, 100)).toBe(true); + }); + + it('isCompressionProfitable(string, cols, cap): truncation cap lets 500KB log become profitable', () => { + // For tool_result paging — actual image cost is bounded by maxImagesPerToolResult + // while the SAVED text is the full pre-truncation length. Without cap we'd + // reject (50k rows = 355 images), with cap=10 we accept (10*2500=25000 vs + // 500000/4=125000 text → win by 100k). + const lines: string[] = []; + for (let i = 0; i < 10_000; i++) lines.push(`log entry ${i} payload`); + const log = lines.join('\n'); + expect(isCompressionProfitable(log, 100)).toBe(false); // 10k rows × 2500 way over + expect(isCompressionProfitable(log, 100, 10)).toBe(true); // capped, profits + }); + it('isCompressionProfitable: smaller-cell atlas (Cozette-shape) would let 16k blocks become 1-image wins (cols proxy)', () => { // True smaller-cell test would need to mock ATLAS_CELL_H. We use cols as // a proxy since CHARS_PER_IMAGE = cols × floor((1568−8)/cell_H) — doubling