Skip to content

[issues/685] Fix link-detection boundary handling for punctuation, empty lines, and file-path warnings - #694

Merged
couimet merged 4 commits into
mainfrom
issues/685
Jul 28, 2026
Merged

[issues/685] Fix link-detection boundary handling for punctuation, empty lines, and file-path warnings#694
couimet merged 4 commits into
mainfrom
issues/685

Conversation

@couimet

@couimet couimet commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes three related bugs where link-detection boundaries were too permissive: parenthesized RangeLinks included the ( in the matched path and failed to navigate, empty-line selections were incorrectly rejected as zero-width, and the file-not-found warning for bare file paths was confusing because RangeLink proactively created the link but then blamed itself when the file didn't exist.

Changes

  • RangeLinks inside surrounding punctuation now navigate correctly — (, [, {, < are trimmed from the matched range before parsing, so (path#Lx-Ly) opens the file instead of failing with "Cannot find file: (path..."
  • Trailing sentence terminators (., ,, :, ;) after a RangeLink no longer interfere with navigation
  • Empty line selections now produce valid #L12 links instead of erroring with "Failed to generate link"
  • Bare file paths that don't exist on disk now show "File does not exist at: {path}" instead of "Cannot find file: {path}", distinguishing benign detection from failed navigation
  • Documentation: CHANGELOG updated

Key Discoveries

  • VSCode's built-in terminal link detection doesn't handle bare absolute paths or #L RangeLink format — RangeLink's providers are the only ones creating these links, so the detection behavior is correct and the fixes focus on making navigation succeed when the file exists
  • The buildFilePathPattern regex character class already excludes () and other punctuation from matches; only the RangeLink pattern (buildLinkPattern) had the issue because its PATH_CHAR is a negation class
  • Trimming at detection level (post-regex, pre-parseLink) was chosen over excluding characters from PATH_CHAR because trimming preserves link detection inside parenthesized prose

Test Plan

  • All existing tests pass (2080 unit + 165 integration)
  • New tests added for: empty-line FullLine validation (2), punctuation trimming (4 unit + 2 integration), warning message (updated 2 existing assertions)
  • Manual testing: verify Cmd+click on a parenthesized RangeLink in a note navigates correctly

Related

Generated by /finish-issue v2026.07.19@1a9d2a6 • my-claude-skills

Summary by CodeRabbit

  • Bug Fixes
    • Improved detection so links wrapped in (...), [...], {...}, or <...> are parsed without including wrapper characters in the target path.
    • Link generation from an empty selected line now succeeds.
    • Missing-path warnings now say: “File does not exist at: …”.
    • Full-line zero-width selections on empty lines are now accepted; other zero-width cases remain rejected.
  • Tests
    • Expanded detection/selection validation cases and updated integration/QA navigation checks and expected behaviors.
  • Chores
    • Ignored coverage2/ in addition to coverage/.

…pty lines, and file-path warnings

## Summary

Fixes three related bugs where link-detection boundaries were too permissive: parenthesized RangeLinks included the `(` in the matched path and failed to navigate, empty-line selections were incorrectly rejected as zero-width, and the file-not-found warning for bare file paths was confusing because RangeLink proactively created the link but then blamed itself when the file didn't exist.

## Changes

- RangeLinks inside surrounding punctuation now navigate correctly — `(`, `[`, `{`, `<` are trimmed from the matched range before parsing, so `(path#Lx-Ly)` opens the file instead of failing with "Cannot find file: (path..."
- Trailing sentence terminators (`.`, `,`, `:`, `;`) after a RangeLink no longer interfere with navigation
- Empty line selections now produce valid `#L12` links instead of erroring with "Failed to generate link"
- Bare file paths that don't exist on disk now show "File does not exist at: {path}" instead of "Cannot find file: {path}", distinguishing benign detection from failed navigation
- Documentation: CHANGELOG updated

## Key Discoveries

- VSCode's built-in terminal link detection doesn't handle bare absolute paths or `#L` RangeLink format — RangeLink's providers are the only ones creating these links, so the detection behavior is correct and the fixes focus on making navigation succeed when the file exists
- The `buildFilePathPattern` regex character class already excludes `()` and other punctuation from matches; only the RangeLink pattern (`buildLinkPattern`) had the issue because its `PATH_CHAR` is a negation class
- Trimming at detection level (post-regex, pre-parseLink) was chosen over excluding characters from `PATH_CHAR` because trimming preserves link detection inside parenthesized prose

## Test Plan

- [x] All existing tests pass (2080 unit + 165 integration)
- [x] New tests added for: empty-line FullLine validation (2), punctuation trimming (4 unit + 2 integration), warning message (updated 2 existing assertions)
- [ ] Manual testing: verify Cmd+click on a parenthesized RangeLink in a note navigates correctly

## Related

- Closes #685
- Closes #661
- Closes #666
- Closes #683

Generated by /finish-issue v2026.07.19@1a9d2a6 • [my-claude-skills](https://github.com/couimet/my-claude-skills)
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 911473d1-302d-4819-a30f-330b7afdbecd

📥 Commits

Reviewing files that changed from the base of the PR and between 5ac0aca and cd9eae8.

📒 Files selected for processing (1)
  • packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/rangelink-vscode-extension/src/integration-tests/suite/linkGeneration.test.ts

Walkthrough

RangeLink now trims leading punctuation from detected links, permits full-line zero-width selections, and uses clearer missing-file warnings. Unit, integration, QA, changelog, and ignore-rule updates cover the changed behavior.

Changes

RangeLink behavior fixes

Layer / File(s) Summary
Trim punctuation from detected links
packages/rangelink-core-ts/src/detection/detectUnquotedLinks.ts, packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts, packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts, packages/rangelink-vscode-extension/qa/qa-test-cases.yaml, .gitignore
Leading wrapper punctuation is removed from detected link ranges, with unit, assisted navigation, QA, and coverage-ignore updates for the related behavior.
Allow full-line zero-width selections
packages/rangelink-core-ts/src/selection/validateInputSelection.ts, packages/rangelink-core-ts/src/__tests__/selection/validateInputSelection.test.ts
Full-line zero-width selections are accepted, while partial-line cursor selections still raise SELECTION_ZERO_WIDTH.
Rename missing-file warning messaging
packages/rangelink-vscode-extension/src/types/MessageCode.ts, packages/rangelink-vscode-extension/src/i18n/messages.en.ts, packages/rangelink-vscode-extension/src/navigation/FilePathNavigationHandler.ts, packages/rangelink-vscode-extension/src/__integration-tests__/suite/filePathNavigation.test.ts, packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathNavigationHandler.test.ts, packages/rangelink-vscode-extension/qa/qa-test-cases.yaml, packages/rangelink-vscode-extension/CHANGELOG.md
Missing paths use WARN_FILE_PATH_DOES_NOT_EXIST and display File does not exist at: {path} across implementation, tests, QA expectations, and changelog entries.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Poem

A bunny found a link in flight,
Trimmed its brackets clean and right.
An empty line now joins the fun,
Missing files speak plainly, one by one.
Hop, hop—RangeLinks shine!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The .gitignore addition for coverage2/ is unrelated to the linked issue fixes and appears to be separate cleanup. Remove the .gitignore change or split it into a separate cleanup PR unless it is required by the test coverage workflow.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main fixes around link boundaries, empty-line selection, and file-path warnings.
Linked Issues check ✅ Passed The code and tests address the linked bugs: punctuation trimming, empty-line selection, and the nonexistent-file warning are all covered.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issues/685

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

⚠️ QA Coverage Gap Detected

New link detection behavior introduced for surrounding punctuation and empty-line selections, which requires additional test coverage.

Suggested test cases:

  • Link Detection: Detect links with surrounding punctuation stripped (automatable)
  • Link Detection: Detect links with prefix-only characters (automatable)
  • Link Detection: Detect links with suffix-only characters (automatable)
  • Input Selection Validation: Allow empty-line FullLine zero-width selection (manual — Requires interaction with the editor to validate selection behavior.)

Generated by QA Gap Check (GPT-4o-mini via GitHub Models)

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

Coverage Report

Title Lines Statements Branches Functions
rangelink-vscode-extension Coverage: 98%
98.7% (4051/4104) 96.64% (1009/1044) 96.63% (660/683)
rangelink-core-ts Coverage: 100%
100% (501/501) 100% (175/175) 100% (48/48)

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown

TODO/FIXME Analysis

  • Base branch (a1d676ac4d6cdf8dd300bbd4f801fb9fb9e2f075): 34
  • Current branch: 34
  • Change: ➖ No change

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@couimet

couimet commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

↩ Triggered by: #694 (comment)


🤖 rabbit-maximizer v0.1.0 — run=671a5230-5b38-4b29-a686-8e3b6aec8d49

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@couimet: Starting a fresh full review of #694.

✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts (2)

63-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract semantic numeric expectations into constants.

The new tests hard-code link counts, expected link length, and line values. Use SCREAMING_SNAKE_CASE constants instead of magic numbers.

As per coding guidelines, numeric literals with semantic meaning must use named SCREAMING_SNAKE_CASE constants.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts`
around lines 63 - 103, Update the “surrounding punctuation trimming” tests to
define SCREAMING_SNAKE_CASE constants for the semantic expected link count, link
length, and start/end line values, then reuse those constants in the relevant
assertions instead of hard-coded numeric literals. Keep the existing test
behavior and index assertions unchanged where they represent positional offsets.

Source: Coding guidelines


63-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use named constants for semantic numeric test values.

The new tests encode counts, link lengths, and selection coordinates as raw numbers. Extract those values into SCREAMING_SNAKE_CASE constants.

  • packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts#L63-L103: name expected link counts, no-link counts, lengths, and coordinate values.
  • packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts#L127-L127: name the expected link count.
  • packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts#L142-L142: name the expected link count.
  • packages/rangelink-core-ts/src/__tests__/selection/validateInputSelection.test.ts#L323-L366: name the line and character coordinates.

As per coding guidelines, numeric literals with semantic meaning must use named SCREAMING_SNAKE_CASE constants.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts`
around lines 63 - 103, Replace semantic numeric literals with descriptive
SCREAMING_SNAKE_CASE constants. In
packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts lines
63-103, define and reuse constants for expected link counts, no-link counts,
link lengths, and start coordinates; in
packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts
lines 127-127 and 142-142, name the expected link counts; and in
packages/rangelink-core-ts/src/__tests__/selection/validateInputSelection.test.ts
lines 323-366, name the line and character coordinates.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts`:
- Around line 133-135: Update the assertions in the link-generation integration
test to compare each links[0].parsed.path value exactly against src/foo.ts
instead of using includes('foo.ts'). Apply the same equality assertion to both
referenced cases while preserving their existing failure messages and test
setup.

In
`@packages/rangelink-vscode-extension/src/navigation/FilePathNavigationHandler.ts`:
- Around line 68-73: Add or verify QA YAML scenarios for the file path
navigation changes, including a nonexistent bare path that expects “File does
not exist at: <path>” and no document opening. Also cover the updated
wrapped-link and empty-line navigation flows, using the existing QA case
conventions and symbols.

---

Nitpick comments:
In `@packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts`:
- Around line 63-103: Update the “surrounding punctuation trimming” tests to
define SCREAMING_SNAKE_CASE constants for the semantic expected link count, link
length, and start/end line values, then reuse those constants in the relevant
assertions instead of hard-coded numeric literals. Keep the existing test
behavior and index assertions unchanged where they represent positional offsets.
- Around line 63-103: Replace semantic numeric literals with descriptive
SCREAMING_SNAKE_CASE constants. In
packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts lines
63-103, define and reuse constants for expected link counts, no-link counts,
link lengths, and start coordinates; in
packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts
lines 127-127 and 142-142, name the expected link counts; and in
packages/rangelink-core-ts/src/__tests__/selection/validateInputSelection.test.ts
lines 323-366, name the line and character coordinates.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 198a50a8-6cb2-4227-ad18-094cba131ac6

📥 Commits

Reviewing files that changed from the base of the PR and between a1d676a and 0bacb43.

📒 Files selected for processing (12)
  • .gitignore
  • packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts
  • packages/rangelink-core-ts/src/__tests__/selection/validateInputSelection.test.ts
  • packages/rangelink-core-ts/src/detection/detectUnquotedLinks.ts
  • packages/rangelink-core-ts/src/selection/validateInputSelection.ts
  • packages/rangelink-vscode-extension/CHANGELOG.md
  • packages/rangelink-vscode-extension/src/__integration-tests__/suite/filePathNavigation.test.ts
  • packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts
  • packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathNavigationHandler.test.ts
  • packages/rangelink-vscode-extension/src/i18n/messages.en.ts
  • packages/rangelink-vscode-extension/src/navigation/FilePathNavigationHandler.ts
  • packages/rangelink-vscode-extension/src/types/MessageCode.ts

Comment on lines +133 to +135
assert.ok(
links[0].parsed.path.includes('foo.ts'),
`Expected path to include foo.ts: ${links[0].parsed.path}`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the parsed path exactly.

includes('foo.ts') would still pass if a wrapper or unexpected prefix remained. The behavior under test is the exact path src/foo.ts; assert equality in both cases.

Suggested assertion
-      links[0].parsed.path.includes('foo.ts'),
-      `Expected path to include foo.ts: ${links[0].parsed.path}`,
+      links[0].parsed.path,
+      'src/foo.ts',

Also applies to: 148-150

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts`
around lines 133 - 135, Update the assertions in the link-generation integration
test to compare each links[0].parsed.path value exactly against src/foo.ts
instead of using includes('foo.ts'). Apply the same equality assertion to both
referenced cases while preserving their existing failure messages and test
setup.

Comment on lines 68 to 73
if (!resolved) {
this.logger.warn({ ...logCtx, expandedPath }, 'Cannot resolve file path');
await this.ideAdapter.showWarningMessage(
formatMessage(MessageCode.WARN_FILE_PATH_NOT_FOUND, { path: rawPath }),
formatMessage(MessageCode.WARN_FILE_PATH_DOES_NOT_EXIST, { path: rawPath }),
);
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add QA YAML coverage for the user-visible navigation change.

This changes the warning shown for a nonexistent bare path. Add or verify a packages/rangelink-vscode-extension/qa/qa-test-cases-*.yaml case covering the warning text and confirming that no editor is opened; also verify QA coverage for the wrapped-link and empty-line flows changed in this PR.

- id: file-path-navigation-missing-bare-path
  feature: File path navigation
  scenario: Navigate to a nonexistent bare path
  preconditions: The path does not exist
  steps: Attempt navigation to the path
  expected_result: A warning says "File does not exist at: <path>" and navigation does not open a document
  automated: false

As per path instructions, user-visible VS Code behavior changes require corresponding QA YAML scenarios.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/rangelink-vscode-extension/src/navigation/FilePathNavigationHandler.ts`
around lines 68 - 73, Add or verify QA YAML scenarios for the file path
navigation changes, including a nonexistent bare path that expects “File does
not exist at: <path>” and no document opening. Also cover the updated
wrapped-link and empty-line navigation flows, using the existing QA case
conventions and symbols.

Source: Path instructions

@couimet

couimet commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

↩ Triggered by scheduler


🤖 rabbit-maximizer v0.1.0 — run=90e0a867-34f5-4378-bea1-9bd123eae543

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@couimet: Starting a fresh full review of #694.

✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 52 minutes.

… add single-sided wrapping coverage

The in-memory findLinksInText() tests only verified regex matching, not the full terminal link provider → punctuation trimming → parse → navigation pipeline. Replaced them with assisted terminal-based tests that create real files, echo RangeLinks to real terminals, and have a human verify Cmd+click navigation.

Added 21 unit tests for single-sided wrapping characters (prefix-only and suffix-only) to ensure link detection works when only one side of a wrapping pair is present. Previously only paired wrapping was tested.

Updated QA YAML entries and stale expected_result text for the new warning message format.

Benefits:
- Terminal wrapping tests now exercise the full VS Code stack (terminal link provider, detection, trimming, navigation, toast)
- Single-sided wrapping is covered by unit tests for all 7 wrapping characters in both prefix and suffix positions
- QA YAML accurately reflects automated: assisted for human-in-the-loop tests

Generated by /commit-msg v2026.07.22@9fcae14 • https://github.com/couimet/my-claude-skills
@github-actions

This comment has been minimized.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts (1)

40-293: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a data-driven helper to remove the 7x duplicated wrapper-navigation test blocks.

The baseline + wrapped-link-navigation-001..006 tests are structurally identical (create target file, echo wrapped link, waitForHumanVerdict, assert pass), differing only by wrapper chars, tcId, and label text. This mirrors the it.each parameterization already used for prefix/suffix punctuation cases in findLinksInText.test.ts.

♻️ Sketch of a parameterized helper
const WRAPPER_CASES = [
  { id: 'baseline', label: 'plain', open: '', close: '', suffix: '' },
  { id: '001', label: 'backtick-wrapped', open: '`', close: '`', suffix: '' },
  { id: '002', label: 'single-quote-wrapped', open: "'", close: "'", suffix: '' },
  { id: '003', label: 'double-quote-wrapped', open: '"', close: '"', suffix: '' },
  { id: '004', label: 'angle-bracket-wrapped', open: '<', close: '>', suffix: '' },
  { id: '005', label: 'paren-wrapped', open: '(', close: ')', suffix: '' },
  { id: '006', label: 'paren-then-colon-wrapped', open: '(', close: ')', suffix: ':' },
];

for (const { id, label, open, close, suffix } of WRAPPER_CASES) {
  test(`[assisted] wrapped-link-navigation-${id}: ${label} RangeLink in terminal is clickable and navigates correctly`, async () => {
    // shared body using open/close/suffix
  });
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts`
around lines 40 - 293, Replace the seven duplicated wrapper-navigation test
blocks with a data-driven WRAPPER_CASES table and a loop that defines each test.
Preserve each case’s id, label, opening and closing wrapper characters, suffix,
target-file setup, terminal output, human-verdict steps, assertions, and
logging; keep the Markdown and URL-exclusion tests unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts`:
- Around line 40-293: Replace the seven duplicated wrapper-navigation test
blocks with a data-driven WRAPPER_CASES table and a loop that defines each test.
Preserve each case’s id, label, opening and closing wrapper characters, suffix,
target-file setup, terminal output, human-verdict steps, assertions, and
logging; keep the Markdown and URL-exclusion tests unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a0972d57-f59b-40ff-b78b-2586b7fb3b14

📥 Commits

Reviewing files that changed from the base of the PR and between 0bacb43 and 5ac0aca.

📒 Files selected for processing (3)
  • packages/rangelink-core-ts/src/__tests__/detection/findLinksInText.test.ts
  • packages/rangelink-vscode-extension/qa/qa-test-cases.yaml
  • packages/rangelink-vscode-extension/src/__integration-tests__/suite/linkGeneration.test.ts

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

…n tests

Replace 7 structurally identical wrapped-link-navigation test blocks in linkGeneration.test.ts with a WRAPPER_CASES data table and a single shared test body. Reduces ~170 lines of duplication while preserving each distinct QA TC ID for traceability.

Benefits:
- Single point of change for future wrapper adjustments
- Follows the it.each pattern already used in findLinksInText.test.ts
- Each test case retains its TC ID for QA traceability

Ref: #694 (review)

Generated by /commit-msg v2026.07.22@9fcae14 • https://github.com/couimet/my-claude-skills
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown

✅ CI / Integration Tests (with overrides) — run summary

Duration: 1m 0s

QA TC IDs: 2 exercised across 0 features

Report: View run & artifacts

Reproduce locally: ./scripts/run-integration-tests.sh --label needs-override --exclude-label cursor --exclude-assisted

Feature breakdown
Feature TCs IDs

@github-actions

Copy link
Copy Markdown

✅ CI / Integration Tests (automated) — run summary

Duration: 10m 54s

QA TC IDs: 158 exercised across 0 features

Report: View run & artifacts

Reproduce locally: ./scripts/run-integration-tests.sh --exclude-label requires-extensions --exclude-label cursor --automated

Feature breakdown
Feature TCs IDs

@github-actions

Copy link
Copy Markdown

✅ CI / Integration Tests (with extensions) — run summary

Duration: 12m 30s

Unit tests: Ran in separate Test & Validate job

QA TC IDs: 190 exercised across 0 features

Report: View run & artifacts

Reproduce locally: ./scripts/run-integration-tests.sh --exclude-label cursor --exclude-label needs-override --exclude-assisted

Feature breakdown
Feature TCs IDs

@couimet
couimet merged commit 1e417e5 into main Jul 28, 2026
7 checks passed
@couimet
couimet deleted the issues/685 branch July 28, 2026 17:06
couimet added a commit that referenced this pull request Jul 28, 2026
…result-testing` (#697)

* [issues/685] Fix link-detection boundary handling for punctuation, empty lines, and file-path warnings

## Summary

Fixes three related bugs where link-detection boundaries were too permissive: parenthesized RangeLinks included the `(` in the matched path and failed to navigate, empty-line selections were incorrectly rejected as zero-width, and the file-not-found warning for bare file paths was confusing because RangeLink proactively created the link but then blamed itself when the file didn't exist.

## Changes

- RangeLinks inside surrounding punctuation now navigate correctly — `(`, `[`, `{`, `<` are trimmed from the matched range before parsing, so `(path#Lx-Ly)` opens the file instead of failing with "Cannot find file: (path..."
- Trailing sentence terminators (`.`, `,`, `:`, `;`) after a RangeLink no longer interfere with navigation
- Empty line selections now produce valid `#L12` links instead of erroring with "Failed to generate link"
- Bare file paths that don't exist on disk now show "File does not exist at: {path}" instead of "Cannot find file: {path}", distinguishing benign detection from failed navigation
- Documentation: CHANGELOG updated

## Key Discoveries

- VSCode's built-in terminal link detection doesn't handle bare absolute paths or `#L` RangeLink format — RangeLink's providers are the only ones creating these links, so the detection behavior is correct and the fixes focus on making navigation succeed when the file exists
- The `buildFilePathPattern` regex character class already excludes `()` and other punctuation from matches; only the RangeLink pattern (`buildLinkPattern`) had the issue because its `PATH_CHAR` is a negation class
- Trimming at detection level (post-regex, pre-parseLink) was chosen over excluding characters from `PATH_CHAR` because trimming preserves link detection inside parenthesized prose

## Test Plan

- [x] All existing tests pass (2080 unit + 165 integration)
- [x] New tests added for: empty-line FullLine validation (2), punctuation trimming (4 unit + 2 integration), warning message (updated 2 existing assertions)
- [ ] Manual testing: verify Cmd+click on a parenthesized RangeLink in a note navigates correctly

## Related

- Closes #685
- Closes #661
- Closes #666
- Closes #683

Generated by /finish-issue v2026.07.19@1a9d2a6 • [my-claude-skills](https://github.com/couimet/my-claude-skills)

* [test] Convert wrapped-link tests to end-to-end integration tests and add single-sided wrapping coverage

The in-memory findLinksInText() tests only verified regex matching, not the full terminal link provider → punctuation trimming → parse → navigation pipeline. Replaced them with assisted terminal-based tests that create real files, echo RangeLinks to real terminals, and have a human verify Cmd+click navigation.

Added 21 unit tests for single-sided wrapping characters (prefix-only and suffix-only) to ensure link detection works when only one side of a wrapping pair is present. Previously only paired wrapping was tested.

Updated QA YAML entries and stale expected_result text for the new warning message format.

Benefits:
- Terminal wrapping tests now exercise the full VS Code stack (terminal link provider, detection, trimming, navigation, toast)
- Single-sided wrapping is covered by unit tests for all 7 wrapping characters in both prefix and suffix positions
- QA YAML accurately reflects automated: assisted for human-in-the-loop tests

Generated by /commit-msg v2026.07.22@9fcae14 • https://github.com/couimet/my-claude-skills

* [PR feedback] Use data-driven table for wrapper-navigation integration tests

Replace 7 structurally identical wrapped-link-navigation test blocks in linkGeneration.test.ts with a WRAPPER_CASES data table and a single shared test body. Reduces ~170 lines of duplication while preserving each distinct QA TC ID for traceability.

Benefits:
- Single point of change for future wrapper adjustments
- Follows the it.each pattern already used in findLinksInText.test.ts
- Each test case retains its TC ID for QA traceability

Ref: #694 (review)

Generated by /commit-msg v2026.07.22@9fcae14 • https://github.com/couimet/my-claude-skills

* Ran `pnpm fix`

* [issues/687] Adopt `@couimet/detailed-result` and `@couimet/detailed-result-testing`

## Summary

Replaces the bespoke `Result` class and local Jest matchers with the shared `@couimet` packages they incubated. The local `Result<T, E>` class is replaced by `CoreResult<T>` extending `DetailedResult<T, RangeLinkError>`. `ExtensionResult<T>` and `FocusResult` become proper classes with `ok()`/`err()` factory methods. All five local matchers are replaced by `@couimet/detailed-result-testing@0.2.1`. Net result: hundreds of lines deleted, 144 test suites passing.

## Changes

- `CoreResult<T>` now extends `DetailedResult<T, RangeLinkError>` with backward-compatible `ok()`/`err()` factories; generic `Result<T, E>` class and its test file deleted
- `ExtensionResult<T>` converted from type alias to class extending `DetailedResult<T, ExtensionError>`; all production modules use `ExtensionResult.ok()`/`.err()` instead of raw `DetailedResult.success()`/`.failure()`
- `FocusResult` converted from type alias to class extending `DetailedResult<FocusedDestination, FocusError>`; capability modules use `FocusResult.ok()`/`.err()` while preserving lightweight `FocusError` signaling with typed `FocusErrorReason`
- All local Jest matchers replaced by `@couimet/detailed-result-testing@0.2.1` (`toBeSuccess`, `toBeSuccessWith`, `toBeFailure`, `toBeFailureWith`, `toHaveDetailedError`); 3 matcher files deleted, setup reduced to 2 import lines
- 200+ test assertions migrated: single-assertion callbacks to value form (`toBeSuccess({...})`), no-op callbacks to manual unwrap, typed callback parameters stripped, multi-field assertions converted to `objectContaining` where applicable
- Upstream: 3 issues and 1 PR contributed to `couimet/ts-npm-packages` fixing `.d.ts` module emission and optional peer dependency inconsistencies

## Key Discoveries

- `@couimet/detailed-result-testing`'s `setup-before-jest-30.d.ts` was emitted as a script, silently dropping `declare global` type augmentations in ts-jest. Fixed upstream by adding a type-level import to force `.d.ts` module emission.
- `@couimet/detailed-error-testing` was marked optional in `peerDependenciesMeta` but unconditionally required at the top level of every entry point. Fixed upstream by removing the optional marker.

## Test Plan

- [x] All 2,622 tests pass (26 core-ts + 118 vscode-extension suites)
- [x] No new tests needed — behavior unchanged, matcher coverage maintained
- [x] Build and lint pass

## Related

- Closes #687
- Closes #692
- Upstream fixes: couimet/ts-npm-packages#123

Generated by /finish-issue v2026.07.22@9fcae14 • [my-claude-skills](https://github.com/couimet/my-claude-skills)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant