Review Agent PRs That Search by Loading Every Row

A reviewer opened an agent pull request on Monday morning.
The description claimed a fast order search endpoint.

The diff looked small, typed, and fully tested.
Green CI sat on a single happy-path fixture.

The route accepted q, status, and from parameters.
The handler then loaded every order for the tenant.

The patch under review

The illustrative agent patch below is not production code.
It matches a pattern that shows up in generated list endpoints.

// illustrative agent patch — do not ship
app.get("/orders", auth, async (req, res) => {
  const { q, status, from } = req.query;
  const orders = await db.order.findMany({
    where: { tenantId: req.tenantId },
  });
  const filtered = orders.filter((o) => {
    const text = `${o.id} ${o.email} ${o.notes}`;
    const hit = !q || text.toLowerCase().includes(String(q).toLowerCase());
    const st = !status || o.status === status;
    const dt = !from || o.createdAt >= new Date(from);
    return hit && st && dt;
  });
  const result = [];
  for (const order of filtered) {
    const items = await db.item.findMany({ where: { orderId: order.id } });
    result.push({ ...order, items });
  }
  res.json({ orders: result, total: result.length });
});

The TypeScript types for the handler compile cleanly.
The handler uses the tenant id from the session.

That combination is the actual review trap.
Correct auth on the outer query hides a cost bomb.

What CI did not see

The fixture inserted only three tenant orders.
The assertion checked email match and item count.

// illustrative test from the same PR
it("finds an order by email fragment", async () => {
  await seedOrders(3);
  const res = await request(app)
    .get("/orders?q=buyer@")
    .set("Authorization", token);
  expect(res.status).toBe(200);
  expect(res.body.orders).toHaveLength(1);
});

Three rows never expose a full table scan.
Three extra item queries never trip a pool timeout.

The test proves a filter works on a toy set.
It does not prove the query plan stays bounded.

Agents often cite “tenant isolation already applied”.
Isolation without a cap still copies the whole tenant.

What to trust

Trust only artifacts that survive a hostile input size.

1. Keep tenant scoping on the first database predicate.
2. Keep auth middleware that rejects a missing token.
3. Keep stable response field names clients already consume.

Do not trust in-memory filters as access control.
Do not trust total when it counts an unbounded array.
Do not trust loops that fetch children per parent row.

A compile-clean select * equivalent is still a smell.
A mapped DTO does not shrink the rows first read.

What to revert

Revert the full table read before anything else.
Revert the JavaScript filter that copies every row.
Revert the per-order item query inside the loop.

Also revert silent casts of from through new Date.
Invalid dates become Invalid Date and fail closed or open.
Agents often pick the open path without a comment.

Keep the route path if the contract is already public.
Keep tenant isolation if a human already reviewed that helper.

Revert total: result.length on the same pass.
That number is a page size, not a table statistic.

Review workflow

Reviewers can run this sequence on any generated list PR.

1. Open the handler and list every findMany, select, or fetch.
2. Mark each call that lacks take, limit, or a cursor.
3. Mark each loop that performs another query or HTTP call.
4. Demand a failing test with a large fixture before merge.
5. Require the rewrite to push filters into the database.
6. Require a hard cap even after the database filter lands.

The next sections give a concrete test and a bounded rewrite.
Both are labeled proposals, not measured production results.

A failing test the agent will not write

Seed enough rows to make the naive path expensive.
Keep the seed local, disposable, and fully deterministic.

# proposed local seed — unexecuted in this article
npx prisma db push
node scripts/seed-orders.mjs --count 5000 --tenant t_demo
npx vitest run tests/orders-search.unbounded.test.js

// proposed test — unexecuted in this article
it("does not materialize the whole tenant order set", async () => {
  await seedOrders(5000);
  const start = Date.now();
  const res = await request(app)
    .get("/orders?q=needle-only-once&limit=20")
    .set("Authorization", token);
  const ms = Date.now() - start;
  expect(res.status).toBe(200);
  expect(res.body.orders.length).toBeLessThanOrEqual(20);
  expect(res.body.orders.length).toBeGreaterThan(0);
  expect(ms).toBeLessThan(500);
});

Five thousand rows is a floor, not a load test.
The assertion on duration is a tripwire, not a benchmark.

Tune the duration against the review machine, not a slogan.
Reject the PR if the handler has no limit argument at all.

Wall clocks flake, so count statements when logs exist.

// proposed — count statements instead of wall clock when possible
const queries = [];
db.$on("query", (e) => queries.push(e));
await request(app)
  .get("/orders?q=needle-only-once&limit=20")
  .set("Authorization", token);
expect(queries.filter((q) => /FROM\s+"Item"/i.test(q.query))).toHaveLength(1);
expect(queries.some((q) => /LIMIT/i.test(q.query))).toBe(true);

One child query for the page is the merge bar.
A child query per parent row still fails this gate.

Bounded rewrite to request

Push search predicates down into the query engine.
Cap the page size and load children in one query.

// proposed rewrite — review against your ORM
function parseStatus(value) {
  const allowed = new Set(["open", "paid", "canceled"]);
  if (value == null || value === "") return null;
  const status = String(value);
  if (!allowed.has(status)) {
    const err = new Error("invalid_status");
    err.statusCode = 400;
    throw err;
  }
  return status;
}

function parseDate(value) {
  if (value == null || value === "") return null;
  const ms = Date.parse(String(value));
  if (Number.isNaN(ms)) {
    const err = new Error("invalid_from");
    err.statusCode = 400;
    throw err;
  }
  return new Date(ms);
}

function decodeCursor(value) {
  if (!value) return null;
  if (!/^[a-z0-9_-]{8,64}$/i.test(String(value))) {
    const err = new Error("invalid_cursor");
    err.statusCode = 400;
    throw err;
  }
  return String(value);
}

app.get("/orders", auth, async (req, res) => {
  const q = String(req.query.q || "").slice(0, 64);
  const status = parseStatus(req.query.status);
  const from = parseDate(req.query.from);
  const limit = Math.min(Number(req.query.limit) || 20, 100);
  const cursor = decodeCursor(req.query.cursor);

  const orders = await db.order.findMany({
    where: {
      tenantId: req.tenantId,
      ...(status ? { status } : {}),
      ...(from ? { createdAt: { gte: from } } : {}),
      ...(q
        ? {
            OR: [
              { email: { contains: q, mode: "insensitive" } },
              { notes: { contains: q, mode: "insensitive" } },
            ],
          }
        : {}),
    },
    take: limit + 1,
    ...(cursor ? { cursor: { id: cursor }, skip: 1 } : {}),
    orderBy: [{ createdAt: "desc" }, { id: "desc" }],
    select: {
      id: true,
      email: true,
      status: true,
      notes: true,
      createdAt: true,
    },
  });

  const page = orders.slice(0, limit);
  const ids = page.map((o) => o.id);
  const items = ids.length
    ? await db.item.findMany({
        where: { orderId: { in: ids } },
        select: { id: true, orderId: true, sku: true, qty: true },
      })
    : [];
  const grouped = Map.groupBy(items, (row) => row.orderId);
  const next = orders.length > limit ? page[page.length - 1].id : null;

  res.json({
    orders: page.map((o) => ({ ...o, items: grouped.get(o.id) || [] })),
    nextCursor: next,
  });
});

The rewrite still needs an index on tenant, createdAt, and email.
Search on notes can still degrade without a dedicated strategy.

That is acceptable for a first merge bar.
It is not a replacement for a search product.

Reject a cursor that is not an identifier the tenant owns.
A leaked foreign id must not page another tenant’s table.

Decision table for the reviewer

Signal in the diff	Trust	Revert	Test
findMany with only tenantId	Isolation only	Unbounded read	Seed 5k rows
JS filter on query params	None	Whole filter	Invalid from
Loop plus findMany per id	None	N+1 loop	Statement count
total equals result.length	None	Fake totals	Page versus table
new Date(from) with no parse	None	Implicit cast	from=yesterday
limit missing from contract	None	Open page size	limit=100000
Cursor fields present	Shape only	Weak encoding	Tampered cursor

Read the table as a merge gate, not a style guide.
Any single red cell is enough to block.

Running the review loop on a free server

Human review still owns the final merge decision.
A hosted coding model can only draft the first pass.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.
MonkeyCode offers free model access and a free server option.
Those two facts are the only product claims used here.

A practical review loop looks like the following.

1. Paste the handler and the test file into the model.
2. Ask for unbounded reads, N+1 loops, and missing caps.
3. Ignore any suggested rewrite until a failing test exists.
4. Run that test on the free server against a local database.
5. Merge only after the bounded query and the cap both land.

Do not ask the model to certify performance.
Do not paste production dumps, tokens, or customer emails.

The free server is useful for the 5k-row fixture.
It is not a substitute for staging traffic replay.

Reviewers who need a disposable host can use that free server after tests exist.

Limitations

This workflow catches materialization bugs on list reads.
It does not catch missing object-level authorization on GET /orders/:id.

It does not replace EXPLAIN on the real engine.
It does not prove index quality under production cardinality.

Duration assertions often flake on noisy shared hosts.
Prefer counting rows read when the ORM exposes query logs.

Cursor pagination still needs a stable orderBy clause.
Ties on createdAt will skip or repeat rows without id.

Full-text search needs remain outside this merge bar.
Notes search with contains can still scan a large index.

ILIKE plus leading wildcards will ignore many btree indexes.
Treat that as a later search ticket, not a silent merge.

Who should not use this approach

Do not use the 5k-row tripwire as a capacity plan.
Platform teams with existing query budgets already have better gates.

Do not run this seed against shared staging data.
The fixture belongs on a disposable database volume.

Do not accept a model summary as evidence of safety.
The value is the failing test and the bounded SQL.

Skip the rewrite if the endpoint is an internal dump.
Those paths need a job, a file, and an explicit ACL.

Treat every unbounded list handler as unmerged until a cap exists.
The agent can draft the route. The reviewer owns the bound.