Repository navigation
Conversation
…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.
ApprovabilityVerdict: Approved at 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. |
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
|
Note Written by Verified in a real dev environmentverified unchanged head reproduced first on runtime observations (persisted message projection opened read-only): nearby behavior checked: ordinary blacksmith passed main, matching csv data: pr head, matching csv data: Not verifiednative 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. |


Problem
A CSV or TSV attachment labelled
binary/octet-streamorapplication/unknownopens as raw text with no table, while the same file labelledapplication/octet-streamgets the table. #29360 movedbinaryPreviewKindandofficePreviewKindtoGENERIC_MIME_TYPESbut missedfilePreviewDelimiter, so it now disagrees withfilePreviewKind, which treats these labels as generic.Change
filePreviewDelimiteruses the sameGENERIC_MIME_TYPESset. As in #29360, the set also holdsvideo/mp2tandvideo/vnd.dlna.mpeg-tts, so a.csvunder 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 onmain(expected null to be ',') and passes here.vp run check:changed upstream/mainpasses.team.csvFilelabelledbinary/octet-streamwent through the composer's file input and was sent in a free Replay thread. The stored attachment kept"mimeType":"binary/octet-stream".main'sdelimitedPreview.tsswapped back in, and the table (3 rows, 3 columns) on this branch.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 underbinary/octet-stream; TSV andapplication/unknownare covered by the unit test. I ran web only. Desktop was not run, and mobile shares this helper inattachmentDocument.tsandZipArchiveView.tsxbut was not run either. Repo-wide checks were not run.Claude Opus 5.5 via T3 Code