From b0c1b2e7ce77c14d64d44d1d68589e0e198238bc Mon Sep 17 00:00:00 2001 From: Daoyuan Li <94409450+DaoyuanLi2816@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:48:32 -0700 Subject: [PATCH] fix(artifacts): handle CRLF splits in CSV/TSV previews (#6180) * fix(artifacts): handle truncated CRLF after quoted fields * Handle LF-first truncated CRLF artifact samples --- README.md | 2 + frontend/AGENTS.md | 6 ++ .../src/core/artifacts/delimited-preview.ts | 13 +++- .../tests/e2e/artifact-table-preview.spec.ts | 16 +++++ .../core/artifacts/delimited-preview.test.ts | 68 +++++++++++++++++++ 5 files changed, 102 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index d2dd52cdb4..91867a6b58 100644 --- a/README.md +++ b/README.md @@ -1865,6 +1865,8 @@ Files presented through `present_files` remain part of the thread's artifact sta CSV and TSV artifacts open as tables in the artifact panel and in a separate window. The preview preserves text values (including leading zeros), supports an optional header row, and pages through up to 200 rows and 50 columns from the initial sample. Long or multiline cells can be opened and copied in full. Switch to source to inspect or edit the file; downloads and separate windows use the saved version. +If the sample cuts a CRLF line ending in half, the preview keeps the earlier complete rows and omits the incomplete final record, including when its last field is quoted. + Text artifacts are streamed with HTTP byte-range support. The Web UI initially loads at most 1 MiB, shows the preview size when a file is larger, and waits for an explicit **Load full file** action before fetching the remainder or mounting diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index 29314ea431..8829fe15cb 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -194,6 +194,12 @@ lists from the server instead of inserting those snapshots into either view. CSV/TSV previews share `artifact-table-preview.tsx` between the panel and standalone viewer. Papa Parse runs only inside `delimited-preview.worker.ts`; `use-delimited-preview.ts` bounds input before transfer, cancels stale work, and enforces a five-second timeout. The parser detects the first record separator outside quoted fields and passes it explicitly to Papa Parse, so embedded newlines in an incomplete quoted field cannot corrupt newline detection. It retains at most 202 logical records and 50 columns, discarding an incomplete final record from truncated input. UI pagination displays at most 200 data rows in pages of 50. Keep the table mounted but inactive when switching to source so header/pagination state survives; changing file identity resets it. Pending `write_file` content stays in source mode until success. +For a truncated sample whose first separator is LF or CRLF, strip a terminal CR +before parsing so a split CRLF separator cannot make a quoted final field +invalidate the whole preview, including LF-first files with later CRLF records. +The terminal record remains incomplete and is discarded; complete CR-only files +and malformed quotes retain their existing behavior. + Custom skill export is admin-only and disabled in static demos. The lazy `skill-export-dialog.tsx` must abort requests and ignore stale callbacks on close or user/skill changes. `core/skills/export.ts` owns the revision-bound Blob download; diff --git a/frontend/src/core/artifacts/delimited-preview.ts b/frontend/src/core/artifacts/delimited-preview.ts index ea2fe758ba..ae3aaa48f4 100644 --- a/frontend/src/core/artifacts/delimited-preview.ts +++ b/frontend/src/core/artifacts/delimited-preview.ts @@ -39,7 +39,15 @@ export function parseDelimitedPreview({ truncated, }: DelimitedPreviewInput): DelimitedPreviewResult { // Papa strips BOM too; strip here so cursor and length use the same coordinates. - const input = content.startsWith("\uFEFF") ? content.slice(1) : content; + let input = content.startsWith("\uFEFF") ? content.slice(1) : content; + // Papa's auto-detection can count CRs inside an unfinished quoted field. + const newline = detectRecordNewline(input, delimiter); + // A range may end halfway through CRLF. Without the LF, Papa treats the CR + // after a closing quote as malformed syntax. Keep the terminal record + // incomplete so the normal prefix handling below discards it. + if (truncated && newline !== "\r" && input.endsWith("\r")) { + input = input.slice(0, -1); + } const result: DelimitedPreviewResult = { rows: [], columnCount: 0, @@ -50,8 +58,7 @@ export function parseDelimitedPreview({ let firstWidth: number | undefined; Papa.parse(input, { delimiter, - // Papa's auto-detection can count CRs inside an unfinished quoted field. - newline: detectRecordNewline(input, delimiter), + newline, header: false, dynamicTyping: false, skipEmptyLines: false, diff --git a/frontend/tests/e2e/artifact-table-preview.spec.ts b/frontend/tests/e2e/artifact-table-preview.spec.ts index f7a81f3e54..5c0cdae23c 100644 --- a/frontend/tests/e2e/artifact-table-preview.spec.ts +++ b/frontend/tests/e2e/artifact-table-preview.spec.ts @@ -189,6 +189,22 @@ test("rejects malformed quotes and keeps source available", async ({ await expect(panel.locator(".cm-editor")).toBeVisible(); }); +test("previews complete rows when a range response splits a quoted row's CRLF", async ({ + page, +}) => { + const panel = await setup( + page, + 'ID,Note\r\n00123,complete\r\n002,"partial"\r', + true, + ); + await expect(panel.getByRole("table")).toBeVisible({ timeout: 20000 }); + await expect(panel.getByText("00123", { exact: true })).toBeVisible(); + await expect(panel.getByText("002", { exact: true })).toHaveCount(0); + await expect( + panel.getByText("Preview of first 1 rows", { exact: true }), + ).toBeVisible(); +}); + test("previews an unsaved draft and preserves it after a save conflict", async ({ page, }) => { diff --git a/frontend/tests/unit/core/artifacts/delimited-preview.test.ts b/frontend/tests/unit/core/artifacts/delimited-preview.test.ts index 900e15481c..3af40396e7 100644 --- a/frontend/tests/unit/core/artifacts/delimited-preview.test.ts +++ b/frontend/tests/unit/core/artifacts/delimited-preview.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "@rstest/core"; import { parseDelimitedPreview } from "@/core/artifacts/delimited-preview"; +import { DELIMITED_INPUT_LIMIT } from "@/core/artifacts/delimited-preview-types"; const parse = ( content: string, truncated = false, @@ -60,6 +61,73 @@ describe("delimited preview parser", () => { expect(() => parse('a\n"bad"x\n', true)).toThrow(); expect(() => parse('a\n"bad"x\n')).toThrow(); }); + it.each(["," as const, "\t" as const])( + "discards a terminal record when CRLF is split after a quoted field (%j)", + (delimiter) => { + for (const field of ['"a"', '""', '"say ""hi"""', '"multi\r\nline"']) { + const complete = `ID${delimiter}Note\r\n001${delimiter}complete\r\n`; + expect( + parse(`${complete}002${delimiter}${field}\r`, true, delimiter), + ).toEqual({ + rows: [ + ["ID", "Note"], + ["001", "complete"], + ], + columnCount: 2, + limited: true, + unevenRows: false, + }); + expect( + parse(`${complete}002${delimiter}${field}\r\n`, true, delimiter).rows, + ).toHaveLength(3); + } + }, + ); + it.each(["," as const, "\t" as const])( + "discards a split CRLF record after an LF-first header (%j)", + (delimiter) => { + const prefix = `ID${delimiter}Note\n001${delimiter}"x"\r`; + expect(parse(prefix, true, delimiter).rows).toEqual([["ID", "Note"]]); + expect(() => parse(prefix, false, delimiter)).toThrow(); + }, + ); + it.each(["," as const, "\t" as const])( + "handles a CRLF split at the actual sample limit (%j)", + (delimiter) => { + const start = `ID${delimiter}Note\r\n001${delimiter}complete\r\n002${delimiter}"`; + const prefix = + start + "x".repeat(DELIMITED_INPUT_LIMIT - start.length - 2) + '"\r'; + const full = `${prefix}\n003${delimiter}later\r\n`; + expect(prefix).toHaveLength(DELIMITED_INPUT_LIMIT); + expect(parse(full, false, delimiter).rows).toHaveLength(4); + expect( + parse(full.slice(0, DELIMITED_INPUT_LIMIT), true, delimiter).rows, + ).toEqual([ + ["ID", "Note"], + ["001", "complete"], + ]); + }, + ); + it.each([false, true])( + "does not hide malformed quotes before a terminal CR (truncated=%j)", + (truncated) => { + expect(() => parse('ID,Note\r\n001,"bad"x\r', truncated)).toThrow(); + expect(() => parse('ID,Note\r\n001,"bad"x\r\n', truncated)).toThrow(); + }, + ); + it("preserves complete CR-only records and quoted carriage returns", () => { + expect(parse('ID,Note\r001,"complete"\r', true).rows).toEqual([ + ["ID", "Note"], + ["001", "complete"], + ]); + expect(parse('ID,Note\r\n001,"embedded\rvalue"\r\n', true).rows).toEqual([ + ["ID", "Note"], + ["001", "embedded\rvalue"], + ]); + }); + it("does not normalize a complete file with a mismatched terminal CR", () => { + expect(() => parse('ID,Note\r\n001,"complete"\r')).toThrow(); + }); it.each(["," as const, "\t" as const])( "ignores embedded CRs in an incomplete quoted field for delimiter %j", (delimiter) => {