Review Agent PRs That Cache With Unbounded Maps

A flash sale hit the catalog service at noon. Latency for product reads climbed past two seconds. An agent opened a pull request before the war room ended.

The title claimed a simple in-memory product cache. The diff touched one new module and two call sites. The reviewer almost approved it on size alone.

The change under review

The agent added a module-scoped Map named productCache. Each miss loaded a row and stored the whole record. No TTL, max size, or invalidation path existed.

Missing product ids were cached as null forever. Mutable row objects were returned to callers by reference. Price edits in admin never touched the Map.

// agent-generated PR fragment — review, do not merge
const productCache = new Map();

async function getProduct(db, id) {
  if (productCache.has(id)) {
    return productCache.get(id);
  }

  const rows = await db.query(
    'SELECT * FROM products WHERE id = $1',
    [id]
  );
  const product = rows[0] ?? null;
  productCache.set(id, product);
  return product;
}

async function updatePrice(db, id, price) {
  await db.query(
    'UPDATE products SET price = $1 WHERE id = $2',
    [price, id]
  );
  // no cache delete, no version bump, no TTL
}

This is a common agent performance patch pattern. The tests used three product fixtures and passed. Nothing in the suite spoke to key cardinality.

What to trust

Trust the diagnosis more than the chosen fix. Repeated catalog reads were a real hot path. Extracting a single lookup helper was a reasonable cleanup.

Trust a hit-path assertion against frozen expected JSON. Trust logs that print cache key count after load. Do not trust an unbounded Map as architecture.

What to revert

Revert these pieces before any merge discussion.

1. Revert the process global Map on multi-instance deploys.
2. Revert storage of live ORM objects without a clone.
3. Revert silent caching of null for missing identifiers.
4. Revert write paths that never delete or bump keys.
5. Revert cache keys built from unsanitized request parameters.

Each item is a production defect, not a nit. Stale prices during a sale are user-facing failures. A Map that grows with traffic is a memory leak.

Review workflow

Run this review in listed order without skips.

1. Name the key space

Write the cache key formula in the review comment. State the expected distinct key count at peak. Reject the PR if that number is unknown.

key = productId
peak distinct ids last week = REPLACE_WITH_METRICS
process replicas = REPLACE_WITH_DEPLOY

The numbers must come from production metrics. A local Map multiplies memory by replica count. It also splits hit ratio across those processes.

2. Demand an eviction policy in the diff

Accept only an explicit max size and TTL. True LRU eviction is a minimum acceptable bar. Document what happens when the cap is hit.

// proposal only — not production code
const MAX_ENTRIES = 10000;
const TTL_MS = 30000;

function evictIfNeeded(cache) {
  if (cache.size <= MAX_ENTRIES) {
    return;
  }
  const oldestKey = cache.keys().next().value;
  cache.delete(oldestKey);
}

Insertion-order eviction is not a true LRU policy. Call that limitation out in the review.

3. Require invalidation on every writer

List every function that mutates cached rows. Each writer must delete keys or bump a version. Missing one writer means silent stale catalog reads.

async function updatePrice(db, cache, id, price) {
  await db.query(
    'UPDATE products SET price = $1 WHERE id = $2',
    [price, id]
  );
  cache.delete(id);
}

This still fails across processes and nodes. Treat single-process delete as a local bandage.

4. Freeze returned values

Agents often cache the object they mutate later. A caller that sets product.price poisons the Map. Return a structured clone or a frozen copy.

function remember(cache, id, product) {
  const copy = structuredClone(product);
  Object.freeze(copy);
  cache.set(id, { value: copy, exp: Date.now() + TTL_MS });
  return structuredClone(copy);
}

Module scoped maps also break isolation between tests. Pass the cache into helpers during the review.

5. Test cardinality, not only correctness

Green unit tests with three rows prove almost nothing. Add a test that inserts far more keys than the cap. Add a test that updates a cached price and re-reads.

// proposal test — unexecuted until the suite runs
import assert from 'node:assert/strict';

async function testPriceUpdateBustsCache() {
  const cache = new Map();
  const db = makeFakeDb({ 7: { id: 7, price: 10 } });

  const first = await getProduct(db, cache, 7);
  await updatePrice(db, cache, 7, 12);
  const second = await getProduct(db, cache, 7);

  assert.equal(first.price, 10);
  assert.equal(second.price, 12);
}

Reproducible memory harness

The harness below is a local Node script. It is labeled unexecuted until a reviewer runs it. The script fills an unbounded Map and prints RSS.

// harness: cache-rss.mjs — run on a review box
// proposal / unexecuted until a reviewer runs it

const cache = new Map();
const PAYLOAD = JSON.stringify({
  id: 0,
  sku: 'sku-000000',
  title: 'x'.repeat(120),
  attrs: { color: 'red', size: 'm', extra: 'y'.repeat(80) },
});

function rssMb() {
  return (process.memoryUsage().rss / 1024 / 1024).toFixed(1);
}

const start = rssMb();
for (let i = 0; i < 200000; i++) {
  cache.set(i, JSON.parse(PAYLOAD.replace('0', String(i))));
  if (i % 50000 === 0) {
    console.log(JSON.stringify({ i: i, rssMb: rssMb(), size: cache.size }));
  }
}

console.log(JSON.stringify({
  startMb: start,
  endMb: rssMb(),
  keys: cache.size,
}));

Run the script with a hard memory ceiling.

node --max-old-space-size=256 cache-rss.mjs

A healthy review records the next three outcomes.

1. RSS should climb in step with distinct keys.
2. The unbounded script should die under 256 MB.
3. A capped Map should stay inside a stated budget.

Store the console JSON in the PR thread. Do not argue from intuition when RSS is cheap.

Decision table

Observation in the diff	Review action	Merge gate
Helper extraction only	Trust	Optional follow-up
Unbounded module Map	Revert	Block
Null cached forever	Revert	Block
No writer invalidation	Revert	Block
Mutable object returned	Revert	Block
TTL plus max size	Test	Load proof required
Replica-local cache	Test	Document split brain
Shared Redis or Memcached	Test	Auth, TTL, stampede

Use the table in the first review comment. Do not negotiate away the listed block rows.

Running the same review on a spare box

Some teams generate several agent patches overnight. Each patch needs the same RSS and invalidation checks. A spare box keeps that loop off a laptop.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

MonkeyCode offers free model access and a free server option. Reviewers can regenerate cache patches against the same failing test. The checklist above stays the source of truth.

Teams without a local review box may run the harness there. That is optional and does not replace replica-aware caching.

Limitations

This workflow catches process-local leaks and stale reads. It does not prove cache correctness under multi-node writes. RSS samples are noisy on shared or burstable hosts.

Insertion-order eviction is not LRU under random access. The structuredClone call fails on some driver objects. SELECT * still over-fetches even with a perfect cache.

Free hosted runners share noisy neighbors and CPU. Treat their RSS numbers as directional, not contractual. No duration, model name, or quota is assumed here.

Who should not use this approach

Do not ship in-process caches for prices or stock. Do not cache personal data in a process Map. Do not use this harness as a capacity plan.

Multi-region catalogs need a shared store with TTL. PCI and health data need encryption and strict eviction. Teams without a measured key cardinality should refuse the PR.

Close

Agent cache patches look small and feel kind. They hide latency by moving it into memory growth. Review the key space, writers, and RSS before merge.

The next agent PR will likely repeat this Map. Keep the decision table next to the review template. Merge only after eviction, invalidation, and a memory gate.