[codex] add result export formats#36
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a reusable export serializer for query results (CSV/TSV/JSON/Markdown), updates the SQL editor UI from a single “Export to CSV” button to an export menu, and extends documentation and audit logging to reflect the selected export format.
Changes:
- Added
serializeExport()supporting CSV, TSV, JSON, and Markdown, plusEXPORT_FORMATSmetadata and a generalizedexportRows()download helper. - Updated SQL editor results toolbar to an export menu and included the chosen export format in
auditExport. - Added focused unit tests for the serializer and updated relevant docs (SQL editor, access control, audit log).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/export.test.ts |
Adds unit coverage for CSV/TSV/JSON/Markdown serialization behavior. |
src/lib/export-csv.ts |
Implements multi-format serialization and export/download plumbing, retaining exportToCsv as a convenience wrapper. |
src/components/sql-editor/QueryResults.tsx |
Replaces the export button with a menu and logs the selected format in audit events. |
docs/features/sql-editor.mdx |
Updates SQL editor documentation to describe multi-format export. |
docs/features/database-access-control.mdx |
Updates the export permission UI description from button to menu. |
docs/features/audit-log.mdx |
Documents the expanded set of possible export formats in audit logs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR extends the SQL editor's export functionality from CSV-only to a four-format menu (CSV, TSV, JSON, Markdown) by introducing a unified
Confidence Score: 4/5Safe to merge; the change is additive and well-contained with no impact on existing CSV behaviour. The serialization logic is cleanly separated, the existing exportToCsv wrapper preserves backward compatibility, and the new formats are exercised by dedicated tests. The two notable gaps are that formatMarkdownCell does not strip bare src/lib/export-csv.ts — specifically the formatMarkdownCell function and the double EXPORT_FORMATS.find() pattern. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant Menu as Export Menu
participant Handler as handleExport
participant Serializer as serializeExport
participant Browser as Browser Download
participant Audit as auditExport
User->>Menu: Click Download button
Menu-->>User: Show format options (CSV / TSV / JSON / Markdown)
User->>Menu: Select format
Menu->>Handler: handleExport(format)
Handler->>Handler: Build filename with extension
Handler->>Serializer: exportRows(columns, rows, filename, format)
Serializer->>Serializer: serializeExport to format-specific serializer
Serializer->>Browser: downloadText(content, filename, mimeType)
Handler->>Audit: auditExport with format
Audit-->>Handler: fire-and-forget
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant Menu as Export Menu
participant Handler as handleExport
participant Serializer as serializeExport
participant Browser as Browser Download
participant Audit as auditExport
User->>Menu: Click Download button
Menu-->>User: Show format options (CSV / TSV / JSON / Markdown)
User->>Menu: Select format
Menu->>Handler: handleExport(format)
Handler->>Handler: Build filename with extension
Handler->>Serializer: exportRows(columns, rows, filename, format)
Serializer->>Serializer: serializeExport to format-specific serializer
Serializer->>Browser: downloadText(content, filename, mimeType)
Handler->>Audit: auditExport with format
Audit-->>Handler: fire-and-forget
Reviews (1): Last reviewed commit: "feat: add result export formats" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function formatMarkdownCell(value: unknown): string { | ||
| if (value === null) return 'null' | ||
| if (value === undefined) return '' | ||
| return String(value) | ||
| .replace(/\|/g, '\\|') | ||
| .replace(/\r?\n/g, ' ') | ||
| } |
There was a problem hiding this comment.
\r not sanitized in Markdown cells
formatMarkdownCell replaces \r?\n (CRLF or LF) with a space, but a bare carriage return (\r) without a following newline is left unchanged. Some Markdown renderers treat a bare \r as a line break, which can corrupt the table row. Consider broadening the replacement to /\r?\n|\r/g.
| function formatMarkdownCell(value: unknown): string { | ||
| if (value === null) return 'null' | ||
| if (value === undefined) return '' | ||
| return String(value) | ||
| .replace(/\|/g, '\\|') | ||
| .replace(/\r?\n/g, ' ') | ||
| } |
There was a problem hiding this comment.
formatMarkdownCell only escapes pipe characters. Cell values containing Markdown formatting tokens — *, _, `, [, ], or \ — will be interpreted as inline Markdown rather than literal text. For example, a DB value of **status** exports as bold text. If the intent is a plain-text data dump, escaping these characters would prevent unintended formatting.
| export function exportRows( | ||
| columns: Array<Pick<ColumnMetadata, 'name'>>, | ||
| rows: Record<string, unknown>[], | ||
| filename: string, | ||
| format: ExportFormat | ||
| ) { | ||
| const exportFormat = EXPORT_FORMATS.find((item) => item.value === format) | ||
| if (!exportFormat) return | ||
|
|
||
| downloadText(serializeExport(columns, rows, format), filename, exportFormat.mimeType) | ||
| } |
There was a problem hiding this comment.
EXPORT_FORMATS.find() is called twice per export
handleExport calls EXPORT_FORMATS.find() to get the file extension, then exportRows calls it again for the MIME type. Since ExportFormat is a discriminated union and always a known value at call sites, the second lookup is redundant. Consider passing the metadata object directly into exportRows to make the contract explicit.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| return String(value) | ||
| .replace(/\|/g, '\\|') | ||
| .replace(/\r?\n/g, ' ') |
There was a problem hiding this comment.
and but leaves bare unchanged, which some Markdown renderers treat as a line break inside the table cell.
| return String(value) | |
| .replace(/\|/g, '\\|') | |
| .replace(/\r?\n/g, ' ') | |
| return String(value) | |
| .replace(/\|/g, '\\|') | |
| .replace(/\r?\n|\r/g, ' ') |
Summary
Validation
pnpm test tests/export.test.tspnpm exec tsc -p tsconfig.app.json --noEmitpnpm exec eslint src/lib/export-csv.ts tests/export.test.tsgit diff --checkpnpm buildNote: focused linting that includes
QueryResults.tsxis not a clean signal today because that file already has unrelated React compiler / hooks findings onmain. The new serializer and tests lint cleanly.