Uh oh!
There was an error while loading. Please reload this page.
feat(social): reserve space for chat image attachments (#315) - #498
Conversation
Room-chat attachments rendered into a zero-height box until the image loaded. Combined with the `loading="lazy"` that #312 added, scrolling UP through history expanded each image as it neared the viewport and shifted the transcript mid-scroll. Chrome and Firefox mostly absorb that with scroll anchoring; Safari has none, so the viewport visibly jumps on every load. It also invalidated the loadEarlier scrollTop compensation, which measures scrollHeight before the prepended images have loaded. This could not be fixed in the frontend alone: room_messages stored only image_url, so there was nothing to reserve a box with. 0040 image_width / image_height on room_messages, nullable models the same two fields on SendMessageBody, optional social.py both columns in the select list and the insert api.ts sendRoomMessage takes an optional imageSize Social.tsx measures the file before upload; renders into an aspect-ratio box The measurement happens on the picked File via an object URL, before upload, so the dimensions travel with the message that creates it — history then renders reserved from the first paint rather than after a round trip. Degrades quietly by design, at every layer. The columns are nullable and unbackfilled, so every message written before this keeps NULL; readImageSize resolves undefined on a non-image or a decode failure rather than blocking the send; and the renderer only applies width/height/aspect-ratio when both values are present. Anything without dimensions renders exactly as it does today. Backfilling would mean fetching every historical image to measure it, which is not worth it for a layout hint. part of #315 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Warning Review limit reached
Next review available in:40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 7c960ed | Commit Preview URL Branch Preview URL | Jul 31 2026, 08:30 AM |
Review finding, and a real one: image_width/image_height arrive from the client and the transcript renders them directly as an aspect-ratio, so an absurd pair is a layout weapon against every member of the room rather than a bad row for its author. Unbounded, any room member could POST width=1, height=2000000000 — comfortably inside Postgres INTEGER range, so it inserts cleanly — and every viewer of that room gets a ~2-billion-pixel-tall bubble, with no edit path to recover short of a DB fix. That is a genuine escalation in blast radius: before this PR the worst a bogus attachment could do was render a broken-image icon. Bounded at three layers: models Field(gt=0, le=20000) on both, matching the Field(gt=0) pattern this file already uses for client-supplied numerics. 20000 is comfortably past any real image (8K is 7680). migration a CHECK constraint, matching the convention 0021 established. NOT VALID so it governs new writes without scanning existing rows — every pre-existing row is NULL in both columns, and NULL passes a CHECK anyway. Wrapped so a re-run is a no-op. render objectFit: contain and maxHeight, so dimensions that are merely WRONG (rather than absurd) letterbox instead of stretching, and a tall ratio cannot escape the box — maxWidth alone cannot cap height once the ratio drives it. Editing 0040 rather than adding 0041 because it has not been applied anywhere: it is unmerged, and the only database that has seen it is the local throwaway the from-empty replay rebuilds from scratch each run. Adds tests for the bounds — nothing covered these fields before. part of #315 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230
commented
Jul 31, 2026
Review found a real escalation in blast radius — fixed in |
| layer | fix | catches |
|---|---|---|
| model | Field(gt=0, le=20000) | the absurd pair, before it reaches the DB |
| migration | CHECK constraint, NOT VALID | anything that bypasses the API |
| render | objectFit: contain + maxHeight | dimensions that are merely wrong — they letterbox instead of stretching |
le=20000 is comfortably past any real image (8K is 7680). The Field(gt=0) shape matches what models/__init__.py already uses for client-supplied numerics, and the CHECK matches the convention 0021 established. NOT VALID so it governs new writes without scanning existing rows — every pre-existing row is NULL in both columns, and NULL passes a CHECK anyway.
Edited 0040 rather than adding 0041 because it isn't applied anywhere: unmerged, and the only database that has seen it is the local throwaway the from-empty replay rebuilds each run.
Added three tests — nothing covered these fields at all before.
From-empty replay (re-run after the hardening)
RESET_EXIT=0 # supabase db reset + every migration, incl. the new CHECK
JOURNEYS 36 passed
ORACLES 0 finding(s)
Backend pytest 1529 passed, 32 skipped · ruff clean · frontend tsc/lint clean · vitest 415.
Also verified clean by review: the realtime path re-fetches through the decrypting REST endpoint rather than trusting payload.new, and the optimistic append uses the POST response (Prefer: return=representation) — so both pick up the new columns without further change.
Room-chat attachments rendered into a zero-height box until the image loaded. Combined with the
loading="lazy"#312 added, scrolling up through history expanded each image as it neared the viewport and shifted the transcript mid-scroll.It also invalidated the
loadEarlierscrollTop compensation, which measuresscrollHeightbefore the prepended images have loaded.This is also the gap #111 deliberately left open — that PR added
decoding="async"to these images but declined to invent dimensions for natural-aspect user uploads, precisely because there were none stored. Now there are.Why it needed the backend
room_messagesstored onlyimage_url. Nothing to reserve a box with, so it couldn't be fixed in the frontend alone.0040image_width/image_heightonroom_messages, nullablemodelsSendMessageBody, optionalsocial.pyapi.tssendRoomMessagetakes an optionalimageSizeSocial.tsxThe measurement runs on the picked
Filevia an object URL before upload, so the dimensions travel with the message that creates it — history then renders reserved from the first paint rather than after a round trip.Degrades quietly at every layer
That's deliberate, because this is a layout hint and must never be able to block a message:
NULLreadImageSizeresolvesundefinedon a non-image or a decode failure rather than throwingwidth/height/aspect-ratioonly when both values are presentAnything without dimensions renders exactly as it does today. Backfilling would mean fetching every historical image to measure it — not worth it for a layout hint.
Gates
tsc --noEmitclean ·npm run lint0 errors ·npx vitest run58 files / 415 testspytest1526 passed, 32 skipped ·ruff checkcleane2e-upruns against a DB that already has the schema): in a comment belowpart of #315