Skip to content

feat(terminal): stitch hard-wrapped file references at click time - #174

Merged
ghackett merged 1 commit into
mainfrom
feat/wrapped-file-references
Aug 5, 2026
Merged

feat(terminal): stitch hard-wrapped file references at click time#174
ghackett merged 1 commit into
mainfrom
feat/wrapped-file-references

Conversation

@ghackett

@ghackett ghackett commented Aug 3, 2026

Copy link
Copy Markdown
Member

Stacked on #173. Live testing there showed the one annoying gap: when the CLI hard-wraps a long path in its output, the screen genuinely contains two strings separated by a newline and indent, so the reference was inert however you clicked it.

How it works

Clicking a fragment that fails (or shouldn't win) direct resolution now tries stitching before giving up:

  • Geometry gate: fragments are only joined downward when the clicked candidate sits at the very end of its row, and upward when it sits at the start (after indent) — the two signatures of a wrapped token. A mid-row candidate never stitches.
  • Join rule: each neighbour row contributes its adjacent whitespace-delimited token, chaining further only while a whole row was a single token (a middle fragment). Bounded at 2 rows up / 3 down.
  • Same false-positive gate as everything else: every join is re-matched against FILE_PATTERN (shedding trailing punctuation, keeping a :line[:col] suffix that landed on the continuation row) and must exist on disk. A stitch that guesses wrong opens nothing.
  • Stitch runs before direct resolution — discovered live: the leading fragment of a wrapped path is often an existing directory prefix (…/.claude/worktrees), and without the reordering the click opens the file manager there instead of the file the reference actually names. The stitcher returns fast for unwrapped references, so ordinary clicks don't pay for it.

The stitcher is pure and GTK-free (resolve_wrapped_reference in linkpatterns.py, unit-tested); terminal.py only converts the click's y into a buffer row (pixel- and row-unit scrolling both handled, ±1 row slop for VTE's inner border) and feeds neighbour row texts.

Verification

  • 7 new unit tests: downward/upward/three-row stitches, suffix on the continuation row, the mid-row gate, unresolvable joins, and prose-at-row-start not being poisoned by the row above. 59 pass total.
  • Live headless run with a hard-wrapped absolute path and 40 lines of scrollback (vadjustment at 738px, pixel scrolling): clicking either fragment resolves the full path via the click handler's exact order; the head-fragment-is-a-directory hazard is asserted; a bogus mid-row candidate stays inert.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Dpjh2MJkFJNTjUbbZm1tSN

Base automatically changed from feat/clickable-file-references to main August 3, 2026 13:37
When the CLI hard-wraps a long path (a real newline plus continuation
indent), the screen holds two separate fragments and no regex can match
across them. Clicking either fragment now stitches: geometry-gated (the
fragment must touch its row's edge — the signature of a wrapped token),
joined with the adjacent token of neighbour rows (chaining across
whole-row middle fragments, 2 up / 3 down), re-matched against
FILE_PATTERN, and existence-checked like any other candidate.

Stitching runs before direct resolution: the leading fragment of a
wrapped path is often an existing directory prefix, and the stitched
whole is the truer reading. Verified live: both fragments of a
hard-wrapped absolute path open the full file (the head fragment alone
resolves to .claude/worktrees — the ordering hazard), a mid-row bogus
candidate stays inert, and the y-to-row math holds under pixel
scrolling with a scrolled-back buffer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dpjh2MJkFJNTjUbbZm1tSN
@ghackett

ghackett commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @ghackett's task in 4m 24s —— View job


Claude finished the review

  • Gather context (read diff, changed files)
  • Understand the request (code review)
  • Review collins/linkpatterns.py changes
  • Review collins/terminal.py changes
  • Review tests/test_linkpatterns.py changes
  • Post review feedback

Summary

Well-scoped change with a clear rationale, and the design is defended in depth: geometry gate → FILE_PATTERN re-match → existence check, so a wrong guess can't open the wrong thing, only fail to open anything. I traced the join logic against all 7 new tests by hand (chain building, the token != frag continuation-row cutoff, suffix preservation, mid-row rejection) and it holds up. No blocking issues found.

Note: I wasn't able to execute pytest in this sandbox (python invocations required approval that wasn't granted), so this review is from static reading + manual tracing, not a live test run. You may want to confirm the reported 59/59 pass count independently, or adjust --allowedTools if you'd like me to run the suite next time.

Minor observations (non-blocking)

  1. The "returns fast for unwrapped references" claim doesn't hold for the common case of a reference alone on its own row (collins/terminal.py:192-193, collins/linkpatterns.py:122-146). The geometry gate is row.rstrip().endswith(candidate) / row.lstrip().startswith(candidate) — when a reference is the only thing on its row (e.g. a CLI prints one path per line, no wrapping involved), both conditions are trivially true, since the whole trimmed row equals the candidate. That means every such click still probes up to 2 rows up / 3 rows down and tries several FILE_PATTERN + filesystem checks before falling through to direct resolution — it's not actually skipped, just eventually falls back correctly. Functionally safe (the existence check + fallback to resolve_file_reference after _resolve_wrapped_at returns None covers it), but the docstring/comment overstates how narrow the slow path is, and it's worth a test that pins down "a standalone one-line reference still resolves correctly when adjacent rows contain deceptive tokens" — right now the closest test (test_prose_at_row_start_is_not_poisoned_by_row_above) covers a failing fragment, not a successful standalone one with active neighbours.

  2. Geometry gate matches by string content, not by click position (collins/linkpatterns.py:124, :136). row.endswith(candidate) / startswith(candidate) can't distinguish "the clicked occurrence is at the row's edge" from "some occurrence of the same text is at the row's edge." If a row repeats the same path-shaped token twice (once mid-row, once trailing), a click on the mid-row occurrence would still pass the edge gate. Very low real-world likelihood, and the existence check still guards the actual file that opens, so I'd call this theoretical rather than a real bug.

  3. Eager neighbour-row fetching (collins/terminal.py:261-282). _resolve_wrapped_at builds rows_above/rows_below unconditionally for every r in (row, row-1, row+1) — up to ~18 calls to terminal.get_text_range_format per file-link click, even when the center row's own geometry check would reject before neighbours are ever needed. Likely negligible in practice (VTE buffer reads on a user-rate event), but a lazy fetch (only pull neighbours after the row's own edge check passes) would trim it if it ever shows up in profiling.

None of these block the PR — they're refinements, not correctness gaps. The stitching design's core safety property (a wrong stitch can only fail to open, never open the wrong thing on a new incorrect path) is sound given the existence-check gate.
· branch

@ghackett
ghackett marked this pull request as ready for review August 5, 2026 17:39
@ghackett

ghackett commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Addressed the review's three observations in the stacked follow-up #175 (which rewrites _resolve_wrapped_at, so fixing them there avoids churning this base branch):

  1. "Returns fast" overstatement + missing standalone test — the comment at the stitch call site now states the real cost honestly (mid-row references bail at the edge gates immediately; a reference alone on its row does probe neighbours before the direct fallback wins), and test_standalone_reference_ignores_deceptive_neighbours pins down exactly the requested case: a good one-line reference with active, deceptive neighbour rows still resolves via the direct fallback because every stitched join fails the existence check.

  2. Edge gate matches by content, not click position — agreed it's theoretical; left as-is. The existence check remains the backstop, and a repeated-token row can at worst re-resolve the same on-screen text, never open a path that isn't rendered.

  3. Eager neighbour-row fetching (~18 grid reads per click) — resolved by design in fix(terminal): continuation fragments of wrapped references are clickable #175: grid reads turned out to be broken under claude's repaint-style renderer anyway (the ring drifts a page away from the adjustment), so row texts now come from one get_text_format snapshot per click instead of per-row grid reads.

On the pass-count verification: CI runs the suite (the local sandbox has no pytest); the current count on the follow-up branch is 28 linkpatterns tests, all green.

@ghackett
ghackett merged commit a7b02d7 into main Aug 5, 2026
2 checks passed
@ghackett
ghackett deleted the feat/wrapped-file-references branch August 5, 2026 17:48
ghackett added a commit that referenced this pull request Aug 5, 2026
…able (#175)

Stacked on #174. A live testing pass there found that clicks on wrapped
file references did nothing — and the failing half was the
*continuation* fragment, the one holding the file name, i.e. exactly
where you'd naturally click. Verified against a live headless terminal:
with PR 174 as-is, only head-fragment clicks ever worked in Claude
Code's real output format.

## The two causes

- **Slashless continuations were never candidates.** `FILE_PATTERN`
deliberately requires a slash, and a wrap point usually falls inside the
basename, so the continuation row shows `o.py:7)` — no VTE match under
the pointer, and the click handler returned before the stitcher was
consulted. Now, when *nothing* under the pointer matches, the raw
whitespace-delimited token at the clicked cell stands in as the stitch
candidate (`token_at_column`, unit-tested). The stitcher's geometry
gates plus the existence check keep Ctrl+clicks on ordinary prose inert
— verified live for row-edge prose tokens and empty areas.

- **Leading junk killed the join.** The emitter wraps whatever surrounds
the path along with it, so fragments arrive glued to prefixes — `⏺
Read(/a/b/c` is Claude Code's own tool-call format, and the row-above
token it contributes is `Read(/a/b/fo`, not `/a/b/fo`. The anchored
`match` could only shed *trailing* punctuation, so every up-stitch
through that format failed even when the continuation did contain a
slash. Joins are now matched with a search over the joined string, with
span guards requiring the hit to overlap the clicked fragment itself —
one mechanism that sheds junk whether it sits in a contributed token or
in a token-derived candidate, and that still refuses to resolve text the
click never touched.

## Verification

- 5 new unit tests: up-stitch through a `Read(`-glued head, a slashless
token candidate keeping its `:7` suffix, a glued-prefix token candidate,
the span guard (a junk-leading candidate must not resolve the row
above's path), and `token_at_column` edge cases. 64 pass total.
- Live headless run (real VTE, 40 rows of scrollback, pixel scrolling,
Claude-style `⏺ Read(...)`/`⏺ Update(...)` wrapped output): all 8 click
scenarios pass — both fragments of a wrapped reference resolve with the
line suffix, a head token glued to `Update(` resolves, plain unwrapped
references still resolve, and prose tokens at row edges plus empty areas
stay inert. Before the fix the same probe failed 4 of 8.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01AXEJjBkzLLfVtVk6HhzFEn

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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