fix(desktop): keep attached images renderable across session switches and restarts#71121
Open
OutThisLife wants to merge 5 commits into
Open
fix(desktop): keep attached images renderable across session switches and restarts#71121OutThisLife wants to merge 5 commits into
OutThisLife wants to merge 5 commits into
Conversation
The desktop gateway passed the vision-enriched, model-only message text (carrying an `image_url:<path>` hint) straight into run_conversation as the persisted user turn. The renderer only parses `@image:<path>`, so it could not rebuild the attachment from history: after a restart the image was gone and only the caption survived, and on a live session switch the warm cache disagreed with the authoritative text and the frontend "rescued" the image by appending it after the caption. run_conversation already supports persist_user_message for exactly this "what the model sees" vs "what gets stored" split; it was simply never wired up for the attachment path.
Persisted history carries no attachment metadata for non-image refs, so resume reconciliation dropped `@file:` chips off a user turn whose text matched. Carry the warm cache's refs forward when the resumed message has none of its own, never replacing refs that are already present. (cherry picked from commit eac5b0a)
The unquoted alternative in the directive pattern is `\S+`, so a ref built by string interpolation truncates at the first space and strands the tail as loose text next to a broken thumbnail. Composer images live in the app's userData dir, which on macOS is `~/Library/Application Support/<App>/` — so every pasted or dropped image hit this. Adds format_reference_value next to REFERENCE_PATTERN, mirroring formatRefValue in the desktop's directive-text.tsx, and covers the round-trip through the parser.
…s too A turn routed to a model that takes pixels directly sends `content` as a parts list, and the session store deliberately ignores a plain-string persist override for a list payload — a text override must not erase a turn's image summary. So the override was dropped for every user on a vision-capable main model, and the durable row kept only the caption plus a literal `[Image attached at: ...]` / `[screenshot]`, which the renderer cannot turn back into an image. Only vision-preprocessed (text-mode) turns were actually fixed. Mirror the shape instead: swap the text part for the `@image:` ref form and keep the image parts, so the model still has the pixels for the rest of the session, and drop the `[screenshot]` stand-in on the way into the bubble when a ref was lifted from the same message.
Matches the two derived values above it and fixes the indentation.
Contributor
૮ >ﻌ< ა ci reviewran on a7182e2 ℹ️ InfoDesktop E2E visual evidence · View test artifacts · View job1 visual diff. inline evidence upload failed. Failed to upload diff-665a0833239e-onboarding-overlay-diff.png with gh image (exit code 1): Error uploading /home/runner/work/_temp/e2e-evidence/diff-665a0833239e-onboarding-overlay-diff.png: step 0 (get upload token): uploadToken not found on repo page — do you have write access to NousResearch/hermes-agent? (or, if NousResearch enforces SAML SSO, authorize at https://github.com/orgs/NousResearch/sso) |
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An attached image in the desktop chat only rendered for the life of the turn that sent it. The gateway persisted the message text it had built for the model — a vision description plus an
image_url:<path>hint, or a flattened parts list — and the renderer only understands@image:<path>. So history had no form the UI could turn back into an image: after a restart the caption survived alone, and on a session switch the warm cache disagreed with the stored text and the frontend "rescued" the image by appending it after the caption.run_conversationalready supportspersist_user_messagefor exactly this "what the model sees" vs "what gets stored" split. This wires it up for the attachment path and makes the stored form actually renderable in both image-input modes, then fixes the two frontend bugs that were dormant until refs started reaching history — a duplicated user bubble from comparing raw text, and the thumbnail sitting inside the bubble's ~2-line clamp where it pushed the caption out of view.Supersedes #70720 and #68176, whose work this builds on directly.
Beyond the superseded PRs
Two gaps that kept the original fix from covering the bug:
contentas a parts list, and the session store deliberately ignores a plain-string persist override for a list payload (a text override must not erase a turn's image summary), so the override was silently dropped. The stored row kept only the caption plus a literal[Image attached at: …]/[screenshot]. The override now mirrors the shape — the@image:ref replaces the text part and the image parts stay, so the model still has the pixels for the rest of the session.\S+, so@image:/Users/me/Library/Application Support/…/a.pngparsed as@image:/Users/me/Library/Applicationand stranded the tail as loose text. Composer images live in the app's userData dir, which on macOS is~/Library/Application Support/<App>/— so this hit every pasted or dropped image there. Refs are now quoted through aformat_reference_valuehelper that sits next toREFERENCE_PATTERNand mirrorsformatRefValueon the TS side.Testing
scripts/run_tests.sh tests/test_tui_gateway_server.py tests/agent/test_context_references.py tests/run_agent/test_run_agent.py tests/test_lazy_session_regressions.py tests/gateway/test_message_timestamps.py tests/cli/test_cli_interrupt_ack_race.py— 955 passed.npm --workspace apps/desktop test— 2874 passed, 3 skipped.typecheckclean,lint0 errors.[screenshot]is dropped only when a ref was lifted from the same message.Closes #70772
Closes #68092
Co-authored-by: alelpoan alelpoan@proton.me
Co-authored-by: 墨綠BG s5460703@gmail.com