fix: resolve image paths before the model ever sees them - #20
Open
carstenlucke wants to merge 1 commit into
Open
fix: resolve image paths before the model ever sees them#20carstenlucke wants to merge 1 commit into
carstenlucke wants to merge 1 commit into
Conversation
A `~/x.png` reference never reached the vision tool as a usable path,
and the paths the model was shown were the raw tokens the user typed
rather than the resolved ones. Both stem from the same gap: path
resolution happening too late, or not at all.
1. Tilde expansion was broken in three places:
- lib/image.ts stripped the tilde (`replace(/^~/, "")`), turning
`~/x.png` into the absolute `/x.png`, which then won the
resolve() against cwd.
- extensions/paste.ts resolved `~/x.png` against cwd, yielding
`<cwd>/~/x.png` (tilde as a literal directory name).
- updateComposePreview() had the same bug in a third variant
(isAbsolute("~/x") is false → `<cwd>/~/x.png`).
Effect: a home-relative path pasted into the editor produced no
[Image-#N] marker, no hint line, no compose preview and no
auto-delegation. It only appeared to work because the LLM expanded
the tilde itself before calling describe_image, so the extension
never saw a tilde on the tool path.
Adds a shared, pure `expandTilde()` in lib/image.ts so the tool
path and the paste path expand identically. `~user/…` is
deliberately not expanded (that needs a passwd lookup; mapping it
onto the current user's home would be wrong) — it falls through and
fails existsSync. resolveImageFile is exported so the resolution
rules are unit-testable without a pi runtime.
2. buildHintLine and buildDescriptionsBlock rendered the raw token
while LoadedImage.abs sat in the same object. A `~/x.png` or
`./x.png` in the hint is not actionable without $HOME or the cwd,
which defeats the stated purpose of the v0.4.0 hint (SPEC-4 §3.4):
list the paths "so the model can actually call describe_image".
Observed in a real GLM-5.2 session: the model shelled out to
`bash echo ~/vision-tilde-test.png` purely to resolve the tilde
before it could call the tool. Both now take resolved paths only —
buildHintLine as `paths: string[]`, since `index` was never read
and `token` only fed a fallback that no call site could reach.
Verified end to end against GLM 5.2 with a text-only primary: the
hint now lists the absolute path, and the model calls describe_image
directly — one tool call instead of two.
Tests: +11 (364 total, was 353), including integration tests that
drive the full input event with $HOME on a temp dir and cwd
deliberately elsewhere. All were verified to fail against the
pre-fix code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Problem
A
~/x.pngreference never reached the vision tool as a usable path, and the paths the model is shown were the raw tokens the user typed rather than the resolved ones. Both come from the same gap: path resolution happening too late, or not at all.1. Tilde expansion was broken in three places
~/x.pngresolved tolib/image.ts:151—replace(/^~/, "")/x.png(stripped tilde wins theresolve()against cwd)extensions/paste.ts:73<cwd>/~/x.png(tilde as a literal directory name)updateComposePreview()<cwd>/~/x.png(isAbsolute("~/x")is false)Neither path exists,
existsSyncfails, and the token is treated as unresolvable. So a home-relative path pasted into the editor produced no[Image-#N]marker, no hint line, no compose preview and no auto-delegation.This went unnoticed because the LLM expands the tilde itself before calling
describe_image— the extension never saw a tilde on the tool path. The paste path, which runs before the model, had no such luck.2. The hint listed tokens the model cannot act on
buildHintLineandbuildDescriptionsBlockrenderedtokenwhileLoadedImage.abssat in the same object. A~/x.pngor./x.pngin the hint is not actionable without$HOMEor the cwd, which defeats the stated purpose of the v0.4.0 hint (SPEC-4 §3.4): list the paths "so the model can actually call describe_image".Observed in a real GLM-5.2 session — the model shelled out purely to resolve the tilde before it could use the tool:
Fix
expandTilde()inlib/image.ts, used by both the tool path and the paste path so they expand identically.~user/…is deliberately not expanded — that needs a passwd lookup, and mapping it onto the current user's home would be wrong; it falls through and failsexistsSync.resolveImageFileis exported so the resolution rules are unit-testable without a pi runtime.buildHintLinenow takespaths: string[], andbuildDescriptionsBlocktakespathinstead oftoken. Both call sites collapse toloaded.map((l) => l.abs).indexwas never read bybuildHintLine, andtokenonly fed a fallback no call site could reach.Verification
End to end against GLM 5.2 with a text-only primary. Same input, before and after:
~/vision-tilde-test.png/Users/carsten/vision-tilde-test.pngbash echo …detourThe delegated description was correct in both runs (the test image carries a checksum string), so this changes only how the path reaches the model.
Tests
+11 (364 total, was 353). Every behavioural test was verified to fail against the pre-fix code, so they are real regression guards rather than passengers.
The integration tests (
T52bhint mode,T32bauto mode) drive the full input event with$HOMEon a temp dir and cwd deliberately elsewhere — that combination catches anl.token-instead-of-l.absslip inpaste.tsthat unit tests onmarker.tsalone did not.tsc --noEmitclean.Note
Independent of #21 — different cause, different scope, no shared commits. Either can merge first.