From d28a66627d71221f58869e08487d461b35973638 Mon Sep 17 00:00:00 2001 From: Yash Maheshwari Date: Sun, 20 Sep 2026 13:07:07 +0530 Subject: [PATCH 1/2] Warn on staged text attachment the model may read truncated --- .../channels/composer/attachment-strip.tsx | 14 ++++++++++ .../components/channels/composer/composer.tsx | 15 +++++++++++ app/src/lib/channels/attachments.ts | 1 + app/tests/composer-attachment-strip.test.tsx | 26 +++++++++++++++++++ shared/attachments.test.ts | 10 +++---- shared/attachments.ts | 12 +++++++++ 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/app/src/components/channels/composer/attachment-strip.tsx b/app/src/components/channels/composer/attachment-strip.tsx index eadbe3efb..10d12c76f 100644 --- a/app/src/components/channels/composer/attachment-strip.tsx +++ b/app/src/components/channels/composer/attachment-strip.tsx @@ -44,6 +44,12 @@ export type StagedFile = { name: string; size?: number; loading: boolean; + /** + * The model reads at most MAX_EXTRACTED_CHARACTERS of a text file, while + * the pick ceiling is MAX_FILE_BYTES. A file over the first may be read + * truncated — warned on the tile, never refused. + */ + mayTruncate?: boolean; }; export function AttachmentStrip({ @@ -132,6 +138,14 @@ export function AttachmentStrip({ {formatBytes(file.size)}

)} + {file.mayTruncate === true ? ( +

+ May be read truncated +

+ ) : null} onRemove(file.id)} /> diff --git a/app/src/components/channels/composer/composer.tsx b/app/src/components/channels/composer/composer.tsx index 9a86a34fd..5625de37b 100644 --- a/app/src/components/channels/composer/composer.tsx +++ b/app/src/components/channels/composer/composer.tsx @@ -30,6 +30,7 @@ import { attachmentUrl, classifyAttachment, MAX_ATTACHMENTS_PER_MESSAGE, + mayBeTruncatedForModel, mediaTypeOf, shouldClaimPaste, } from "@/lib/channels/attachments"; @@ -1004,6 +1005,20 @@ export function Composer({ name: attachment.filename ?? "Attachment", size: attachment.size, loading: attachment.status === "uploading", + /* + * Warned, never refused. The server accepts the whole file and the + * model reads its first MAX_EXTRACTED_CHARACTERS; mayBeTruncatedForModel + * is exact in the safe direction (N bytes decode to at most N chars), + * so a file at or under the ceiling cannot be cut. Only text reaches + * the extraction path: images are excluded by the filter above, and an + * unnamed pick whose type is still unknown gets no warning until the + * server has sniffed it (then the strip re-renders off the url source). + */ + mayTruncate: + attachment.status !== "uploading" && + attachment.type !== "image" && + attachment.size !== undefined && + mayBeTruncatedForModel(attachment.size), })), [staged], ); diff --git a/app/src/lib/channels/attachments.ts b/app/src/lib/channels/attachments.ts index 7ff8b2f0b..687f5e1e8 100644 --- a/app/src/lib/channels/attachments.ts +++ b/app/src/lib/channels/attachments.ts @@ -17,6 +17,7 @@ export { MAX_EXTRACTED_CHARACTERS, MAX_FILE_BYTES, MAX_IMAGE_BYTES, + mayBeTruncatedForModel, mediaTypeOf, namesNoFormat, shouldClaimPaste, diff --git a/app/tests/composer-attachment-strip.test.tsx b/app/tests/composer-attachment-strip.test.tsx index 51732b70a..afe4c4dd3 100644 --- a/app/tests/composer-attachment-strip.test.tsx +++ b/app/tests/composer-attachment-strip.test.tsx @@ -121,3 +121,29 @@ test("an unnamed image going up falls back to the same word the finished one use expect(getByRole("img", { name: "Attachment, uploading" })).toBeTruthy(); }); + +test("a text file that fits shows no truncation warning", () => { + // mayTruncate is what composer.tsx sets from mayBeTruncatedForModel: a file + // at or under MAX_EXTRACTED_CHARACTERS cannot be cut (N bytes decode to at + // most N chars), so the tile must not warn about it. + const { queryByText } = render( + {}} />, + ); + + expect(queryByText("May be read truncated")).toBeNull(); +}); + +test("a text file over the extraction ceiling warns it may be read truncated", () => { + // The gap this closes: the server accepts up to MAX_FILE_BYTES and the + // model reads the first MAX_EXTRACTED_CHARACTERS, so a file between the two + // arrives truncated with nothing on screen saying so. Warned, never refused. + const { getByText } = render( + {}} + />, + ); + + expect(getByText("May be read truncated")).toBeTruthy(); +}); diff --git a/shared/attachments.test.ts b/shared/attachments.test.ts index 9bcd72649..339668169 100644 --- a/shared/attachments.test.ts +++ b/shared/attachments.test.ts @@ -4,6 +4,7 @@ import { MAX_ATTACHMENTS_PER_MESSAGE, MAX_EXTRACTED_CHARACTERS, MAX_FILE_BYTES, + mayBeTruncatedForModel, mediaTypeOf, namesNoFormat, shouldClaimPaste, @@ -135,13 +136,10 @@ describe("what is uploaded and what the model reads are different limits", () => // characters. A file at or under the character ceiling therefore CANNOT be // truncated, which is what makes `file.size > MAX_EXTRACTED_CHARACTERS` a // warning that never fires on a file that arrives whole. - const mayBeTruncated = (byteLength: number) => - byteLength > MAX_EXTRACTED_CHARACTERS; - - expect(mayBeTruncated(MAX_EXTRACTED_CHARACTERS)).toBe(false); - expect(mayBeTruncated(MAX_EXTRACTED_CHARACTERS + 1)).toBe(true); + expect(mayBeTruncatedForModel(MAX_EXTRACTED_CHARACTERS)).toBe(false); + expect(mayBeTruncatedForModel(MAX_EXTRACTED_CHARACTERS + 1)).toBe(true); // The case the gap is about: an accepted upload that will still be cut. - expect(mayBeTruncated(MAX_FILE_BYTES)).toBe(true); + expect(mayBeTruncatedForModel(MAX_FILE_BYTES)).toBe(true); }); }); diff --git a/shared/attachments.ts b/shared/attachments.ts index 3ce58ccee..6740337ed 100644 --- a/shared/attachments.ts +++ b/shared/attachments.ts @@ -94,6 +94,18 @@ export const MAX_FILE_BYTES = 1024 * 1024; */ export const MAX_EXTRACTED_CHARACTERS = 120_000; +/** + * Whether a file of this many BYTES may reach the model truncated. + * + * Exact in the safe direction: UTF-8 decodes N bytes to at most N characters, + * so `size <= MAX_EXTRACTED_CHARACTERS` cannot be cut, and `size > ...` may be + * (fewer characters for multi-byte scripts, hence "may"). Callers show a + * warning, never a refusal — the server still accepts and reads the prefix. + */ +export function mayBeTruncatedForModel(sizeBytes: number): boolean { + return sizeBytes > MAX_EXTRACTED_CHARACTERS; +} + /** * `image/svg+xml` is deliberately absent. * From 847e1b36114d75cb85b18ffd24cd2a83285a284e Mon Sep 17 00:00:00 2001 From: David McKay Date: Sun, 20 Sep 2026 16:41:58 -0700 Subject: [PATCH 2/2] Keep the tile's icon whole when the truncation warning is on it The warning arrived as a third line of text in a tile whose height is fixed, so it did not fit: at this repository's 15px root the two in-flow children need 63.75px inside a 58px box, and the icon is the only one that can give. Measured in Chromium, it rendered 18.75x13 on exactly the tiles carrying the warning. Pinning the icon alone only moves the overflow onto the border, so the warning folds onto the size line instead and the tile keeps two lines. `truncate` is what guarantees that: `formatBytes` is unbounded, and a large enough file would otherwise wrap the line and put the third one back. Also drops the `attachment.type` test from the composer's derivation. The filter above it already excludes images using `stagedModality`, which prefers the mime the server sniffed; `attachment.type` is the browser's claim, fixed at pick time and never revised. Testing both could only subtract, and the file it subtracted is the one that matters: a file the browser called a PNG and the server read as text goes down the extraction path and is exactly what gets cut. Covered by a composer-level test, which the warning did not have. --- .../channels/composer/attachment-strip.tsx | 39 ++++++++++++----- .../components/channels/composer/composer.tsx | 8 +++- app/tests/composer-attachment-strip.test.tsx | 4 +- app/tests/composer-attachments-ui.test.tsx | 42 +++++++++++++++++++ 4 files changed, 80 insertions(+), 13 deletions(-) diff --git a/app/src/components/channels/composer/attachment-strip.tsx b/app/src/components/channels/composer/attachment-strip.tsx index 10d12c76f..6885b960e 100644 --- a/app/src/components/channels/composer/attachment-strip.tsx +++ b/app/src/components/channels/composer/attachment-strip.tsx @@ -126,7 +126,13 @@ export function AttachmentStrip({ className="flex h-20 w-40 flex-col justify-between rounded-xl border border-border bg-muted/40 p-2" key={file.id} > - + {/* + * `shrink-0` because the tile's height is fixed and the text below it is not. A third + * line, which is exactly what the truncation warning adds, takes the extra out of the + * only child that can give: this icon. It renders visibly smaller on precisely the + * tiles that carry the warning, which is the wrong moment to look broken. + */} +

{file.name} @@ -134,18 +140,31 @@ export function AttachmentStrip({ {file.loading ? (

Uploading…

) : file.size === undefined ? null : ( -

- {formatBytes(file.size)} -

- )} - {file.mayTruncate === true ? ( + /* + * ON THE SIZE LINE, NOT UNDER IT, AND THAT IS A LAYOUT CONSTRAINT RATHER THAN A + * PREFERENCE. The tile's height is fixed so that a file and a thumbnail read as + * one row. A third line does not fit: measured in Chromium at this repository's + * 15px root, the two in-flow children need 63.75px inside a 58px box. Something + * has to give, and the only choices are the icon shrinking to 13px or the text + * spilling past the border. Folding the warning onto this line keeps two. + * + * `truncate` is the guarantee, not the wording: `formatBytes` is unbounded, so a + * large enough file would wrap this line and put the third one back. The short + * phrasing is what keeps it from ellipsizing at any realistic size; `title` + * carries the sentence that actually explains it. + */

- May be read truncated + {formatBytes(file.size)} + {file.mayTruncate === true ? " · may be cut" : null}

- ) : null} + )}
onRemove(file.id)} /> diff --git a/app/src/components/channels/composer/composer.tsx b/app/src/components/channels/composer/composer.tsx index 5625de37b..35c75b4c5 100644 --- a/app/src/components/channels/composer/composer.tsx +++ b/app/src/components/channels/composer/composer.tsx @@ -1013,10 +1013,16 @@ export function Composer({ * the extraction path: images are excluded by the filter above, and an * unnamed pick whose type is still unknown gets no warning until the * server has sniffed it (then the strip re-renders off the url source). + * + * THE FILTER IS THE ONLY IMAGE TEST, DELIBERATELY. `attachment.type` is + * the browser's claim, fixed at pick time and never revised; the filter + * runs `stagedModality`, which prefers what the server sniffed. Testing + * both would not narrow this to text, it would only subtract: a file the + * browser called a PNG and the server read as text lands in this strip, + * goes down the extraction path, and is exactly the one that gets cut. */ mayTruncate: attachment.status !== "uploading" && - attachment.type !== "image" && attachment.size !== undefined && mayBeTruncatedForModel(attachment.size), })), diff --git a/app/tests/composer-attachment-strip.test.tsx b/app/tests/composer-attachment-strip.test.tsx index afe4c4dd3..7ee036f0f 100644 --- a/app/tests/composer-attachment-strip.test.tsx +++ b/app/tests/composer-attachment-strip.test.tsx @@ -130,7 +130,7 @@ test("a text file that fits shows no truncation warning", () => { {}} />, ); - expect(queryByText("May be read truncated")).toBeNull(); + expect(queryByText(/may be cut/)).toBeNull(); }); test("a text file over the extraction ceiling warns it may be read truncated", () => { @@ -145,5 +145,5 @@ test("a text file over the extraction ceiling warns it may be read truncated", ( />, ); - expect(getByText("May be read truncated")).toBeTruthy(); + expect(getByText(/· may be cut/)).toBeTruthy(); }); diff --git a/app/tests/composer-attachments-ui.test.tsx b/app/tests/composer-attachments-ui.test.tsx index 8a53d5f59..2658a1035 100644 --- a/app/tests/composer-attachments-ui.test.tsx +++ b/app/tests/composer-attachments-ui.test.tsx @@ -404,3 +404,45 @@ test("a file the browser called an image is drawn as a file once the server read expect(container.querySelector("img")).toBeNull(); expect(container.textContent).toContain("notes.png"); }); + +/** + * THE WARNING IS DERIVED, AND THIS IS THE CASE THE DERIVATION CAN GET WRONG. + * + * A tile renders the warning it is handed; whether it is handed one is decided in the composer, off + * the same `stagedModality` the file filter uses. Those have to be the same test. The file below is + * the one that proves it: the browser calls it a PNG, the server reads it as text, so it lands in + * the file strip and goes down the extraction path, and at 120,001 bytes the model sees roughly a + * tenth of it. Deciding on the browser's claim instead would drop the warning on exactly this file. + */ +test("warns on a large file the browser called an image and the server read as text", async () => { + serverSniffs("text/plain"); + const view = render( + {}} />, + ); + const { container } = view; + + drop(container.querySelector("form") as HTMLFormElement, [ + new File(["x".repeat(120_001)], "notes.png", { type: "image/png" }), + ]); + + await uploaded(view, "notes.png"); + + expect(container.textContent).toContain("may be cut"); +}); + +/** The same file under the ceiling earns no warning, so the one above is the size and not the path. */ +test("does not warn on a small file the server read as text", async () => { + serverSniffs("text/plain"); + const view = render( + {}} />, + ); + const { container } = view; + + drop(container.querySelector("form") as HTMLFormElement, [ + new File(["hello"], "notes.png", { type: "image/png" }), + ]); + + await uploaded(view, "notes.png"); + + expect(container.textContent).not.toContain("may be cut"); +});