Skip to content

fix review findings from lui extension replacement - #32

Draft
RCmerci wants to merge 1 commit into
devin/1790580543-composer-assetsfrom
devin/1790677501-review-fixes
Draft

RCmerci wants to merge 1 commit into
devin/1790580543-composer-assetsfrom
devin/1790677501-review-fixes

Conversation

@RCmerci

@RCmerci RCmerci commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Address the 5 Devin Review findings on #31 (e8751e7), applied on top of the merged content on devin/1790580543-composer-assets.

  • decode_picked: String.sub "" 1 0 on extensionless filenames raised and silently discarded the pick → derive file_type through the existing file_extension helper ("bin" fallback) moved above the decoder.
  • stage_copy: really_input_string read the entire pick into one string → stream copy in 64 KiB chunks; on failure the partial temp file is removed.
  • staged_completion: echoed the armed request token whenever pending <> [], so a newly armed pick request was completed before its pick could land → track spent_requests (armed tokens that produced an accepted pick) and only echo completion for those.
  • clear_graph_surface: media_preview survived graph switches, leaving the prior graph's QuickLook preview mounted → reset it with the rest of the graph surface.
  • journal_media_view.is_image_type: tif/avif were dropped versus the retired extension's isImage (JournalMedia.swift) → restored; file_image's ImageIO thumbnail path decodes both.

dune build / dune runtest green except the pre-existing V.progress boundary failure (identical on main).

Link to Devin session: https://app.devin.ai/sessions/55c3d31f1c6d4ee4beda2f832725d7b7
Open in Devin Desktop: https://app.devin.ai/desktop/session/55c3d31f1c6d4ee4beda2f832725d7b7?variant=devin
Requested by: @RCmerci

- decode_picked: tolerate extensionless picks via file_extension fallback\n- stage_copy: stream the temp copy in 64KiB chunks instead of reading the whole file into memory\n- staged_completion: only echo completion for request tokens that already produced a staged pick, so a newly armed picker request isn't prematurely completed\n- clear_graph_surface: drop media_preview so a previous graph's preview can't persist across a graph switch\n- journal_media_view: restore tif/avif to the inline image list (parity with the retired extension)
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

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