Skip to content

Include terminal words when source offsets are absent - #2172

Closed
xiehuanyi wants to merge 1 commit into
postcss:mainfrom
xiehuanyi:fix/source-end-word-r2-20261013
Closed

Include terminal words when source offsets are absent#2172
xiehuanyi wants to merge 1 commit into
postcss:mainfrom
xiehuanyi:fix/source-end-word-r2-20261013

Conversation

@xiehuanyi

@xiehuanyi xiehuanyi commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown

positionBy({word}) and rangeBy({word}) fail to locate a word that ends at the last character of a node when a custom syntax omits source offsets. Their source slices treat the inclusive line/column end as exclusive, dropping the final character. Diagnostics then point at the start of the declaration instead of the requested value.

Convert line/column end positions to an exclusive slice boundary when searching for words. Explicit source offsets already use exclusive ends and are preserved unchanged.

Validation:

  • Both regressions fail on unchanged main for a{color:red} without offsets: the word red incorrectly resolves to the declaration start.
  • Full unit suite: 722/722 passed, including existing explicit-offset and multi-root document tests.
  • pnpm test:lint, pnpm test:types, pnpm test:size (15.54 kB under16.5 kB), and git diff --check pass.

Discovery context: this PR was found and implemented by an AI coding agent (gpt-6.1-sol, xhigh) during source inspection. It was verified with the minimal reproduction and regression tests above. There is no reported production incident or external user report behind this PR.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected position and range lookup when the target word is at the end of the source text and explicit boundary offsets are unavailable. The final character is now included in the result.

Copilot AI balanced review requested due to automatic review settings October 13, 2026 09:01
Copilot AI reviewed Oct 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0c642039-fbd0-4587-b3ea-15572ea66229
📥 Commits

Reviewing files that changed from the base of the PR and between f1aca7e and 289b341.

📒 Files selected for processing (2)
  • lib/node.js
  • test/node.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

When source offsets are absent, Node.positionBy and Node.rangeBy include the character at the source end while locating a word. Tests cover position and range lookup for the terminal word red.

Changes: Node word lookup

Layer / File(s) Summary
Inclusive source-end lookup
lib/node.js, test/node.test.ts		 sourceOffset supports an inclusive end, which positionBy and rangeBy use when searching source text. Tests check the returned positions for the terminal word red when source offsets are absent.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 289b3

Word lookup now includes the final character when source offsets are inferred, while explicit offsets keep their existing behavior. No actionable merge risk is established.

Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: including terminal words when source offsets are absent.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@ai

ai commented Oct 13, 2026

Copy link
Copy Markdown
Member

Is it your real case or LLM told you to fix it?

@xiehuanyi

xiehuanyi commented Oct 15, 2026

Copy link
Copy Markdown
Author

@ai This was discovered by an LLM during a proactive source audit, not an issue I encountered in an application. The reproduction removes source offsets to exercise the fallback already covered by the existing word-range test. For a{color:red}, positionBy({ word: 'red' }) and rangeBy({ word: 'red' }) then start at column 3 instead of 9. I have not identified an affected real parser or a production incident; the evidence is this existing fallback behavior and the regression tests.

@ai

ai commented Oct 15, 2026

Copy link
Copy Markdown
Member

Sorry, not interesting. We are making code more complex without users who need it.

Please, stop looking for issues with LLMs. It is bad an unethical practice.

@ai ai closed this Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xiehuanyi @ai