feat: Improve Parquet Viewer

[camielvs]

What this does

Improves how Parquet files are previewed in the artifact viewer.

Before: opening a Parquet preview downloaded the whole file and then showed up to 1,000 rows. Big files were slow, and files above the preview size limit couldn't be opened at all.

Now: the viewer reads the file in pages — it only pulls the parts it actually needs (the file's metadata plus the first rows) instead of downloading everything up front. In practice this means:

• Large files open quickly, at any size. Because we no longer download the whole file, Parquet previews are no longer blocked by the size limit that applies to other artifact types.
• You get a fast preview of the first rows of data as soon as it opens.
• The header shows the total row and column counts read straight from the file.
• A "Download schema" button lets you save the file's column layout (names, types, nullability) as a JSON file.

Type of Change

• Improvement

Test Instructions

1. Open a run that has a Parquet output artifact.
2. Select the task (or output) and open its artifact preview.
3. Confirm you see: the data preview table, the total row/column counts in the header, and a working Download schema button.
4. Try a large Parquet file and confirm it opens quickly — previously a file this size may have been blocked as too large to preview.

Notes

Also includes some internal cleanup raised in review: Parquet-only code was moved into its own module so non-Parquet previews (CSV/TSV) stay lightweight, and the fetch/error-handling logic that was duplicated is now shared.

Follow-up

Additional viewer affordances — incremental Load more / Load max, a Download full dataset escape hatch, and CSV/TSV row/column-count parity — are stacked on top in #2593.

[github-actions]

🎩 Preview

A preview build has been created at: 07-21-feat_improve_parquet_viewer/d3eb937

[camielvs]

• feat: Artifact Previews on v2 Runs Output Nodes #2588
• feat: Improve Table Previews #2593
• feat: Improve Parquet Viewer #2557 👈 (View in Graphite)
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Copilot]

Pull request overview

This PR improves the Parquet artifact viewer by switching to range-request-backed Parquet reads (so previews aren’t bounded by full file size) and by enriching the table UI with Parquet metadata (row/column counts) plus a downloadable schema JSON.

Changes:

• Replace full-file Parquet fetching with hyparquet async/range-request utilities and TanStack Query suspense caching.
• Add Parquet-derived stats (total rows / column count) and a “Download schema” action to the table viewer header.
• Add utilities + unit tests for formatting/types, column counting, and schema JSON generation; expand component tests accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/utils.ts	Adds Parquet helper utilities for type formatting, column building, column counting, and schema JSON generation.
src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/utils.test.ts	Adds unit tests for the new Parquet utility helpers.
src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/TableVisualizer.tsx	Renders optional metadata stats + “Download schema” action in the table header.
src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/TableVisualizer.test.tsx	Adds tests for the new header behavior and schema download action.
src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/ParquetVisualizer.tsx	Switches to range-request-backed Parquet reads, adds stats + schema download wiring, and uses suspense query caching.
src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/ParquetVisualizer.test.tsx	Updates tests to mock the async/range-request Parquet flow and validate stats/schema download behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/IOCell.tsx:11

• IOCell now imports normalizeRawType/resolveArtifactType from ArtifactPreviewContent.tsx, which also pulls in React Query, providers, and preview components. Importing that module here just to determine whether an artifact is range-readable increases coupling and can accidentally drag heavier dependencies into the IOCell bundle. Consider extracting the artifact-type normalization/aliasing helpers into a small non-React module (e.g. ArtifactVisualizer/artifactType.ts) and importing from both IOCell and ArtifactVisualizer.

import {
  normalizeRawType,
  resolveArtifactType,
} from "./ArtifactVisualizer/ArtifactPreviewContent";

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/TableVisualizer.test.tsx:143

• This assertion assumes an en-US thousands separator (", "). TableVisualizer uses toLocaleString() without an explicit locale, so output depends on the test runner's default locale and can be flaky. Consider building the expected string using Intl.NumberFormat() so the test matches the runtime locale.

    expect(screen.getByText("1,234,567 rows · 12 columns")).toBeInTheDocument();

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/ParquetVisualizer.tsx:35

• Parquet FileMetaData.num_rows is a bigint (int64). Storing totalRows as a number forces later conversion (and potential precision loss for large row counts). Keeping this as bigint (or a bigint/number union) preserves correctness for large parquet files.

interface ParquetPreview {
  parsed: ParsedArtifact;
  totalRows: number;
  columnCount: number;
  schemaJson: unknown;
}

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/ParquetVisualizer.tsx:85

• Converting metadata.num_rows (bigint) to Number can lose precision for large parquet files (> 2^53-1 rows). Prefer passing through the bigint value (it supports toLocaleString) and letting the UI render it as text.

      });

      const totalRows = Number(metadata.num_rows);
      const columnCount = countColumns(metadata);
      const schemaJson = buildSchemaJson(metadata);

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/TableVisualizer.tsx:26

• TableVisualizer.totalRows is typed as number, but parquet metadata row counts are bigint (int64). With ParquetVisualizer now passing through metadata.num_rows, this prop should accept bigint to avoid precision loss.

interface TableVisualizerProps {
  data: ArtifactTableData;
  isFullscreen: boolean;
  onLoadMore?: () => void;
  onLoadAll?: () => void;
  /** Total row count from file metadata (may exceed the previewed rows). */
  totalRows?: number;
  /** Total column count from file metadata. */
  columnCount?: number;
  /** When provided, renders a "Download schema" action in the stats header. */
  onDownloadSchema?: () => void;

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/TableVisualizer.tsx:76

• The decorative Download icon inside the "Download schema" button is missing aria-hidden. In this codebase, icons adjacent to visible text are typically marked aria-hidden="true" to avoid screen readers announcing the icon (e.g. src/components/Onboarding/OnboardingNavPill.tsx:27, src/components/Learn/TipOfTheDay.tsx:102).

          {onDownloadSchema && (
            <Button variant="link" size="inline-xs" onClick={onDownloadSchema}>
              <Icon name="Download" size="xs" />
              Download schema
            </Button>

[camielvs]

Thanks @Copilot — went through the review. Addressed 1–6:

• Extracted normalizeRawType/resolveArtifactType (+ isVisualizableType, inferTypeFromUri, aliases) into a new non-React artifactType.ts; IOCell, ArtifactVisualizer, and ArtifactPreview now import from there instead of the heavy ArtifactPreviewContent module.
• Split parquet-only helpers into parquetUtils.ts so utils.ts (used by CSV/TSV) no longer top-level-imports hyparquet; renamed the test to parquetUtils.test.ts.
• De-duplicated the fetch-or-throw logic into a shared fetchArtifactOrThrow in useArtifactFetch.tsx, reused by both the hook and ParquetVisualizer.
• Fixed the stale test comment (was referencing Number.MAX_SAFE_INTEGER), made the stats assertion locale-independent (derives via toLocaleString), and added aria-hidden to the Download icon.

Declining the num_rows bigint suggestion. Number(metadata.num_rows) only loses precision above 2^53 ≈ 9 quadrillion rows, which is not a realistic parquet row count, and this value is used purely for a display-only stat in the header (not for any calculation or indexing). Threading a bigint | number union through the ParquetPreview → TableVisualizer prop chain adds type friction across the boundary for a case that can't occur in practice. Keeping it as number for simplicity.

[Copilot]

🟡 Not ready to approve

The new hyparquet runtime import in a shared utils.ts used by non-parquet paths can unnecessarily pull Parquet code into common previews, impacting bundle size/performance.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (1)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/parquetUtils.test.ts:49

• This comment says the row count is a “plain large count”, but the value is 42n. It would be clearer to explain that the test uses a bigint to ensure buildSchemaJson sanitizes bigint values (so JSON.stringify won’t throw).

      // A bigint row count exercises hyparquet's toJson bigint sanitization.
      42n,
    );

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

There are confirmed error-handling edge cases that can break parquet previews (hyparquet HEAD method detection) or trigger unbounded full downloads when Content-Length is unavailable, plus “load more” failures currently don’t surface to the UI.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (3)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/parquetUtils.ts:73

• In openParquet's non-range fallback, if Content-Length is missing/unavailable (common with some CORS setups), contentLength becomes NaN and the code will proceed to response.arrayBuffer(), potentially downloading an arbitrarily large parquet into memory. It would be safer to treat unknown length as not previewable without range support (same 413 path).

    const response = await fetchArtifactOrThrow(signedUrl);
    const contentLength = Number(response.headers.get("Content-Length"));
    if (
      Number.isFinite(contentLength) &&
      contentLength > PARQUET_FULL_DOWNLOAD_LIMIT_BYTES
    ) {

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/useArtifactFetch.tsx:47

• fetchArtifactForHyparquet infers the HTTP method only from init?.method, but fetch callers can pass a Request object where the method lives on input. If hyparquet uses a Request for its HEAD probe, this code will mis-detect it as GET and incorrectly throw on the expected HEAD 403, breaking parquet previews.

export const fetchArtifactForHyparquet: typeof fetch = async (input, init) => {
  const response = await fetch(input, init);
  if (response.ok) return response;

  const method = (init?.method ?? "GET").toUpperCase();
  if (method === "HEAD" && response.status === 403) {
    return response;
  }

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/ParquetVisualizer.tsx:108

• When loading additional parquet rows, errors are only logged to the console and the UI silently stays on the previous data. This makes failures hard to notice and bypasses the existing ArtifactVisualizer error boundary behavior used for initial load failures.

  const loadUpTo = async (target: number) => {
    if (isLoading || target <= loadedCount) return;
    setIsLoading(true);
    try {
      const more = await readParquetRows(
        base.opened,
        base.columns,
        loadedCount,
        target,
      );
      setRows((prev) => [...prev, ...more]);
    } catch (error) {
      console.error("Failed to load more parquet rows:", error);
    } finally {
      setIsLoading(false);
    }

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

The Parquet fallback size-cap check can be bypassed when the Content-Length header is missing/unexposed (parsed as 0), allowing unintended large full downloads.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (1)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/parquetUtils.ts:73

• Number(response.headers.get("Content-Length")) treats a missing/blocked Content-Length header as 0 (because Number(null) === 0). In that case the full-download fallback will proceed even for very large files, defeating the intended safety cap when the header isn't exposed via CORS or isn't present. Parse the header explicitly and treat missing/NaN as unknown so the size check doesn't silently pass.

    const contentLength = Number(response.headers.get("Content-Length"));
    if (
      Number.isFinite(contentLength) &&
      contentLength > PARQUET_FULL_DOWNLOAD_LIMIT_BYTES
    ) {

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

There are confirmed correctness/security issues in the new fetch and fallback paths (HEAD method detection and Content-Length parsing, plus missing noopener/noreferrer on signed URL opens) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (3)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/parquetUtils.ts:73

• Number(response.headers.get("Content-Length")) treats a missing header (null) as 0, which can incorrectly bypass the full-download size cap and trigger an unbounded download in the fallback path. Parse the header explicitly so missing/invalid values become NaN (and therefore skip the cap check rather than defaulting to 0).

    const response = await fetchArtifactOrThrow(signedUrl);
    const contentLength = Number(response.headers.get("Content-Length"));
    if (
      Number.isFinite(contentLength) &&
      contentLength > PARQUET_FULL_DOWNLOAD_LIMIT_BYTES
    ) {

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/ParquetVisualizer.tsx:143

• Opening the signed artifact URL in a new tab should use noopener,noreferrer to prevent reverse-tabnabbing and avoid leaking referrer information to the storage origin.

      onDownloadFull={() => window.open(signedUrl, "_blank")}

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/useArtifactFetch.tsx:37

• fetchArtifactForHyparquet only checks init?.method to detect HEAD, but callers can pass a Request object with method: "HEAD" and no init. In that case this defaults to GET and will throw on the expected 403 HEAD response, breaking hyparquet's fallback behavior for GCS signed URLs.

  const method = (init?.method ?? "GET").toUpperCase();

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

New window.open(..., "_blank") calls should add noopener,noreferrer for security, and the Parquet “load more” failure path currently provides no user-visible feedback.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (4)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/ParquetVisualizer.tsx:144

• window.open(..., "_blank") should include noopener,noreferrer (tabnabbing protection). There’s already an established pattern for this in src/components/Learn/LearnSearchBar.tsx:21 using a third argument.

      onDownloadSchema={handleDownloadSchema}
      onDownloadFull={() => window.open(signedUrl, "_blank")}
    />

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/CsvVisualizer.tsx:80

• window.open(..., "_blank") should include noopener,noreferrer to prevent the opened page from getting a handle to window.opener (tabnabbing). This is consistent with src/components/Learn/LearnSearchBar.tsx:21.

  return (
    <CsvContent
      parsed={parsed}
      isFullscreen={isFullscreen}
      onDownloadFull={() => window.open(signedUrl, "_blank")}
    />
  );

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/ParquetVisualizer.tsx:114

• If loading additional parquet rows fails, the error is only logged to the console; the UI keeps showing the existing rows with no user-visible feedback. Consider surfacing a small inline error message and/or a toast so users know the "Load more" action failed.

        target,
      );
      setRows((prev) => [...prev, ...more]);
    } catch (error) {
      console.error("Failed to load more parquet rows:", error);
    } finally {
      setIsLoading(false);

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/utils.ts:47

• parseCsv now parses the entire (already-downloaded) CSV/TSV input to compute an exact totalRows (via step), which can noticeably block the main thread for near-limit files (up to 50MB). If the row count is mainly for UI stats, consider an approach that avoids fully parsing synchronously (e.g. Papa worker: true where supported) or reverting to a bounded parse and showing an approximate/unknown total when truncated.

/**
 * Parse delimited text into a preview table. The whole (already-downloaded)
 * text is streamed row by row so we can report the exact `totalRows` while
 * retaining only the first {@link MAX_PREVIEW_ROWS} rows in memory.
 */
export function parseCsv(text: string, delimiter?: string): ParsedArtifact {
  let headers: string[] | undefined;
  const rows: string[][] = [];
  let totalRows = 0;

  Papa.parse<string[]>(text, {
    delimiter,
    header: false,
    skipEmptyLines: true,
    step: (result) => {
      const row = result.data;
      if (headers === undefined) {
        headers = row;
        return;
      }
      totalRows++;
      if (rows.length < MAX_PREVIEW_ROWS) rows.push(row);
    },
  });

• Files reviewed: 17/17 changed files
• Comments generated: 0 new
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

There are confirmed security and operational robustness issues (window.open without noopener and fallback full-download safety gaps) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (2)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/useArtifactFetch.tsx:41

• fetchArtifactForHyparquet only checks init.method to detect the HEAD probe. If a Request object is passed (method set on the Request, no init), this will be treated as GET and the expected 403 will be thrown, preventing hyparquet’s fallback to ranged GET.

  transform: (response: Response) => Promise<T>,
): T {
  const { data } = useSuspenseQuery({
    queryKey: [`artifact-${queryKey}`, signedUrl],
    queryFn: async () => {

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/ParquetVisualizer.tsx:113

• loadUpTo swallows range-read failures by only logging to the console; from the UI perspective the action just does nothing. Consider surfacing an inline error state or a toast so users know the load failed and can retry.

    } catch (error) {
      console.error("Failed to load more parquet rows:", error);
    } finally {

• Files reviewed: 17/17 changed files
• Comments generated: 3
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

There are a few correctness/safety gaps (notably parquet size-limit/type inference in IOCell and the parquet full-download fallback size guard when Content-Length is missing) that could reintroduce blocked previews or large unintended downloads.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (5)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/IOCell.tsx:32

• artifactType should consider the artifact URI as a fallback (via inferTypeFromUri) so parquet artifacts without an explicit type/type_name still get the parquet preview behavior (including bypassing the 50MB size limit).

  const artifactType =
    type ?? artifact?.type_name ?? (artifactData?.is_dir ? "Directory" : "Any");

  const isRangeReadable =
    resolveArtifactType(normalizeRawType(artifactType)) === "apacheparquet";

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/parquetUtils.ts:57

• In the non-range fallback path, Number(response.headers.get("Content-Length")) becomes 0 when the header is missing (and NaN when unparsable), which bypasses the size cap and can still trigger a very large full download into memory. It’s safer to fail closed when the size is unknown, since this code path only exists when range requests are unavailable.

    const response = await fetchArtifactOrThrow(signedUrl);
    const contentLength = Number(response.headers.get("Content-Length"));
    if (
      Number.isFinite(contentLength) &&
      contentLength > PARQUET_FULL_DOWNLOAD_LIMIT_BYTES

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/IOCell.tsx:11

• IOCell's parquet size-limit exemption relies on artifactType, but artifactType can fall back to "Any" when type/type_name are missing. In that case a .parquet URI would still be treated as non-range-readable and the preview may be incorrectly hidden for large parquet artifacts. Importing and using inferTypeFromUri enables consistent type inference from the URI.

This issue also appears on line 28 of the same file.

import {
  normalizeRawType,
  resolveArtifactType,
} from "./ArtifactVisualizer/artifactType";

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/CsvVisualizer.tsx:59

• CsvVisualizerValue is used for both CSV and TSV inline previews (see InlineContent), but the full-dataset download always saves as data.csv with a text/csv content-type. This produces the wrong extension/MIME for TSV inline values.

  <CsvContent
    parsed={parseCsv(value)}
    isFullscreen={isFullscreen}
    onDownloadFull={() =>
      downloadStringAsFile(value, "data.csv", "text/csv;charset=utf-8")

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/utils.ts:33

• parseCsv now iterates the entire input via step in order to compute an exact totalRows. Previously the parser used Papa's preview option to cap work; this change can significantly slow down rendering for large (but still <= 50MB) CSV/TSV artifacts because it must parse the whole file before showing the first rows.

  Papa.parse<string[]>(text, {
    delimiter,
    header: false,
    skipEmptyLines: true,
    step: (result) => {

• Files reviewed: 17/17 changed files
• Comments generated: 0 new
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

There are confirmed edge-case bugs that can incorrectly hide Parquet previews (missing type metadata) and bypass the fallback full-download size guard when Content-Length is absent.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (2)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/parquetUtils.ts:56

• Number(response.headers.get("Content-Length")) treats a missing header as 0 (because Number(null) === 0), which bypasses the >50MB safety check and can lead to unexpectedly full-downloading very large parquet files when range requests fail. Parse the header defensively so null becomes NaN (or otherwise handled) instead of 0.

    const response = await fetchArtifactOrThrow(signedUrl);
    const contentLength = Number(response.headers.get("Content-Length"));
    if (
      Number.isFinite(contentLength) &&
      contentLength > PARQUET_FULL_DOWNLOAD_LIMIT_BYTES

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/IOCell.tsx:32

• isRangeReadable is computed only from the declared type/type_name. If those are missing but the URI ends in .parquet, large parquet artifacts will still be treated as non-range-readable and the 50MB preview limit will hide the preview button. Consider inferring the type from artifactData.uri as a fallback for this check.

  const artifactType =
    type ?? artifact?.type_name ?? (artifactData?.is_dir ? "Directory" : "Any");

  const isRangeReadable =
    resolveArtifactType(normalizeRawType(artifactType)) === "apacheparquet";

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

The Parquet fallback size check can be bypassed when Content-Length is missing because Number(null) becomes 0, enabling unintended full-file downloads.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (1)

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection/IOCell/ArtifactVisualizer/parquetUtils.ts:55

• Number(response.headers.get("Content-Length")) coerces a missing header (null) to 0, so if the server omits Content-Length this fallback path will treat the file as size 0 and proceed to arrayBuffer() download the entire parquet into memory—bypassing the intended 50MB safety cap.

    const response = await fetchArtifactOrThrow(signedUrl);
    const contentLength = Number(response.headers.get("Content-Length"));
    if (
      Number.isFinite(contentLength) &&
      contentLength > MAX_VISUALIZABLE_SIZE_BYTES
    ) {

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.