Skip to content

Chat: image attachments that can't brick a session, file-chip rendering, and a retry that doesn't duplicate the reply - #210

Open
Olga Lavrichenko (OLavrik) wants to merge 5 commits into
mainfrom
olga.lavrichenko/review
Open

Chat: image attachments that can't brick a session, file-chip rendering, and a retry that doesn't duplicate the reply#210
Olga Lavrichenko (OLavrik) wants to merge 5 commits into
mainfrom
olga.lavrichenko/review

Conversation

@OLavrik

Copy link
Copy Markdown
Collaborator

Summary

Four independent fixes to the chat surface, all in apps/web/src/chat plus one server-side guard. The headline is the image-attachment fix: a single oversized pasted image used to permanently break a chat —
pi re-sends the whole history every turn, so once Anthropic's cap rejected that image, every subsequent turn 400'd and the conversation was unrecoverable.

Oversized images — two lines of defense

Anthropic caps an image side at 8000px, dropping to 2000px once a single request carries more than 20 images. The composer was sending pasted images raw, and pi's own resizer is deliberately off server-side
(images.autoResize:false — the photon/WASM bundling problem).

Client — apps/web/src/chat/imageAttachment.ts (new). Pasted and dropped images are decoded in the browser and downscaled to a 1568px long edge (Claude's own standard-tier edge) before becoming ImageContent.
Within-bounds images pass through byte-identical; an undecodable file falls back to raw. The pending chip shows mime · W×H.

Server — packages/server/src/agent/imageGuard.ts (new). An inline pi extension on the context event (fired before every LLM call, live sessions included) sniffs each image block's dimensions straight from
the base64 header bytes — PNG/JPEG/GIF/WebP, no codec, never strips what it can't sniff — and replaces any block over the cap with a text note carrying the W×H and a re-attach hint.

Sessions are append-only ("pi owns state") and the host has no image codec, so the guard transforms the outgoing context only — the session file and the visible transcript stay untouched. That's what
un-bricks the already-poisoned chats: they recover on their very next message. The count-aware cap also self-heals the read-tool case — a raw 3000px read is legal while the context holds ≤20 images and
degrades to a note (not a brick) once the session crosses 21.

Attachments render as file chips

UserTurn shows image blocks as compact "attached file" chips above the text rather than inline previews; clicking opens the image in a dialog (the diagram-fullscreen pattern). The chip label is the picked
file's name, carried on the echo turn as attachmentNames — UI-side only, since pi's ImageContent has no filename field — index-aligned with the image blocks. A hydrated turn has no names and falls back to
mime-type labels.

New ChatAttachment type (name + content) threads the filename from composer to echo turn without touching the wire.

No horizontal scroll in the transcript

Long unbroken tokens in the user bubble (URLs, pasted paths) pushed it wider than the container. break-words on the bubble and overflow-x-hidden on the Virtuoso scroller — wide content scrolls inside its own
code/diff block, never the whole chat.

Auto-retry no longer duplicates the reply

pi's _prepareRetry removes the failed attempt's assistant message from the transcript before re-running the turn, and the retried run re-streams the reply as a brand-new message. The reducer only showed a
countdown on auto_retry_start and kept the failed attempt's turn, so the client rendered the reply twice (frozen failed partial + retried copy) while pi's transcript — and any reloaded client hydrating it —
held one. The reducer now mirrors pi's slice: drop the trailing assistant turn on auto_retry_start, skipping client-only retry-countdown turns, conservative like pi's own last-message guard.

Testing

  • Unit: imageAttachment.test.ts (downscale math, passthrough, fallback), imageGuard.test.ts (hand-built header bytes per format, both caps, the count threshold), rows.test.ts (attachment names on derived
    rows), appStore.test.ts — including a red-first reducer test replaying pi's exact retry event sequence (message_end error → agent_end willRetry → auto_retry_start → retried stream ⇒ exactly one assistant
    turn) plus the error-before-message_start edge.
  • E2E: e2e/composer-images.spec.ts (no-agent — attach, chip metadata, downscale assertions). A true e2e repro of the retry bug needs a mid-turn provider network failure the real-provider-only agent suite
    can't induce; instead e2e/hydrate-midstream.live.spec.ts (AGENT, Inc. (@agent)) pins the neighbouring invariant that falsified the first hypothesis — reloading mid-stream hydrates exactly one copy of the streaming
    message.
  • Gates (all green on the rebased branch): check:deps, check:seams, lint, typecheck, test (570 pass / 0 fail).

Specs updated: apps/web/src/chat/SPEC.md, apps/web/src/store/SPEC.md, packages/server/src/agent/SPEC.md.

Long unbroken tokens in the user bubble (URLs, pasted paths) pushed it wider than the container; add break-words there and overflow-x-hidden on the Virtuoso scroller — wide content scrolls inside its own code/diff block, never the whole chat.
…context

An oversized attached image bricked the chat: the composer sent pasted
images raw, and once Anthropic's >20-image 2000px per-side cap kicked in,
the image in the re-sent history 400'd every subsequent turn.

- web: new chat/imageAttachment.ts — pasted/dropped images are decoded in
  the browser and downscaled to a 1568px long edge (Claude's own
  standard-tier edge) before becoming ImageContent; the pending chip shows
  mime · W×H.
- server: new agent/imageGuard.ts — an inline pi extension on the context
  event sniffs each image's dimensions from its base64 header bytes (no
  codec) and replaces blocks over the provider cap (8000px, 2000px when
  the context carries >20 images) with a text note. Sessions are
  append-only and the host has no image codec, so only the outgoing
  context is transformed — an already-poisoned chat recovers on its next
  message, file and transcript untouched.
- specs updated (agent SPEC imageGuard bullet, chat SPEC composer bullet);
  unit tests for both halves + a no-agent e2e (composer-images.spec.ts).
…licated replies after a provider 'fetch failed'

pi's _prepareRetry removes the failed assistant message from the transcript
before re-running the turn, and the retried run re-streams the reply as a
brand-new message. The reducer only showed a countdown on auto_retry_start
and kept the failed attempt's turn, so the client rendered the reply twice
(the frozen failed partial + the retried copy) while pi's transcript — and
any reloaded client hydrating it — held one. Mirror pi's slice: on
auto_retry_start, drop the trailing assistant turn (skipping client-only
retry countdown turns), conservative like pi's own last-message guard.

Red-first coverage: a reducer unit test replaying pi's exact retry event
sequence (message_end error → agent_end willRetry → auto_retry_start →
retried stream ⇒ exactly one assistant turn) plus the error-before-
message_start edge. A true e2e repro needs a mid-turn provider network
failure the real-provider-only agent suite can't induce; instead a new
@agent spec pins the neighbouring invariant that falsified the first
hypothesis — reloading mid-stream hydrates exactly one copy of the
streaming message (getMessages returns only committed messages).

@jetbrains-air jetbrains-air 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.

Requesting changes — please address the blocking inline findings.

const turns = [...rt.turns];
let last = turns.length - 1;
while (last >= 0 && turns[last]?.kind === "retry") last--;
if (turns[last]?.kind === "assistant") turns.splice(last, 1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking:
Problem: pi 0.82.1 removes the failed assistant only from agent.state.messages; message_end has already appended it to SessionManager, so this reducer now diverges from the persisted transcript and violates hydrate-then-stream.
Failure scenario: after an automatic retry succeeds, the live client shows one assistant turn, but reloading rehydrates the persisted failed assistant plus its synthesized error and the successful reply, so the duplicate reappears and clients disagree.
Suggested fix: make live reduction and messagesToRuntime apply the same retry-attempt presentation rule based on pi’s actual persisted sequence, and cover a persisted failed-attempt-plus-success hydration case.

if (!base64) return undefined;
let bytes: Buffer;
try {
bytes = Buffer.from(base64, "base64");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking:
Problem: this purported header sniff decodes the entire base64 image, and guardOversizedImages calls it repeatedly for violating blocks on every provider request, creating unbounded allocations in the in-process host.
Failure scenario: a supported raw read or legacy attachment containing a large compressed image makes each turn allocate full decoded copies during the context hook; several such images can exhaust memory and take down every session with the host.
Suggested fix: decode only a bounded base64 prefix needed by the format parsers (with a bounded JPEG marker scan), and compute/cache each block’s dimensions once per guard pass.

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.

2 participants