Repository navigation
fix(opencode): expand single-line file mentions to enclosing LSP symbol using 1-based lines - #47503
kaeldrin-gh wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
needs:compliance
…ol using 1-based lines
61350f8 to
7d500f9
Compare
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found (Note: PR #42888 titled "expand single-line selection to enclosing LSP symbol" appears in search results but is addressing a different scope—selection expansion vs. file mention expansion—and is a previous related PR, not a duplicate.) |
There was a problem hiding this comment.
🟡 Changes recommended
The new lineCount computation ignores Range.end.character, which can undercount lines for symbols whose end position is within (not at the start of) the final line.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes single-line file://...?...start=N&end=N file mentions so they expand to the intended LSP document symbol range by correctly handling 1-based URL line numbers vs 0-based LSP ranges, and by allowing matches on line 0.
Changes:
- Adjusted symbol-range matching in
SessionPromptto compare 1-based URL lines against 0-based LSPRange.start.line, and to avoid falsy checks that exclude line 0. - Converted LSP end-exclusive ranges into
Readtooloffset/limitsemantics for expanded reads. - Added regression tests that mock
documentSymboland assert the resultingReadtool call uses the expandedoffset/limit.
File summaries
| File | Description |
|---|---|
| packages/opencode/src/session/prompt.ts | Fixes 1-based vs 0-based line comparisons and expands single-line file mentions using LSP symbol ranges. |
| packages/opencode/test/session/prompt.test.ts | Adds regression tests (with mocked LSP symbols) covering expansion from line 1 and line 2. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // URL line numbers are 1-based; LSP ranges are 0-based and end-exclusive. | ||
| if (r?.start?.line != null && r.start.line + 1 === start) { | ||
| const lineCount = Math.max(1, (r.end?.line ?? r.start.line) - r.start.line) | ||
| start = r.start.line + 1 | ||
| end = start + lineCount - 1 | ||
| break | ||
| } |
There was a problem hiding this comment.
LSP ranges are end-exclusive by spec: the range [1,4) covers 0-based lines 1-3 (1-based 2-4), and a non-zero end.character on the exclusive end line does not add an extra line. \lineCount = end.line - start.line\ is therefore the correct line span; the character offset only matters if the end line were inclusive, which it isn't for document symbols.
github-actions
Bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Issue for this PR
Closes #42887
Type of change
What does this PR do?
Single-line file mentions (
@file.ts#42) were not expanded to the enclosing LSP symbol: the URL line numbers are 1-based while LSP ranges are 0-based, sor.start.line === startnever matched. A truthiness check (r?.start?.line &&) also dropped symbols starting on line 0.The fix compares
r.start.line + 1against the 1-based URL line, converts the matched range to 1-based inclusive lines, and derives the Read tool'soffset/limitfrom the end-exclusive LSP range. Symbols on line 1 now match too.How did you verify your code works?
documentSymboland assert the read call uses the expanded range (offset/limit):?start=2&end=2with a 0-based [1,4) symbol -> offset 2, limit 3;?start=1&end=1with a 0-based [0,3) symbol -> offset 1, limit 3.bun test test/session/prompt.test.tsfrompackages/opencode: 47 pass, 14 skip, 0 fail.bun typecheckfrompackages/opencode: clean.Screenshots / recordings
No UI change.
Checklist