Skip to content

fix(files): reserve image layout space so images stop reflowing on load - #6299

Merged
waleedlatif1 merged 10 commits into
stagingfrom
fix/files-image-layout-shift
Aug 6, 2026
Merged

fix(files): reserve image layout space so images stop reflowing on load#6299
waleedlatif1 merged 10 commits into
stagingfrom
fix/files-image-layout-shift

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • A markdown image with no stored dimensions reserved zero vertical space until it downloaded, then snapped to its natural height and pushed everything below it down (cumulative layout shift). This reserves the box up front from the image's intrinsic aspect ratio.
  • Dimensions are stored as workspace_file metadata (not in the markdown — it stays clean ![](src)), read synchronously from the already-loaded file list to reserve a responsive aspect-ratio box on the first render, and lazily backfilled once per image on first view via a write-gated, idempotent PATCH.
  • The node view falls back to on-load measurement for the very first view and for external ![](url) images. Images stay fluid (max-width:100%, height:auto).

How it works

  • DB: nullable width/height on workspace_files (migration 0282).
  • Server: mapper carries the columns; updateWorkspaceFileDimensions is idempotent (WHERE width IS NULL) and never bumps updatedAt (so it can't cache-bust the served image); PATCH …/[fileId]/dimensions is write-gated.
  • Client: the dimension capability lives on the FileContentSource seam (only the workspace source implements it); the adapter reads/patches the file-list cache; image.tsx reserves the box (memoized so a resize drag never re-scans the list) and measures + persists on load.

Notes

  • Only remaining reflow is an image's very first-ever view — dimensions can't be known before the first load; every later view (incl. hard refresh) is reflow-free.
  • Markdown is never touched.

Type of Change

  • Bug fix

Testing

  • Route test (auth / permission / validation) and a pure resolver unit test (src → record). Typecheck, api-validation (+strict), react-query, client-boundary all green. Ran /simplify + /cleanup over the diff.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

A markdown image with no stored dimensions reserved zero vertical space until
it downloaded, then snapped to its natural height and pushed content below it
down (cumulative layout shift). Reserve the box up front from the image's
intrinsic aspect ratio instead.

Store intrinsic width/height as workspace_file metadata (not in the markdown —
it stays clean `![](src)`), read it synchronously from the already-loaded file
list to reserve a responsive aspect-ratio box on first render, and lazily
backfill it once per image on first view via a write-gated, idempotent PATCH.
The node view falls back to on-load measurement for the first-ever view and for
external images. Images stay fluid (max-width:100%, height:auto).
@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 6, 2026 12:49am

Request Review

@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Adds a new authenticated write API and DB columns on file metadata with optimistic client cache updates; scope is rendering hints only, with key guards limiting stale writes after content swaps.

Overview
Fixes cumulative layout shift when markdown images load by reserving space from intrinsic width/height stored on workspace_files (not in markdown). Nullable width/height columns are added via migration 0282; list records and content replacement clear dimensions so stale aspect ratios cannot stick.

A write-gated PATCH …/files/[fileId]/dimensions persists the browser’s measured size when it disagrees with cache/DB. Writes commit only when the row still has the client’s storage key, so in-flight measurements for replaced bytes are rejected (success: false triggers a list refetch).

On the client, optional getImageDimensions / reportImageDimensions on FileContentSource (workspace viewer only) read synchronously from the active file-list cache and fire-and-forget PATCH with optimistic cache updates. The TipTap image.tsx node applies width + aspect-ratio from stored metadata or on-load measurement, and reports mismatches so EXIF-corrected or post-swap sizes self-correct.

Reviewed by Cursor Bugbot for commit d2b1195. Configure here.

Comment thread apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts
Comment thread apps/sim/hooks/queries/workspace-files.ts Outdated
Comment thread apps/sim/hooks/queries/workspace-files.ts Outdated
@greptile-apps

greptile-apps Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds persisted intrinsic image dimensions so markdown images can reserve responsive layout space before loading.

  • Adds nullable workspace-file dimensions and the corresponding database migration.
  • Adds a permission-gated, content-version-aware dimensions endpoint.
  • Resolves, caches, measures, and reconciles image dimensions through the workspace file-content source.
  • Clears dimensions when file content is replaced to prevent stale aspect-ratio metadata.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image.tsx Reserves image space from stored or measured intrinsic dimensions and backfills mismatches after load.
apps/sim/hooks/queries/workspace-files.ts Adds the cache-backed dimensions adapter, optimistic metadata updates, guarded persistence, and stale-write reconciliation.
apps/sim/app/api/workspaces/[id]/files/[fileId]/dimensions/route.ts Adds an authenticated, write-gated endpoint that accurately reports content-version guard rejection.
apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts Maps and persists intrinsic dimensions while clearing stale values on content replacement.
packages/db/migrations/0282_chubby_psylocke.sql Adds nullable width and height columns to workspace_files.

Sequence Diagram

sequenceDiagram
  participant UI as Markdown image
  participant Cache as Workspace file cache
  participant API as Dimensions API
  participant DB as workspace_files
  UI->>Cache: Read stored width and height
  Cache-->>UI: Known dimensions or null
  UI->>UI: Reserve aspect-ratio box
  UI->>UI: Load image and measure natural size
  UI->>Cache: Optimistically store measured dimensions
  UI->>API: PATCH key, width, height
  API->>DB: Update where workspace, file, and key match
  DB-->>API: Written or rejected
  API-->>UI: success true or false
  alt Content key changed
    UI->>Cache: Invalidate active file list
  end
Loading

Reviews (10): Last reviewed commit: "docs(files): align stale dimension docs ..." | Re-trigger Greptile

Comment thread apps/sim/hooks/queries/utils/find-workspace-file-by-src.test.ts Outdated
…tent swap

- onLoad guards on the memoized storedDimensions the render uses (not a fresh
  cache read), so a sibling's non-reactive backfill can't leave a view unreserved.
- updateWorkspaceFileContent clears width/height when it swaps bytes, so stale
  dimensions can't be reserved for new content (and the null re-enables backfill).
- Keep optimistically-cached dimensions when the PATCH fails (correct measurement;
  a 403/transient error shouldn't wipe sibling reservations).
- Test imports the sibling via the absolute @/ path.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts
…aring

Clearing width/height to NULL on a content swap reopened the width IS NULL
backfill path, so a late fire-and-forget PATCH for the previous image could
write its stale size onto the new content. Instead, measure the new bytes'
intrinsic dimensions server-side (image-size, headers only) and store those
(or null for a non-image), so the row always matches the current content and a
stale backfill can't apply.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts Outdated
Comment thread apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts Outdated
…rver-measuring

Round-3 review: server-side image-size returns raw (non-EXIF) dimensions, and
clearing dims on content swap reopened the stale-PATCH race for non-image or
unmeasurable content. Move authority to the browser's own naturalWidth/Height
(EXIF-correct): the node view reserves from it and reports on any mismatch, and
updateWorkspaceFileDimensions overwrites (no width IS NULL gate) so stale values
self-correct on the next view. Reverts the server-side measurement and the
content-swap dimension touch entirely.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 17499b1. Configure here.

The self-heal rework left the old image's dimensions in the row after a content
replacement, so the next view of the new bytes reserved a wrong-sized box before
correcting. Clear width/height on the content-swap write so the row never
describes stale content: the next view falls back to the baseline first-load
reflow and the browser's measurement backfills the correct size. No server-side
decode (EXIF-safe), and the client's overwrite-on-mismatch handles a late PATCH.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Addressed the remaining summary concern (stale metadata after content replacement): updateWorkspaceFileContent now clears width/height on a content swap, so the row never describes old bytes. Combined with the browser-authoritative self-heal (overwrite on mismatch), the next view of replaced content reserves nothing → the baseline first-load reflow → then backfills the correct size, and a late in-flight PATCH is corrected on the next view. No server-side image decode, so no EXIF-orientation issues.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit ba8507c. Configure here.

…n't persist

Ties the dimensions write to the storage key the client measured. The key is
regenerated on every content replacement, so an in-flight PATCH measured against
superseded bytes is rejected at the DB (WHERE key = measured key) instead of
persisting the old aspect ratio for new content. Closes the last stale-ordering
window Greptile flagged — the write is now content-version-conditioned, not just
corrected on the next render.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Closed the stale-ordering window at the write, as requested: updateWorkspaceFileDimensions now commits only WHERE key = <the key the client measured>. The storage key is regenerated on every content replacement, so an in-flight PATCH for superseded bytes is rejected at the DB rather than persisting the old aspect ratio — the write is content-version-conditioned, not merely corrected on the next render.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit cd1b2be. Configure here.

…p pass)

Post-review /cleanup: the dimensions route TSDoc still described backfill-once
behavior (now overwrite-on-mismatch via the content-key CAS); the bare-pixel
width regex is hoisted to module scope. No behavior change.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 94348f0. Configure here.

Comment thread apps/sim/app/api/workspaces/[id]/files/[fileId]/dimensions/route.ts Outdated
…ns response

The route returned success:true even when updateWorkspaceFileDimensions matched
0 rows (the CAS rejected a write whose measured key no longer matches the row).
Return success:<whether a row was written> and widen the contract response to
{ success: boolean }. Not an error path — the client's next measurement persists
once its file list has the new key; this just stops the API claiming a persist
that did not happen.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 497b223. Configure here.

Comment thread apps/sim/hooks/queries/workspace-files.ts Outdated
…sion-rejected

Previously the client discarded a success:false (CAS-rejected) response, leaving
its optimistic patch — which is for superseded bytes — lingering in the file-list
cache. On rejection, invalidate the list so the cache reconciles with the new
content (whose real size persists on its next load). Deliberately NOT a retry:
re-sending the old measurement under the new key would write the wrong size. A
transport error / read-only 403 still keeps the optimistic value (it's the real
displayed size).
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4cabb83. Configure here.

…behavior

Cleanup audit: the ImageDimensionsSource/reportImageDimensions interface docs and
one route log string still said backfill-once/no-op; the mechanism overwrites on
mismatch to self-correct. Wording only, no behavior change.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d2b1195. Configure here.

@waleedlatif1
waleedlatif1 merged commit 5dbe95e into staging Aug 6, 2026
24 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/files-image-layout-shift branch August 6, 2026 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant