Let an image show another image's picture

[SunkenInTime]

A placed image's id is also the name of its picture. A copy of an image (cloud "+", copy to page, duplicate) needs a new id, so until now it also needed a new picture entry, and that entry can only point at bytes whose upload has finished. That is why "+" could leave out an image that was still uploading. This PR is the server half of fixing that at the root: an image can name the picture it shows.

What changes

• An image element may carry assetId, the picture it shows. Without one, its own id names the picture, exactly as today. collectAssetIdFromElementPayload reads assetId ?? id, and references, upload placeholders and cleanup all go through it, so a picture several images show stays alive while any of them does.
• Old builds keep working:
  • Page and full snapshots also list each picture under the id of every image that shows it (withPictureAliases). Shipped desktop builds and the live web build look a picture up by the image's own id, so they find it there. This is worked out when the snapshot is read and adds no database rows.
  • A write that leaves assetId out keeps the stored one (keepPictureId). Old builds rewrite an image's whole payload when it is moved, or brought back with undo, and don't know the field. An image never changes picture.
  • Clients that don't send acceptsPictureIds: true get image payloads without assetId, in snapshots and in refused-op results. They keep no such field, so otherwise every copy would read to them as an unsaved change of their own.
  • images:getAssetUrl answers for an image's own id with the picture it shows. That's how old builds ask when a picture's link expires.
• Legacy pictures stay findable. A picture from before upload statuses is now looked up by index range: this strategy's rows from before statuses, then rows of no strategy (active, or from before statuses), five each. Before, the lookup read the newest 20 rows across all strategies, and copies into other strategies keep the picture's id, so they could crowd it out. Reading by status also keeps failed upload attempts from crowding it out. The lookup stays within the 22 rows a copy's budget charges for.
• Nothing writes assetId yet. The client half (Find an image's picture by its picture id #277, stacked on this) teaches the app to read it, and the copy PRs start writing it.

Ground truth

On dev, with every PR in this stack deployed, I built a web app from main's client code, standing in for an old build. It opened pages full of copies made by the new code, and showed each copy's picture through the aliases:

In the same old build, opening those pages rewrote nothing (every copy stayed at revision 1), and moving a copy kept its picture id on the server.

Tests

convex/pictureId.test.ts:

• a copy shows the picture, and both snapshots list it under the copy's id too;
• the copy keeps the picture referenced after the original is deleted;
• a whole write from an old build keeps the picture;
• an undo from an old build keeps the picture;
• payloads go with the picture id only to clients that ask for it;
• a picture's address is given for an image's own id;
• a legacy picture is found beside 20 copies in another strategy and 10 failed attempts in its own.

The full Convex suite passes (276 tests).

Astra found the old-build payloads, the address lookup and the legacy lookup in its first review. A second review confirmed the first two fixes and found that my first legacy fix, ten rows per owner, could still be crowded out by failed attempts. Reading by status fixes that, and the test fails on the earlier version.

This changes how the server reads what an image shows, so it's a sync-format change: you look first, and it deploys before anything writes assetId.

ci: convex contract

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 77a54c44-dfab-4060-bc02-71c12dc78b20

---

---

---

---

---

---

---

📥 Commits

Reviewing files that changed from the base of the PR and between 830f3b4 and 09301ab.

---

---

---

---

---

---

---

📒 Files selected for processing (5)

• convex/lib/imageAssets.ts
• convex/ops.ts
• convex/page.ts
• convex/pictureId.test.ts
• convex/strategy.ts

---

---

---

---

---

---

---

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

---

---

---

---

---

---

---

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

Walkthrough

The changes retain existing picture asset IDs during element updates and add aliases for image copies in page and full snapshots. Tests cover snapshot URLs, deletion of the original image element, and older-build patch and restore operations.

Changes

Picture references

Layer / File(s)	Summary
Resolve and preserve picture IDs convex/lib/imageAssets.ts, convex/ops.ts, convex/pictureId.test.ts	Asset ID resolution prefers a non-empty assetId and otherwise uses id. Element additions and patches preserve an existing picture ID when the incoming payload omits it. Tests cover older-build patch and restore operations.
Add picture aliases to snapshots convex/lib/imageAssets.ts, convex/page.ts, convex/strategy.ts, convex/pictureId.test.ts	Page and full snapshots add aliases for eligible live image elements. Tests check that image copies share the picture URL and that the asset remains available after deletion of the original element.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

---

---

---

---

---

---

---

---

---

---

---

---

---

---

---

---

---

---

---

---

---

---

Merge Risk: 🔵 Low · up to 09301

In an ID-collision case, an older client can show the wrong picture. The change remains mergeable with owner awareness and follow-up on collision prevention.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly summarizes the main change: an image can display another image's picture.

---

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

---

---

---

---

---

---

---

---

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

---

---

---

---

---

---

---

---

---

---

---

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}