Skip to content

Code tab: resolve bare-filename file refs when basename is unique#900

Merged
srid merged 2 commits into
masterfrom
fix-898-basename-fileref
May 14, 2026
Merged

Code tab: resolve bare-filename file refs when basename is unique#900
srid merged 2 commits into
masterfrom
fix-898-basename-fileref

Conversation

@srid
Copy link
Copy Markdown
Member

@srid srid commented May 14, 2026

The Code tab now resolves a bare filename like Foo.hs:42 to the unique repo file ending in that basename. Compiler output frequently emits source locations without the full path prefix; previously those clicks fell through to a "File reference not found" toast even when exactly one file in the repo could match. Closes #898.

Resolution still tries the path-based candidates first (repo-relative, cwd-relative, absolute-stripped) — the basename scan only fires when all of them miss, and only commits to a result when there's exactly one match. Ambiguous basenames keep returning null; opening the wrong file is worse than the toast.

repo:                       resolveLineRefPath("Foo.hs"):
  src/lib/Foo.hs              1. cwd-rel  → miss
  src/lib/Bar.hs              2. repo-rel → miss
  ...                         3. basename → src/lib/Foo.hs  ✓ (unique)

The fallback covers terminal-link clicks and right-click Open path:N alike — both producers route through the single openInCodeTab resolver added in #891.

Coverage

  • UnitlineRef.test.ts: unique basename, ambiguous basename → null, slash-with-bad-prefix falls back, exact path candidate still preferred over basename
  • E2Efile-ref-link.feature: terminal emits notes.txt:2 while the file lives at src/lib/notes.txt; click opens the file in browse mode at line 2

Try it locally

```sh
nix run github:juspay/kolu/fix-898-basename-fileref
```

Generated by `/do` on Claude Code (model `claude-opus-4-7`).

srid added 2 commits May 14, 2026 16:23
#898)

The Code-tab open-file pipeline (terminal link clicks, right-click "Open
path:N") only matched repo-relative or cwd-relative path candidates. A
compiler error printing just `Foo.hs:42` without its `src/lib/` prefix
silently fell through to a "File reference not found" toast even when
exactly one repo file ended in that basename.

`resolveLineRefPath` now falls back to a basename match when the
path-based candidates miss. Ambiguous basenames (multiple repo files
share the name) keep returning null — opening the wrong file is worse
than the toast.
The basename-fallback rationale lives in the JSDoc on resolveLineRefPath.
The same paragraph (compiler output / src/lib prefix / ambiguity) was also
restated as a section banner in lineRef.test.ts, three per-test inline
comments, and a four-line Gherkin preamble. Test names and assertions
already say what each test does; the feature scenario name carries the
intent. Trimmed all four sites — JSDoc remains canonical.
@srid
Copy link
Copy Markdown
Member Author

srid commented May 14, 2026

Hickey/Lowy Analysis

# Lens Finding Disposition
1 Hickey resolveByBasename extends concern 3 (resolution) cleanly No-op
2 Hickey New local basename is not a duplicate of cwdBasename No-op
3 Hickey Hand-rolled basename vs node:path — 3 lines, no branching No-op
4 Lowy Slash-containing paths fall back to basename (deliberate) No-op
5 Lowy basename re-implements a standard utility No-op
6 Lowy Three-concern co-location (parse / format / resolve) No-op
7 Lowy formatLineRef has near-zero volatility, single consumer No-op
8 Lowy No layering inversion — public signature unchanged No-op

Hickey rationale

The module continues to address parsing, formatting, and resolution. The new resolveByBasename extends concern 3 as a fallback strategy without introducing a new top-level concern; the "single resolution site" invariant noted in CodeTab.tsx:222 still holds. No fragmentation — three invariants (candidates-first, ambiguity returns null, empty-rawPath rejected) are all enforced structurally by control flow, not by convention. The new local basename() is not a duplicate of cwdBasename in packages/common/src/path.ts — they have different semantics on edge cases (empty input, ~ expansion), and reusing cwdBasename would actually break correctness because the empty-string guard in resolveByBasename depends on basename("") returning "". Hand-rolled basename flagged then dismissed: node:path is unused in the browser client, and a 3-line private helper has less surface than pulling in a node-path polyfill. Entanglement: resolveLineRefPath remains a linear pipeline (Set → generator iteration → fallback); resolveByBasename is one concern, pure, no I/O.

Lowy rationale

resolveLineRefPath carries sequence volatility (orders the resolution strategies); the pre-existing for loop already separated sequence from activity, so the new fallback call slots in cleanly. resolveByBasename carries activity volatility — it encapsulates the single-match uniqueness rule, and plausible future changes (case-insensitive matching, restricting to bare filenames, switching the tie-break rule) are all contained inside this one function. The two consumers — fileRefLinkProvider.ts (imports only parseLineRefs) and CodeTab.tsx (imports only resolveLineRefPath) — are already fully decoupled at the import level. Public resolveLineRefPath signature is unchanged under the added volatility — the correct indicator that the interface is stable. Held in reserve: if resolution strategies grow to three or four, an explicit strategy array would make the escalation ladder more visible — not yet worth the indirection at N=2.

@srid srid mentioned this pull request May 14, 2026
@srid
Copy link
Copy Markdown
Member Author

srid commented May 14, 2026

Evidence

Clicked a bare notes.txt:2 link in a terminal scrolled to a fresh repo /tmp/kolu-evidence-898 (one matching basename under src/lib/). The Code tab opens src/lib/notes.txt and highlights line 2 (beta), confirming the basename fallback resolves an unambiguous filename without a path prefix.

notes.txt:2 click resolved via basename fallback to src/lib/notes.txt with line 2 highlighted

@srid
Copy link
Copy Markdown
Member Author

srid commented May 14, 2026

/do results

Step Status Duration Verification
sync 1s git fetch ok; forge=github; noGit=false
research 3m 19s Issue #898: extend resolveLineRefPath in lineRef.ts:108 with basename fallback when no path-based candidate matches; return null when ambiguous
branch 7s On feature branch fix-898-basename-fileref
implement 2m 4s Modified resolveLineRefPath with basename fallback; added 4 unit tests + 1 e2e scenario; vitest 122/122 passed
check 2m 17s just check exited 0; only pre-existing unrelated warnings
docs 17s README.md line 38 already accurate — basename fallback is a resolution refinement, not a new shape
fmt 12s just fmt — no changes
commit 21s Commit 7f99a4b3 pushed
hickey+lowy 3m 34s Hickey: 0 findings. Lowy: 5 findings, all No-op
police 6m 7s Rules + fact-check clean; elegance trimmed duplicated WHY comments — commit 1aff3f03
test 5m 1s 44/44 e2e scenarios pass across file-ref-link + code-tab; new scenario passed first try
create-pr 1m 33s Draft PR #900; hickey/lowy ledger posted
ci 28m 35s 11/13 statuses success; e2e@aarch64-darwin success on retry; e2e@x86_64-linux flakes (different scenario each run, all pass individually — see #320); ci/unit verified pre-existing on origin/master
evidence 6m 41s Screenshot captured + uploaded to evidence-assets release
Total 60m 25s

Slowest step: ci (28m 35s — 47% of total)

Optimization suggestions

  • CI ate the budget — five full ci::e2e@x86_64-linux runs to chase a different agent-mock-state-polling flake each time (opencode/codex/claude-code state expectations + the Given a Code tab in "branch" mode setup). Hardening these via e2e-poll-async-state (page.waitForFunction with POLL_TIMEOUT) would collapse the ladder; flake patterns already filed to Flaky tests log #320.
  • Pre-existing ci/unit failure (subscribeGitInfo watcher churn > setCwd between two distinct git repos) is a master-branch regression that gates the sequential unit → surface-example-build chain — fixing it independently unblocks the chain and stops surface-example-build from being marked "not run" on every full just ci.
  • Re-runs from this point can use --from ci-only to skip ~12 minutes of research → implement → check → fmt → commit → hickey/lowy → police → test that the workflow already verified.
  • Evidence step's 6m 41s is mostly chrome-devtools MCP cold-start + dev-server boot — if multiple /do runs queue up the same day, leaving the dev server running across runs would reclaim ~3m per evidence capture.

Workflow completed at 2026-05-14 21:14 UTC.

@srid srid marked this pull request as ready for review May 14, 2026 21:18
@srid srid merged commit beedb38 into master May 14, 2026
14 of 15 checks passed
@srid srid deleted the fix-898-basename-fileref branch May 14, 2026 21:18
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.

Open file reference (in code tab) should search by filename too

1 participant