Skip to content

fix(shared): CSV and TSV attachments keep their table under generic file types - #30601

Open
tiliakoos wants to merge 1 commit into
pingdotgg:mainfrom
tiliakoos:fix/csv-generic-mime-preview
Open

tiliakoos wants to merge 1 commit into
pingdotgg:mainfrom
tiliakoos:fix/csv-generic-mime-preview

Conversation

@tiliakoos

Copy link
Copy Markdown
Contributor

Problem

A CSV or TSV attachment labelled binary/octet-stream or application/unknown opens as raw text with no table, while the same file labelled application/octet-stream gets the table. #29360 moved binaryPreviewKind and officePreviewKind to GENERIC_MIME_TYPES but missed filePreviewDelimiter, so it now disagrees with filePreviewKind, which treats these labels as generic.

Change

filePreviewDelimiter uses the same GENERIC_MIME_TYPES set. As in #29360, the set also holds video/mp2t and video/vnd.dlna.mpeg-tts, so a .csv under those labels gets a table too; that follows from reusing the set and adds no formats. Specific MIME types still win. Name-only callers are unchanged.

Scope and approval

Very small fix for an obvious bug: one guard reuses the shared set its two siblings already moved to in #29360, with no new formats, settings or UI.

There is no user report behind this. I found no real source that labels a CSV this way. The MCP upload (AttachmentUpload.ts:117) and guest upload (guestAttachments.ts:117) paths and the composer keep whatever label they are given, so one could arrive, but the case for this change is that the helpers should agree.

Verification

  • vp test run packages/shared/src/delimitedPreview.test.ts: the new test fails on main (expected null to be ',') and passes here. vp run check:changed upstream/main passes.
  • Real client: the web dev client on an empty dev database, in a real desktop browser. A team.csv File labelled binary/octet-stream went through the composer's file input and was sent in a free Replay thread. The stored attachment kept "mimeType":"binary/octet-stream".
  • Opening that same stored attachment from its chip shows raw text with main's delimitedPreview.ts swapped back in, and the table (3 rows, 3 columns) on this branch.
Before After
before-csv-binary-octet-stream after-csv-binary-octet-stream

Not checked: the label was set by hand on the File, so this does not show that any real picker sends it. In the client I ran only a CSV under binary/octet-stream; TSV and application/unknown are covered by the unit test. I ran web only. Desktop was not run, and mobile shares this helper in attachmentDocument.ts and ZipArchiveView.tsx but was not run either. Repo-wide checks were not run.

Claude Opus 5.5 via T3 Code

…ile types

filePreviewDelimiter still hardcoded application/octet-stream after pingdotgg#29360, so
binary/octet-stream and application/unknown CSVs lost the table. Use GENERIC_MIME_TYPES.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Nov 4, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 9a3c2ce

Macroscope's review found this PR approvable — CSV and TSV attachments with generic MIME labels now retain their table preview through the existing filename fallback. The change is confined to preview classification, with explicit MIME handling and parsing limits unchanged.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

filePreviewDelimiter now accepts generic MIME types through GENERIC_MIME_TYPES. Tests check comma selection for CSV files and tab selection for TSV files with generic MIME types.

Changes: Delimited preview MIME handling

Layer / File(s) Summary
Delimiter MIME guard and tests
packages/shared/src/delimitedPreview.ts, packages/shared/src/delimitedPreview.test.ts
filePreviewDelimiter now uses GENERIC_MIME_TYPES to check MIME types. Tests verify delimiter selection for .csv and .tsv files with generic MIME types.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

Suggested labels: size:XS

Suggested reviewers: macodev00, maria-rcks

Merge Risk: Minimal · up to 9a3c2

Generic-labeled CSV and TSV attachments retain table previews without overriding explicit MIME classifications.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preserving CSV and TSV table previews under generic MIME types.
Description check Passed The description includes all required sections. It explains the problem, implementation, scope rationale, focused verification, observed results, limitations, and agent details.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 647bd14 and 9a3c2ce.

Files selected for processing: 2
  • packages/shared/src/delimitedPreview.test.ts
  • packages/shared/src/delimitedPreview.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

@maria-rcks

maria-rcks commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 9a3c2ce184937dd87ab7999c2a91b3ebaf88ba3e on linux with the real dev web client, repository playwright, /usr/bin/google-chrome, and real codex gpt-6.1-sol turns. no replay provider or mocked server.

reproduced first on github/main at e68e19d605 in a separate worktree after vp i. created a project pointing to that git checkout through the app. uploaded matching csv and tsv data through the composer's actual file input, explicitly supplying the mime labels with playwright. after sending, opened the persisted attachment chips. both generic-mime files showed source only on main. stopped main before starting the exact pr head, also after vp i, with isolated dev state copied from this task's main run. the same stored attachments now show two-row, three-column tables. a fresh upload and real codex turn on the pr head also succeeded.

runtime observations (persisted message projection opened read-only):

team.csv      binary/octet-stream    59 bytes
team.tsv      application/unknown    59 bytes
ordinary.csv  text/csv               59 bytes
codex response on main: Ada: London / Grace: New York
codex response on PR:   Ada: London / Grace: New York

nearby behavior checked: ordinary text/csv still renders the same table; switching generic csv from table to source and back preserves its contents. a 10.85-second recording of these interactions was reviewed in t3.

blacksmith passed vp test run packages/shared/src/delimitedPreview.test.ts (18 tests) and vp run check:changed 647bd148b3fbd7c8b57a10fd41ae6d5fc7b35cc1 (format, lint, knip, affected typechecks, 19 tests). current ci checks have no failures or pending runs. no code changes were needed.

main, matching csv data:

main: generic-mime csv opens as source without a table toggle

pr head, matching csv data:

pr: the same generic-mime csv opens as a two-row three-column table

Not verified

native desktop, ios, android, remote/relay/tunnel connections, other providers, and the two video mime labels in the shared generic set were not exercised. these mime labels were explicitly supplied at upload; this does not prove that an operating system picker naturally assigns them. uploaded video playback and seeking were checked in t3's browser player; github's image proxy does not render the external mp4 inline, so only the before/after images are embedded here.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants