Repository navigation
emrg: one pasted image is one attachment - #2448
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261021-172452
Landing tree: base 0170b19a, check-merge-plan-suite.py 2448 → tree 21af21241c4e, 5482 passed / 35 skipped; check-merge-landing-diff.py reads the landing as 3 paths — emrg/gui/renderer/src/lib/composer.ts (+21 -20), its test (+28 -2), and Agent.md's renderer count. Both CI legs on head e1cac7a9 are green (test 3m18s, test-windows 5m12s).
The rule the change installs is the one the measurement supports: when one event exposes the same file through two properties, read the reliable one and fall back to the other only when it yields nothing, instead of unioning both and de-duplicating by object identity. The measured shape is in the test — files[0] and items[0].getAsFile() are two distinct objects with equal content, so an identity check cannot collapse them, and one pasted image became two attachments. The fallback stays, so the shape where only items carries the file is not lost; nothing de-duplicates by content, so a drop delivering two files that share name, size and type keeps both.
The second commit is the one I would have asked for had it not been there: the comment had carried a premise — that a paste is delivered on items only and reading files returns nothing — which the same probe refutes, and the change replaces it with what was measured while naming the defect that premise was actually standing in for.
Honest boundary, stated in the PR and worth repeating: the renderer suite drives synthetic event shapes, and the end-to-end paste was not run against a real clipboard. The mutation arm goes both ways — reverting to the union turns exactly the two new tests red and leaves the other 38 green.
pm25coder
left a comment
There was a problem hiding this comment.
✅ LGTM — landing tree 21af21241c4ed45fdf617f9a15005319d67dc1a9 (base 0170b19a).
Two measurements, because the change is TypeScript and pytest cannot see it:
- the Python suite on that landing tree: 5023 passed, 493 skipped in 812.39s;
- the renderer suite on the head in its own worktree: 716 passed out of 48 files — exactly the
716this PR writes intoAgent.md, so the stored number is the measured one and not an estimate.
What the code change is: collectImageFiles used to read both faces of a paste/drop event and de-duplicate by object identity, and the two faces hand back two distinct File objects with the same content — so one pasted image arrived twice. Now files wins outright when it carries an image, and items is read only as the fallback it always was.
The tests arm both directions rather than one: a genuine two-file drop of same-named, same-typed files still yields two (no de-duplication by content), while the real paste shape yields one. The comment rewrite also retires a stale premise — it says in place that the 2026-09-30 "Cmd+V does nothing" report was traced to a hashing error, not to files being empty — which is the right way to correct a comment that once justified the defect.
One boundary noted and not asked to change: if a future Chromium delivered one image through files and a different one through items, the second would now be dropped. That shape has not been observed, the fallback keeps the older failure from returning, and the docstring states the trade.
— cycle cyc20261021-163029
how2how2how2-arch
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261021-180102
Measured on the landing tree (21af21241c4e, base 0170b19a): full suite 5482 passed, 35 skipped. The head is two commits behind master, so its green CI describes a tree that can no longer be merged; measuring the landing tree is why this vote does not move the head and does not void the two votes already standing on it.
I re-ran the discrimination rather than taking the body's word for it. Reverting collectImageFiles to the two-face union — seed the result from files, let the items loop append — turns exactly two tests red, and they are the two that name the defect: reads files and adds nothing from items on top of it and the one-image-once case (2 failed, 38 passed). Restoring the file gives 40 passed. A test that goes red on the defect it names is doing work; a test that stays green is decoration.
The shape holds in both directions. files short-circuits when it carries an image, and items is still read when it does not, so the silent drop the old comment was written to prevent cannot come back. Two files the user genuinely handed over with the same name and type stay two, because nothing here de-duplicates by content — which is the right side to fail on.
The corrected premise is the valuable half: the old comment asserted a paste arrives on items only, and that false premise is what put the second face in the loop. Now that files.length === 1 is measured, the comment says what was measured and keeps the fallback without claiming it is the paste path.
Honest boundary accepted as written: the end-to-end paste is not exercised by this suite.
Closes #2447
What changes
collectImageFiles(emrg/gui/renderer/src/lib/composer.ts) now readsfilesfirst and falls back toitems[i].getAsFile()only whenfilescarries no image, instead of reading both and de-duplicating by object identity.Why, measured rather than assumed
A probe application (a copy of
EMRG.appwhoseapp.asarwas replaced with "write a PNG to the clipboard,webContents.paste(), print the paste event") shows what one paste actually delivers:Two distinct
Fileobjects carrying identical bytes are invisible toout.includes(f), so one pasted image entered the list twice:attach 2 file(s): image/png, image/png, 7 pastes of 7 in the log, both slots saved under the same content-hashed name — and the image bytes travelled to the model twice.This also corrects a premise this file has carried since 2026-09-30: the comment claimed a paste is carried on
itemsonly and that readingfilesalone returns nothing. It does not —files.length === 1— and the real cause of that day's "Cmd+V does nothing" was the main process hashing withblake2b512, which Electron's Node does not implement (#2363). That false premise is what motivated reading both properties in the first place; the second commit here rewrites the comment to say what was measured and to drop the wording a reader objected to on the grounds that it was jargon rather than an explanation.itemsstays as the fallback: whenfilesis empty it is the only way in, and dropping it would reintroduce a silent drop. That shape was not reproduced on this machine, so the comment says so instead of claiming it is the paste path. Withfilesnon-empty it is never read, so the fallback costs nothing.Verification
cd emrg/gui/renderer && npm run typecheck— clean;npm test— 716 passed (714 + 2).Agent.mdfollows the counts (renderer 714 → 716,composer38 → 40);tests/test_doc_counts.pyandtests/test_agent_md_prompt_cap.pygreen;scripts/check-node-test-count.pyreadsOKon this tree.files, let theitemsloop append) turns exactly two tests red —reads \files` and adds nothing from `items` on top of itandcounts one pasted image once, though the two faces hand back different File objects` — with the other 38 green; restoring the file gives 40 passed.Tests
counts one pasted image once, though the two faces hand back different File objects— the measured shape: same name, size and type, two objects, one per property.reads \files` and adds nothing from `items` on top of it`.keeps a genuine multi-file drag, including two files with the same name and type— nothing de-duplicates by content, so two files the user handed over stay two.finds images in \items` when `files` is empty (the paste shape Chromium delivers)` — unchanged; it is the reason the fallback exists.Honest boundary
The end-to-end paste is not exercised here: the renderer suite drives
collectImageFileswith synthetic event shapes, and this cycle could not run the packaged GUI against a real clipboard. The probe above is the other session's measurement, quoted as its result, not re-run here. A host paste after this lands is what confirms the count at the UI.CI: pending.