Skip to content

Copy placed images to the next or previous cloud page - #272

Open
SunkenInTime wants to merge 33 commits into
t3/cloud-page-copyfrom
t3/cloud-image-copy
Open

SunkenInTime wants to merge 33 commits into
t3/cloud-page-copyfrom
t3/cloud-image-copy

Conversation

@SunkenInTime

@SunkenInTime SunkenInTime commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

An image copied while its upload was held, shown on the copy

After #267, "Copy to next page" and "Copy to previous page" work on cloud strategies for everything except placed images. This adds images.

What changes

Changed since the first version

The first version asked the server to copy the picture under the copy's id first (#271), and refused to copy an image that was still uploading. Dara pointed out that the waiting was the wrong model. #271 is closed. This branch takes its server code back out and builds on #276/#277 instead.

Ground truth

Web build in Edge against the dev deployment, which runs this stack, signed in as a test account on localhost:8765:

  • I placed a new image with its upload held for 20 seconds. Frames above: just placed, then on the copy during the hold, then after it landed.
  • Right-click → "Copy to next page" on that image put a copy on the next page with assetId naming its picture, and no new picture rows. The next page showed it.
  • In a build of main's client code, standing in for an old build, the copies showed their pictures.

Tests

strategy_page_session_provider_test, "an image":

  • it is offered both neighbours, and is copied showing its picture, whatever state its upload is in;
  • a copy's copy shows the first picture.

The full suite passes on the combined branch (#274).

Merge order: #276, #277, then this (it contains #267's changes and both of those).

ci: test analyze web

🤖 Generated with Claude Code

SunkenInTime and others added 14 commits October 9, 2026 14:54
Resolving an unrelated cloud conflict (Use cloud, Keep both) reloads the
page on screen, which closed Edit placement and threw away the user's
drags. A reload of the same page, the one that keeps undo history, now
keeps the edit open on whichever of its lineups survive. A drag stays
while the reload has its end where the drag started; an end the cloud
moved or removed shows the cloud's version, with a toast. Opening another
page still ends the edit without writing it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A reload in the middle of a drag (pointer still down) left the editor's
drag based on the old position: the next pointer move put back the drag
the reload had just undone, and when the dragged end was gone, the next
drag of another end started from its numbers. The editor now lets a drag
carry on only if the reload kept its end where the drag began.

fromHive sets the reopened edit in one assignment, so nothing watching
sees the edit end on the way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The editor compared a reload against where its own gesture began, which
for a second drag of an end is the earlier draft, not the saved position
fromHive compares against. The two could disagree: a reload that moved
the spot to the first draft dropped the drag, and the editor then put it
back. The editor now asks the reopened edit whether it kept the draft.

A drag taken back on an end the reload removed no longer outlives its
gesture, and dragging another end clears it.

The screenshot view's lineup fake takes fromHive's new samePage flag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A reload that took a drag back and a second reload that removed its end,
both before the pointer lifted, left the marker set: when a later reload
brought the end back, its first drag was ignored. Every reopen now clears
a marker whose end is no longer in the edit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use cloud and Keep both reloaded the whole page to show the cloud's
version, which closed Edit placement and threw away its drags even when
the conflict was on another lineup. Teammates' changes already apply item
by item (#208); conflict resolution now goes the same way, holding what
the user has open except what they chose the cloud's version of. A
placement edit of a lineup in the conflict ends.

This replaces the earlier commits' reopen-after-reload logic in the lineup
provider and editor, which go back to #265's versions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When part of Use cloud's discard fails, the redraw that follows puts the
user's version back on every refused entity, held or not; holding one
kept the cloud's version on screen while the work still waited.

A teammate's change held back by a hold the user let go of during the
discard waited for a reapply that conflict resolution then cleared. It
now resumes once resolution ends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a server that merges by field, Save names only the spots the user
dragged, measured from the version the edit started from: the user's
drag wins over a teammate's move of that spot, and the teammate's move
of another spot the edit held back stays. The test that Save waits as a
conflict now says it runs on a server that checks whole groups.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve cloud conflicts item by item so Edit placement stays open
A placed image's id is also its image's id, so copying one to another
page of a cloud strategy needs a row of its own. images:copyAsset gives
the copy's id a row pointing at the same stored bytes, as a duplicated
strategy's images do, taking over a placeholder if the copy's content
landed first. Additive: no client calls it yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An image's id also names its picture on the server, so a copy, which gets
an id of its own, first has the server give that id the original's
picture (images:copyAsset, sharing the stored bytes), and only then is
sent. An image still uploading, or one the cloud cannot show, is not
copied, and a toast says why.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Astra's review: a copied image whose content never reaches the server
kept its row, and so its bytes, for good. A day after each copy, the copy
now goes through the reclaim check, which removes it only if nothing
shows it. A failed upload under the copy's id no longer reads as one
still on its way: the copy replaces it. Tests cover both, and a viewer
collaborator being refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3473e749-6057-4d34-a73c-1cc2fa52f441

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • 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

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

SunkenInTime and others added 3 commits October 10, 2026 12:37
Astra's review of the app side: a copy can wait in a device's outbox for
days and still lands needing its picture, so the day-later reclaim of
unused copies broke copies delivered late. Copied rows now go as any
image does, once content that showed it is purged or with the strategy;
a copy whose content never arrives keeps its row until then.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Astra's review: an image placed on this device whose upload had not
reached the server yet read as one the cloud doesn't have. The copy now
checks the upload queue and says it is still uploading. The test also
proves the copy waits for its picture before it is queued.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SunkenInTime and others added 3 commits October 10, 2026 12:42
CodeRabbit's review: an active row under the target's id counted as the
copy whatever it showed. It now counts only if it has the source's
stored bytes; otherwise the copy is refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Astra's review: an image whose own save failed keeps an upload job that
never runs, since an upload waits for its image to be saved to send. A
copy of it said "still uploading" for good. Only uploads that will go
count now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SunkenInTime and others added 5 commits October 10, 2026 15:40
An image element may name the picture it shows (`assetId`); without
one, its own id names it, as before. A copy of an image can then be a
new element showing its original's picture, with nothing copied or
uploaded, and nothing to wait for while the original is still
uploading.

References, upload placeholders and cleanup all read the picture
through collectAssetIdFromElementPayload, so they follow. For builds
from before the field, page and strategy snapshots also list each
picture under the id of every image showing it, and a whole write that
leaves the field out keeps it: an image never changes picture.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A placed image may name the picture it shows (assetId, Hive field 11);
without one, its own id names the picture, as before, so every image,
.ica file and backup made so far reads exactly as it did. pictureId is
the id the picture is stored, uploaded and found under: rendering, the
page transition, screenshots, video export, the upload queue, cloud
export, local-to-cloud migration and the cleanup of unused picture
files all use it. Item identity (undo, transitions, sync rows, hero
tags) stays the image's own id.

Nothing makes such images yet; copies will (see the server half).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copies will show their original's picture instead (see the picture id
change), so the server never needs to copy a picture within a strategy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A copied image now names its original's picture (assetId), so copying
needs no call to the server first, and an image still uploading copies
like any other: its copy shows the picture once the upload lands.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SunkenInTime and others added 8 commits October 10, 2026 16:35
Three gaps for builds and rows from before picture ids:
- Image payloads go without assetId to clients that don't ask for it
  (acceptsPictureIds on the snapshots and applyBatch): such a client
  keeps no assetId, so a copy would read to it as an unsaved change.
- images:getAssetUrl answers for an image's own id with the picture it
  shows, as old builds ask when a picture's address expires.
- A picture from before upload statuses is looked up among its own
  strategy's rows first: copies keep pictures' ids across strategies,
  so twenty copies could fill a lookup of every strategy's rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This client keeps PlacedImage.assetId, so it reads snapshots and op
results with it (acceptsPictureIds); older clients get them without.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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