Skip to content

fix(rewind): say what the rewind could not bring back - #1100

Merged
Juliusolsson05 merged 2 commits into
mainfrom
fix/rewind-attachment-loss
Sep 20, 2026
Merged

Juliusolsson05 merged 2 commits into
mainfrom
fix/rewind-attachment-loss

Conversation

@Juliusolsson05

Copy link
Copy Markdown
Owner

Closes #1075. Follow-up from #929 / #1073.

What was wrong

#1073 made main produce a structured loss report for every attachment on a rewound prompt, and the Rewind picker uses it — a turn with an attachment is labelled [Image prompt] or [Attachment unavailable: shot.png] instead of vanishing from the list.

Nothing used it after the rewind. The user acted on that label and got a composer with nothing in it and no explanation.

The sharpest case has no broken attachment at all: rewinding a Codex, OpenCode or Grok image prompt restores the image perfectly and then drops it on the way into the composer, because those providers declare supportsImageAttachments: false. The picker said [Image prompt], the composer is empty, and the toast said the rewind worked.

The fix

A pure describer reads the report and the provider capability — a restored image is a loss too, when the composer cannot carry it — and the note joins the existing pane toast rather than replacing it, because the undo affordance still has to be advertised:

Rewound to prompt, but 1 attachment (shot.png) did not come back: it is an image and this provider's composer cannot carry images. Undo Rewind available until next submit

Three wording decisions, each with its reason in the code:

  • Up to two names plus a count. A toast listing eight filenames is a toast nobody reads, and "which one is missing?" is the question a person actually has.
  • The ellipsis tracks what is HIDDEN, not how many names exist. 3 attachments (a.png, b.png) reads as a complete list and sends the reader looking for a third name that was never printed.
  • Both reasons when a prompt lost attachments two ways. Picking one describes half the loss.

Tests

The reports are built by the real rewindAttachments from real recorded content (testing/fixtures/image-reads/, captured verbatim from live sessions with only the base64 swapped for a 1×1 PNG), then flattened by the real toRewindSessionAttachments — the same two functions that produce what crosses IPC.

The corpus has exactly one recorded multi-attachment case and it is an exec tool result, not a user prompt; a rewind target is a user prompt, so it cannot stand in for one. Those cases are built from the report schema and labelled as such rather than pretending a recording says something it does not.

Four more drive the real rewindSessionToPrompt through the provider-actions harness and assert what the pane is told, including two controls: a Claude rewind that really did get everything says the ordinary thing, and so does a rewind with no attachments at all.

Mutations, all killed:

mutation result
never mention the loss (the bug) 2 failed
ignore the provider capability 1 failed
a restored image is never a loss 3 failed
count everything, not just what was lost 3 failed
report only the first reason 1 failed
no ellipsis for a partial list 2 failed
name every file 1 failed
unreadable and reference read the same 3 failed

Not decided here, and left as the issue left it: whether a provider that cannot carry images should offer an image-only prompt as a rewind target at all. The rewind still happens and is still useful — the prompt text comes back — so the loss is now stated rather than the target removed.

npx tsc -b clean.

🤖 Generated with Claude Code

Juliusolsson05 and others added 2 commits September 20, 2026 12:09
Closes #1075.

Main has produced a structured loss report for every attachment on a rewound
prompt since #1073, and the Rewind PICKER uses it: a turn with an attachment is
labelled `[Image prompt]` or `[Attachment unavailable: shot.png]` instead of
vanishing from the list. Nothing used it after the rewind, so the user acted on
that label and got a composer with nothing in it and no explanation.

The sharpest case has no broken attachment at all. Rewinding a Codex, OpenCode
or Grok image prompt restores the image PERFECTLY and then drops it on the way
into the composer, because those providers declare
`supportsImageAttachments: false`. The picker said `[Image prompt]`; the
composer is empty; the toast said the rewind worked. So the describer reads the
report AND the provider capability — a restored image is a loss too, when the
composer cannot carry it.

The note joins the existing pane toast rather than replacing it, because the
undo affordance still has to be advertised. It names up to two files and says
how many there were: a toast listing eight filenames is a toast nobody reads,
and two names plus a count is enough to answer the question a person actually
has, which is WHICH attachment is missing. The ellipsis tracks what is hidden
rather than how many names exist — "3 attachments (a.png, b.png)" reads as a
complete list and sends the reader looking for a third name that was never
printed. A prompt that lost attachments two different ways says both reasons,
because picking one describes half the loss.

Tests build the report with the REAL `rewindAttachments` from REAL recorded
content (`testing/fixtures/image-reads/`, captured verbatim from live sessions
with only the base64 swapped), flattened by the REAL `toRewindSessionAttachments`
— the same two functions that produce what crosses IPC. The corpus has exactly
one recorded multi-attachment case and it is an exec tool RESULT rather than a
user prompt, so the several-attachment cases are built from the report schema
and labelled as such instead of pretending a recording says something it does
not. Four more drive the real `rewindSessionToPrompt` and assert what the pane
is told.

Mutations, all killed: never mention the loss 2 failed; ignore the provider
capability 1; a restored image is never a loss 3; count everything rather than
what was lost 3; report only the first reason 1; no ellipsis for a partial list
2; name every file 1; unreadable and reference read the same 3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… true

Review findings on #1100. The deliverable of this PR is one sentence of
English, and the sentence was wrong in four ways.

1. It was FALSE on exactly the providers the issue is about. `unsupported`
   printed "the composer carries images only" without consulting the
   capability — but on Codex, OpenCode and Grok the composer carries NOTHING,
   so a user told their PDF was dropped for not being an image would convert it
   to a PNG and watch that go too. A prompt carrying an image AND a document
   made one sentence assert both halves of a contradiction. Reachable: an
   OpenCode `{type:'file', mime:'application/pdf'}` part decodes to a document,
   and OpenCode declares no image support.

2. Names were listed in attachment order and reasons in category order, so a
   reader pairing them by position paired them wrongly — the PDF read as the
   image. It needs no capability mismatch at all: two Claude images that failed
   differently do it on the provider that works.

   Both are gone because each reason is now attached to ITS OWN file. That
   removes the plural-agreement bug with them — "they are an image", which the
   old wording produced on a real recording.

3. The message was shown for 2000 ms and clamped to three lines, for a sentence
   of up to 333 characters, and the tests passed a `vi.fn()` so nothing covered
   what was painted. The loss case now gets 6000 ms — the house number for a
   toast carrying something to act on — and the duration is asserted.

4. The claim that the corpus has no recorded multi-attachment USER prompt was
   false: `atp-claude-image-inside-codex-rollout.json` is one, with three
   images, decoded here through the real classifier. The real limitation is
   narrower — no recorded user prompt carries a FILENAME — so only the naming
   and truncation cases are schema-built now, and they say so.

Five of the reviewer's mutants survived; all are dead. Two of them were alive
because I had written the unavailable-reason wording twice, so a mutation of
one copy left the other to satisfy the tests. It is written once now.

Mutations after these fixes: ignore the capability for unsupported 1 failed; a
restored image is never a loss 2; names dropped from the phrases 4; the hidden
tail dropped 1; a tail always claimed 1; name every attachment 1; only the
first phrase 3; the subject loses its verb phrase 1; reference and unreadable
read the same 4; phrase order reversed 1; the toast keeps the 2 s default 1;
the call site ignores the capability 1; the loss is never mentioned 2.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Every finding valid, and the first one is the kind I most needed catching: this PR's whole deliverable is one sentence, and that sentence was false on the providers the issue exists for. Fixed in the push above.

1. [BLOCKING] False on every provider that cannot carry images, and self-contradicting on a mixed prompt. Accepted. Your decode chain is right — an OpenCode {type:'file', mime:'application/pdf'} part becomes a document, classify calls it unsupported, and the wording then told the user it was dropped for not being an image on a composer that carries nothing. Converting the PDF to a PNG and watching that go too is exactly the trap.

The fix is the one you pointed at: when the composer carries nothing, every kind collapses into one true clause about attachments, not images. The contradiction case has its own test.

2. [BLOCKING] Names and reasons ordered by different keys. Accepted, and this is the better fix for finding 4 as well: each reason is now attached to its own file — broken.png (the record could not be decoded); design.png (the transcript recorded a reference, not the file). Nothing has to be paired by position, and the plural-agreement bug ("they are an image", which your probe reproduced on a real recording) disappears with the grouping rather than being patched.

Phrase order follows the attachments, so what the reader sees first is what came first in their prompt. Asserted.

3. [BLOCKING] 2 seconds, three lines, and a vi.fn() where the paint should be. Accepted. The loss case now passes 6000 — your house-number table is the argument, and the ordinary success toast keeps the default because "it worked" needs no reading time. The duration is asserted in the renderer test, and the sentence is much shorter now that each file carries its own short reason.

I did not change PaneToast's clamp: three lines is a deliberate, tested contract, and the right response was to make the message fit it rather than to widen the surface for one caller.

4. "they are an image". Gone with the regrouping, and there is a case on the recorded three-image prompt asserting it cannot come back.

5. [BLOCKING for honesty] The fixture claim was false. You are right, and I have replaced the claim with the narrower true one: atp-claude-image-inside-codex-rollout.json is a recorded user prompt with three images, and it is now the multi-attachment case, decoded through the real classifyClaudeRecord → decodeClaudeConversation → rewindAttachments → toRewindSessionAttachments chain. What the corpus genuinely lacks is a recorded user prompt with a filename, so only the naming and truncation cases are schema-built — and the comment says exactly that now.

6. Mutations. All five survivors are dead:

mutation result
the hidden tail is dropped 1 failed
a tail is always claimed 1 failed
name every attachment 1 failed
the subject loses "did not come back" 1 failed
reference and unreadable read the same 4 failed

Two of those were alive for a reason worth recording: I had written the unavailable-reason wording twice, once per capability branch, so mutating one copy left the other to satisfy the tests. It is written once now — which is also why your M19 killed and M9-equivalent did not.

Full post-fix battery (13 mutations): ignore the capability for unsupported 1; a restored image is never a loss 2; names dropped 4; hidden tail dropped 1; tail always claimed 1; name every attachment 1; only the first phrase 3; subject loses its verb phrase 1; reference vs unreadable 4; phrase order reversed 1; toast keeps the 2 s default 1; call site ignores the capability 1; the loss is never mentioned 2.

npx tsc -b clean; src/renderer + src/main/providerSwitch 363 files / 2706 tests pass.

@Juliusolsson05
Juliusolsson05 merged commit 92c7013 into main Sep 20, 2026
2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the fix/rewind-attachment-loss branch September 20, 2026 20:30
Juliusolsson05 added a commit that referenced this pull request Sep 20, 2026
…erge

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

Rewind does not tell the user an attachment could not be restored

1 participant