Merge pull request #513 from alectimison-maker/refactor/extract-image-budget

refactor(agent): extract image budget helpers

Author: esokullu

TODOs.md

@@ -194,15 +194,15 @@ parity regressions likely.
 
 ## 9. Test the real shared logic instead of copied shims
 
-**Status:** Partially resolved. Loop detection now lives in the browser-free
-`agent/loop-detector.js` module in both builds. `Agent` inherits that production
-class, `test/run.js` imports it directly, and a parity assertion keeps the
-Chrome and Firefox copies byte-identical.
+**Status:** Partially resolved. Loop detection and image-budget sizing now live
+in browser-free modules in both builds. `Agent` consumes the production logic,
+`test/run.js` imports it directly, and parity assertions keep the Chrome and
+Firefox copies byte-identical.
 
 **Concrete next steps:**
-1. Move image budget sizing and other remaining pure logic into browser-free
-   modules and import those modules directly from `agent.js` and `test/run.js`.
-2. Add regression tests for the text tool-call parser and context trimming,
+1. Move other remaining pure logic into browser-free modules and import those
+   modules directly from `agent.js` and `test/run.js`.
+2. Add broader regression tests for the text tool-call parser and context trimming,
    since both are high-impact agent reliability code.
 
 ---

src/chrome/ARCHITECTURE.md

@@ -42,6 +42,7 @@ src/chrome/
 │   ├── agent/
 │   │   ├── agent.js            # Core agent loop + tool dispatch
 │   │   ├── loop-detector.js     # Browser-free loop detection, directly unit-tested
+│   │   ├── image-budget.js      # Browser-free screenshot sizing, directly unit-tested
 │   │   ├── mutation-tools.js    # This build's state-change + mutating tool sets
 │   │   ├── tools.js            # Tool schemas + system prompts
 │   │   ├── skills.js           # Settings skills + dynamic skill tool manifests

src/chrome/src/agent/agent.js

@@ -2,6 +2,7 @@ import { AGENT_TOOLS, AGENT_TOOL_NAMES, RESERVED_AGENT_TOOL_NAMES, getToolsForMo
 import { handleDoneJson } from './cloud-output.js';
 import { LoopDetector } from './loop-detector.js';
 import { parseToolCallsFromText } from './tool-call-parser.js';
+import { IMAGE_BUDGET, estimateImageTokens, fitImageDimensions } from './image-budget.js';
 import { BROWSER_MUTATION_TOOLS, STATE_CHANGE_TOOLS as SHARED_STATE_CHANGE_TOOLS } from './mutation-tools.js';
 import { isCredentialField, CREDENTIAL_NOTE_STRICT, STRICT_SECRET_SYSTEM_NOTE } from './credential-fields.js';
 import { detectProgressAction, formatLedgerRow, formatLedgerSummary, isBlockedLedgerDowngrade, isTerminalLedgerStatus, isValidLedgerStatus, ledgerDoneBlock, ledgerRowKey, normalizeLedgerStatus, progressCounts, selectLedgerRows, unresolvedLedgerRows, upsertLedgerItems } from './progress-ledger.js';
@@ -5747,23 +5748,15 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
   // are tuned to Claude's native vision encoder and happen to be
   // reasonable for most other endpoints too. Override per-capture if you
   // need sharper (coord_aligned) or looser (full_page) constraints.
-  static IMAGE_BUDGET = {
-    pxPerToken: 28,        // rough px² per vision token across providers
-    maxTargetPx: 1568,     // no dimension bigger than this
-    maxTargetTokens: 1568, // total image tokens budget
-    maxBase64Chars: 1398100, // ~1.4 MB base64, matches Anthropic's cap
-    initialJpegQuality: 0.75,
-    minJpegQuality: 0.10,
-    jpegQualityStep: 0.05,
-  };
+  static IMAGE_BUDGET = IMAGE_BUDGET;
 
   /**
    * Anthropic's token-cost approximation: ceil((w*h) / pxPerToken²).
    * Good enough to compare two capture sizes under the same budget; not
    * exact for any specific provider's tokenizer, but better than eyeballing.
    */
   static _estimateImageTokens(w, h, pxPerToken) {
-    return Math.ceil((w / pxPerToken) * (h / pxPerToken));
+    return estimateImageTokens(w, h, pxPerToken);
   }
 
   /**
@@ -5773,33 +5766,7 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
    * Claude-for-Chrome's `C(w, h, params)` (same algorithm, clearer names).
    */
   static _fitImageDimensions(origW, origH, budget = Agent.IMAGE_BUDGET) {
-    const { pxPerToken, maxTargetPx, maxTargetTokens } = budget;
-    // Already fits — no work.
-    if (origW <= maxTargetPx && origH <= maxTargetPx &&
-        Agent._estimateImageTokens(origW, origH, pxPerToken) <= maxTargetTokens) {
-      return [origW, origH];
-    }
-    // Search the long side; the other follows from aspect ratio.
-    if (origH > origW) {
-      const [h, w] = Agent._fitImageDimensions(origH, origW, budget);
-      return [w, h];
-    }
-    const aspect = origW / origH;
-    let hi = origW, lo = 1;
-    // eslint-disable-next-line no-constant-condition
-    while (true) {
-      if (lo + 1 >= hi) {
-        return [lo, Math.max(Math.round(lo / aspect), 1)];
-      }
-      const mid = Math.floor((lo + hi) / 2);
-      const midH = Math.max(Math.round(mid / aspect), 1);
-      if (mid <= maxTargetPx &&
-          Agent._estimateImageTokens(mid, midH, pxPerToken) <= maxTargetTokens) {
-        lo = mid;
-      } else {
-        hi = mid;
-      }
-    }
+    return fitImageDimensions(origW, origH, budget);
   }
 
   /**

src/chrome/src/agent/image-budget.js

@@ -0,0 +1,52 @@
+// Browser-free image sizing shared by Agent and the unit suite. This file is
+// mirrored in the Firefox tree; keep both copies byte-identical.
+
+// These defaults match Claude for Chrome. Pixel dimensions and approximate
+// vision-token area bound provider cost, while the byte/quality fields are
+// consumed by each browser's platform-specific image encoder. Frozen so an
+// accidental mutation of the shared default throws instead of silently
+// changing every caller; per-capture overrides spread into a fresh object.
+export const IMAGE_BUDGET = Object.freeze({
+  pxPerToken: 28,
+  maxTargetPx: 1568,
+  maxTargetTokens: 1568,
+  maxBase64Chars: 1398100,
+  initialJpegQuality: 0.75,
+  minJpegQuality: 0.10,
+  jpegQualityStep: 0.05,
+});
+
+export function estimateImageTokens(w, h, pxPerToken) {
+  return Math.ceil((w / pxPerToken) * (h / pxPerToken));
+}
+
+// Return the largest dimensions no greater than the original that fit both
+// the per-side and token-area caps while preserving the aspect ratio.
+export function fitImageDimensions(origW, origH, budget = IMAGE_BUDGET) {
+  const { pxPerToken, maxTargetPx, maxTargetTokens } = budget;
+  if (origW <= maxTargetPx && origH <= maxTargetPx
+      && estimateImageTokens(origW, origH, pxPerToken) <= maxTargetTokens) {
+    return [origW, origH];
+  }
+  if (origH > origW) {
+    const [h, w] = fitImageDimensions(origH, origW, budget);
+    return [w, h];
+  }
+  const aspect = origW / origH;
+  let hi = origW;
+  let lo = 1;
+  // eslint-disable-next-line no-constant-condition
+  while (true) {
+    if (lo + 1 >= hi) {
+      return [lo, Math.max(Math.round(lo / aspect), 1)];
+    }
+    const mid = Math.floor((lo + hi) / 2);
+    const midH = Math.max(Math.round(mid / aspect), 1);
+    if (mid <= maxTargetPx
+        && estimateImageTokens(mid, midH, pxPerToken) <= maxTargetTokens) {
+      lo = mid;
+    } else {
+      hi = mid;
+    }
+  }
+}

src/firefox/ARCHITECTURE.md

@@ -55,6 +55,7 @@ src/firefox/
 │   ├── agent/
 │   │   ├── agent.js                # Core agent loop
 │   │   ├── loop-detector.js        # Browser-free loop detection, directly unit-tested
+│   │   ├── image-budget.js         # Browser-free screenshot sizing, directly unit-tested
 │   │   ├── mutation-tools.js       # This build's state-change + mutating tool sets
 │   │   ├── tools.js                # Tool schemas + system prompts (incl. 4 AX tools)
 │   │   ├── skills.js               # Settings skills + dynamic skill tool manifests

src/firefox/src/agent/agent.js

@@ -2,6 +2,7 @@ import { AGENT_TOOLS, AGENT_TOOL_NAMES, RESERVED_AGENT_TOOL_NAMES, getToolsForMo
 import { handleDoneJson } from './cloud-output.js';
 import { LoopDetector } from './loop-detector.js';
 import { parseToolCallsFromText } from './tool-call-parser.js';
+import { IMAGE_BUDGET, estimateImageTokens, fitImageDimensions } from './image-budget.js';
 import { BROWSER_MUTATION_TOOLS, STATE_CHANGE_TOOLS as SHARED_STATE_CHANGE_TOOLS } from './mutation-tools.js';
 import { isCredentialField, CREDENTIAL_NOTE_STRICT, STRICT_SECRET_SYSTEM_NOTE } from './credential-fields.js';
 import { detectProgressAction, formatLedgerRow, formatLedgerSummary, isBlockedLedgerDowngrade, isTerminalLedgerStatus, isValidLedgerStatus, ledgerDoneBlock, ledgerRowKey, normalizeLedgerStatus, progressCounts, selectLedgerRows, unresolvedLedgerRows, upsertLedgerItems } from './progress-ledger.js';
@@ -4062,44 +4063,14 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
   // and can use regular <canvas> / toBlob instead of OffscreenCanvas.
   // Constants match Anthropic's Claude-for-Chrome defaults and the Chrome
   // Agent's IMAGE_BUDGET — keep them in sync.
-  static IMAGE_BUDGET = {
-    pxPerToken: 28,
-    maxTargetPx: 1568,
-    maxTargetTokens: 1568,
-    maxBase64Chars: 1398100,
-    initialJpegQuality: 0.75,
-    minJpegQuality: 0.10,
-    jpegQualityStep: 0.05,
-  };
+  static IMAGE_BUDGET = IMAGE_BUDGET;
 
   static _estimateImageTokens(w, h, pxPerToken) {
-    return Math.ceil((w / pxPerToken) * (h / pxPerToken));
+    return estimateImageTokens(w, h, pxPerToken);
   }
 
   static _fitImageDimensions(origW, origH, budget = Agent.IMAGE_BUDGET) {
-    const { pxPerToken, maxTargetPx, maxTargetTokens } = budget;
-    if (origW <= maxTargetPx && origH <= maxTargetPx &&
-        Agent._estimateImageTokens(origW, origH, pxPerToken) <= maxTargetTokens) {
-      return [origW, origH];
-    }
-    if (origH > origW) {
-      const [h, w] = Agent._fitImageDimensions(origH, origW, budget);
-      return [w, h];
-    }
-    const aspect = origW / origH;
-    let hi = origW, lo = 1;
-    // eslint-disable-next-line no-constant-condition
-    while (true) {
-      if (lo + 1 >= hi) return [lo, Math.max(Math.round(lo / aspect), 1)];
-      const mid = Math.floor((lo + hi) / 2);
-      const midH = Math.max(Math.round(mid / aspect), 1);
-      if (mid <= maxTargetPx &&
-          Agent._estimateImageTokens(mid, midH, pxPerToken) <= maxTargetTokens) {
-        lo = mid;
-      } else {
-        hi = mid;
-      }
-    }
+    return fitImageDimensions(origW, origH, budget);
   }
 
   /**

src/firefox/src/agent/image-budget.js

@@ -0,0 +1,52 @@
+// Browser-free image sizing shared by Agent and the unit suite. This file is
+// mirrored in the Firefox tree; keep both copies byte-identical.
+
+// These defaults match Claude for Chrome. Pixel dimensions and approximate
+// vision-token area bound provider cost, while the byte/quality fields are
+// consumed by each browser's platform-specific image encoder. Frozen so an
+// accidental mutation of the shared default throws instead of silently
+// changing every caller; per-capture overrides spread into a fresh object.
+export const IMAGE_BUDGET = Object.freeze({
+  pxPerToken: 28,
+  maxTargetPx: 1568,
+  maxTargetTokens: 1568,
+  maxBase64Chars: 1398100,
+  initialJpegQuality: 0.75,
+  minJpegQuality: 0.10,
+  jpegQualityStep: 0.05,
+});
+
+export function estimateImageTokens(w, h, pxPerToken) {
+  return Math.ceil((w / pxPerToken) * (h / pxPerToken));
+}
+
+// Return the largest dimensions no greater than the original that fit both
+// the per-side and token-area caps while preserving the aspect ratio.
+export function fitImageDimensions(origW, origH, budget = IMAGE_BUDGET) {
+  const { pxPerToken, maxTargetPx, maxTargetTokens } = budget;
+  if (origW <= maxTargetPx && origH <= maxTargetPx
+      && estimateImageTokens(origW, origH, pxPerToken) <= maxTargetTokens) {
+    return [origW, origH];
+  }
+  if (origH > origW) {
+    const [h, w] = fitImageDimensions(origH, origW, budget);
+    return [w, h];
+  }
+  const aspect = origW / origH;
+  let hi = origW;
+  let lo = 1;
+  // eslint-disable-next-line no-constant-condition
+  while (true) {
+    if (lo + 1 >= hi) {
+      return [lo, Math.max(Math.round(lo / aspect), 1)];
+    }
+    const mid = Math.floor((lo + hi) / 2);
+    const midH = Math.max(Math.round(mid / aspect), 1);
+    if (mid <= maxTargetPx
+        && estimateImageTokens(mid, midH, pxPerToken) <= maxTargetTokens) {
+      lo = mid;
+    } else {
+      hi = mid;
+    }
+  }
+}

test/run.js

@@ -453,6 +453,16 @@ const ToolCallParserCh = await import(
 const ToolCallParserFx = await import(
   'file://' + path.join(ROOT, 'src/firefox/src/agent/tool-call-parser.js').replace(/\\/g, '/')
 );
+const ImageBudgetCh = await import(
+  'file://' + path.join(ROOT, 'src/chrome/src/agent/image-budget.js').replace(/\\/g, '/')
+);
+const ImageBudgetFx = await import(
+  'file://' + path.join(ROOT, 'src/firefox/src/agent/image-budget.js').replace(/\\/g, '/')
+);
+const {
+  estimateImageTokens,
+  fitImageDimensions,
+} = ImageBudgetCh;
 // The mutating-tool surface differs per build, so it lives outside the
 // byte-identical loop-detector module. Import the real sets rather than
 // restating them here — a hand-copied list is exactly the drift this suite
@@ -5939,50 +5949,25 @@ test('failed API replay suppresses future bulk replay hints until a success clea
     }
   }
 });
-//
-// These mirror the static helpers on Agent exactly — keep them in sync
-// with src/chrome/src/agent/agent.js `_estimateImageTokens` and
-// `_fitImageDimensions`. We shim rather than import because agent.js
-// pulls in chrome.* and cdp-client.
-// ────────────────────────────────────────────────────────────────────────
-
-const IMAGE_BUDGET_DEFAULT = {
-  pxPerToken: 28,
-  maxTargetPx: 1568,
-  maxTargetTokens: 1568,
-};
-
-function estimateImageTokens(w, h, pxPerToken) {
-  return Math.ceil((w / pxPerToken) * (h / pxPerToken));
-}
-
-function fitImageDimensions(origW, origH, budget = IMAGE_BUDGET_DEFAULT) {
-  const { pxPerToken, maxTargetPx, maxTargetTokens } = budget;
-  if (origW <= maxTargetPx && origH <= maxTargetPx &&
-      estimateImageTokens(origW, origH, pxPerToken) <= maxTargetTokens) {
-    return [origW, origH];
-  }
-  if (origH > origW) {
-    const [h, w] = fitImageDimensions(origH, origW, budget);
-    return [w, h];
-  }
-  const aspect = origW / origH;
-  let hi = origW, lo = 1;
-  while (true) {
-    if (lo + 1 >= hi) return [lo, Math.max(Math.round(lo / aspect), 1)];
-    const mid = Math.floor((lo + hi) / 2);
-    const midH = Math.max(Math.round(mid / aspect), 1);
-    if (mid <= maxTargetPx &&
-        estimateImageTokens(mid, midH, pxPerToken) <= maxTargetTokens) {
-      lo = mid;
-    } else {
-      hi = mid;
-    }
-  }
-}
-
 console.log('\nimage budget');
 
+test('image budget helpers are production code shared by both browser agents', () => {
+  assert.equal(
+    fs.readFileSync(path.join(ROOT, 'src/chrome/src/agent/image-budget.js'), 'utf8'),
+    fs.readFileSync(path.join(ROOT, 'src/firefox/src/agent/image-budget.js'), 'utf8'),
+    'chrome and firefox image-budget modules must remain byte-identical',
+  );
+  assert.deepEqual(ImageBudgetFx.IMAGE_BUDGET, ImageBudgetCh.IMAGE_BUDGET);
+  assert.equal(
+    ImageBudgetFx.estimateImageTokens(1920, 1080, 28),
+    estimateImageTokens(1920, 1080, 28),
+  );
+  assert.deepEqual(
+    ImageBudgetFx.fitImageDimensions(3840, 2160),
+    fitImageDimensions(3840, 2160),
+  );
+});
+
 test('small viewport passes through unchanged (fast path)', () => {
   // 1280×800 at pxPerToken=28 → 1307 tokens < 1568 — no resize.
   const [w, h] = fitImageDimensions(1280, 800);