fix: match nested review sources before the node_modules exclude by viclafouch · Pull Request #302 · TanStack/intent

viclafouch · 2026-10-01T13:00:54Z

🎯 Changes

Fixes #301.

review lists the files of each source with git ls-files -- <source> <node_modules exclude>. When one include comes before the exclude, Git drops matches for some nested paths, so a valid source such as packages/client/src/** reports Source matched no available files.

list() now passes dependencyExclude before the patterns. diff() is unchanged: git diff returns the right files in both orders.

The new test in review.test.ts fails without the fix and passes with it.

✅ Checklist

I have followed the steps in the Contributing guide.
I have tested this code locally with pnpm run test:pr (run pnpm build:all first).

🚀 Release Impact

This change affects published code, and I have generated a changeset.
This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

Bug Fixes
- Review checks now correctly include files matching a nested source path, even when that path appears before installed-dependency exclusions. Files within node_modules remain excluded from the review snapshot. This resolves cases where a valid nested source was incorrectly reported as having no available files, while preserving the existing exclusion of dependency files from review results.

coderabbitai · 2026-10-01T13:01:07Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 39e684af-345f-497a-912c-4ff4915e87c2

📥 Commits

Reviewing files that changed from the base of the PR and between 5bebfd2 and f8932af.

📒 Files selected for processing (2)

packages/intent/src/review/review.ts
packages/intent/tests/review.test.ts

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

📝 Walkthrough

Walkthrough

The review file-list call now places the dependency exclusion before supplied source patterns. A regression test checks that a nested source glob finds its file without reporting problems.

Changes

Review source matching

Layer / File(s)	Summary
Order file-list arguments `packages/intent/src/review/review.ts`, `packages/intent/tests/review.test.ts`, `.changeset/review-source-pathspec-order.md`	The Git file-list call applies the dependency exclusion before source patterns. A regression test covers a nested source glob, and a changeset records the patch.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: ladybluenotes

Merge Risk: ⚪ Minimal · up to f8932

This fix lets the review command match nested source paths that previously reported no available files. The change is small, tested, and narrowly scoped, so there are no known merge-blocking risks.

Architecture Summary

Architecture risk: 🟡 Medium · up to f8932

The change affects 1 system.

Changed systems: packages/intent

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — packages/intent (library) was modified; 2 changed files map to changed impact.

Before / after behavior

observed — Modified behavior in packages/intent/src/review/review.ts: The list helper now orders Git pathspecs with dependencyExclude before caller-supplied patterns; previously the patterns preceded the dependency exclusion.
observed — Modified behavior in packages/intent/tests/review.test.ts: Added a nested-source-glob test that commits a package source file and a dependency file beneath src/node_modules, then checks that the review has no problems and snapshots only the package source file and the skill.
observed — Modified behavior in .changeset/review-source-pathspec-order.md: Adds patch-release metadata for @tanstack/intent and a changeset note describing the reported nested-source path-matching failure.

Reliability and maintainability

inferred — Risk-relevant change factors for packages/intent: blast_radius_1; direct_dependents_1

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary fix: ordering nested review source matching before the node_modules exclude.
Description check	✅ Passed	The description explains the problem, the code change, the unchanged behavior in diff(), the added regression test, and the required checklist and release impact information.
Linked Issues check	✅ Passed	Issue `#301` requires `review` to pass `dependencyExclude` before source patterns in `list()`. `packages/intent/src/review/review.ts` changes the argument order. The added `review.test.ts` case verifies…
Out of Scope Changes check	✅ Passed	The source change directly implements issue `#301`. The regression test verifies the required behavior. The changeset documents the user-facing fix. No unrelated change is identified in the supplied who…

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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.}

nx-cloud · 2026-10-01T20:26:56Z

View your CI Pipeline Execution ↗ for commit f8932af

Command	Status	Duration	Result
`nx affected --targets=test:eslint,test:sherif,t...`	✅ Succeeded	1m 3s	View ↗
`nx run-many --targets=build`	✅ Succeeded	3s	View ↗

☁️ Nx Cloud last updated this comment at 2026-10-03 01:59:40 UTC

pkg-pr-new · 2026-10-01T20:27:04Z

Open in StackBlitz

npm i https://pkg.pr.new/@tanstack/intent@302

commit: f8932af

LadyBluenotes · 2026-10-01T20:29:12Z

        '-z',
        '--',
-        ...patterns,
+        // Git drops matches when one include precedes the exclude (#301).


please remove this comment

sorry done !

LadyBluenotes

The test mentions excluding installed dependencies but doesn’t create any. Could you add a dependency fixture under a matching glob and assert that the snapshot includes the source file but excludes the dependency?

codspeed · 2026-10-01T20:31:46Z

Merging this PR will not alter performance

✅ 12 untouched benchmarks

_{Comparing viclafouch:fix-review-source-pathspec-order (f8932af) with main (4fbc23c)¹}

No successful run was found on main (2697183) during the generation of this report, so 4fbc23c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

viclafouch · 2026-10-02T07:41:08Z

The test mentions excluding installed dependencies but doesn’t create any. Could you add a dependency fixture under a matching glob and assert that the snapshot includes the source file but excludes the dependency?

Good point, test added

LadyBluenotes

Thank you!

viclafouch mentioned this pull request Oct 1, 2026

maintainer review: a source matches no file when the node_modules exclude comes after it #301

Closed

LadyBluenotes reviewed Oct 1, 2026

View reviewed changes

LadyBluenotes requested changes Oct 1, 2026

View reviewed changes

viclafouch added 2 commits October 2, 2026 09:39

fix: match nested review sources before the node_modules exclude

0b12d40

test: assert the nested source snapshot excludes installed dependencies

f8932af

viclafouch force-pushed the fix-review-source-pathspec-order branch from d1946f4 to f8932af Compare October 2, 2026 07:40

LadyBluenotes approved these changes Oct 3, 2026

View reviewed changes

LadyBluenotes merged commit f6e6286 into TanStack:main Oct 3, 2026
9 checks passed

github-actions Bot mentioned this pull request Oct 3, 2026

ci: Version Packages #304

Merged

Navigation Menu

Uh oh!

fix: match nested review sources before the node_modules exclude - #302

fix: match nested review sources before the node_modules exclude#302
LadyBluenotes merged 2 commits into
TanStack:mainfrom
viclafouch:fix-review-source-pathspec-order

viclafouch commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

nx-cloud Bot commented Oct 1, 2026 •

edited

Loading

Uh oh!

pkg-pr-new Bot commented Oct 1, 2026 •

edited

Loading

Uh oh!

LadyBluenotes Oct 1, 2026

Uh oh!

viclafouch Oct 2, 2026

Uh oh!

LadyBluenotes left a comment

Uh oh!

codspeed Bot commented Oct 1, 2026 •

edited

Loading

Uh oh!

viclafouch commented Oct 2, 2026

Uh oh!

LadyBluenotes left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

viclafouch commented Oct 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎯 Changes

✅ Checklist

🚀 Release Impact

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Architecture Summary

❌ Failed checks (1 warning)

Uh oh!

nx-cloud Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pkg-pr-new Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LadyBluenotes Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

viclafouch Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

LadyBluenotes left a comment

Choose a reason for hiding this comment

Uh oh!

codspeed Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

viclafouch commented Oct 2, 2026

Uh oh!

LadyBluenotes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

viclafouch commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading

nx-cloud Bot commented Oct 1, 2026 •

edited

Loading

pkg-pr-new Bot commented Oct 1, 2026 •

edited

Loading

codspeed Bot commented Oct 1, 2026 •

edited

Loading