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.

Author: davidmckayv

app/src/components/channels/composer/attachment-strip.tsx

@@ -126,26 +126,45 @@ 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}
           >
-            <IconFile className="size-5 text-muted-foreground" />
+            {/*
+             * `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.
+             */}
+            <IconFile className="size-5 shrink-0 text-muted-foreground" />
             <div className="min-w-0">
               <p className="truncate font-medium text-xs" title={file.name}>
                 {file.name}
               </p>
               {file.loading ? (
                 <p className="text-muted-foreground text-xs">Uploading…</p>
               ) : file.size === undefined ? null : (
-                <p className="text-muted-foreground text-xs">
-                  {formatBytes(file.size)}
-                </p>
-              )}
-              {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.
+                 */
                 <p
-                  className="text-muted-foreground text-xs"
-                  title="The model reads the first 120,000 characters of this file."
+                  className="truncate text-muted-foreground text-xs"
+                  title={
+                    file.mayTruncate === true
+                      ? "The model reads the first 120,000 characters of this file."
+                      : undefined
+                  }
                 >
-                  May be read truncated
+                  {formatBytes(file.size)}
+                  {file.mayTruncate === true ? " · may be cut" : null}
                 </p>
-              ) : null}
+              )}
             </div>
             <RemoveButton name={file.name} onRemove={() => onRemove(file.id)} />
           </Staged>

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),
         })),

app/tests/composer-attachment-strip.test.tsx

@@ -130,7 +130,7 @@ test("a text file that fits shows no truncation warning", () => {
     <AttachmentStrip files={[notes]} images={[]} onRemove={() => {}} />,
   );
 
-  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();
 });

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(
+    <Composer channelId="channel-1" compact onSubmit={() => {}} />,
+  );
+  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(
+    <Composer channelId="channel-1" compact onSubmit={() => {}} />,
+  );
+  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");
+});