fix(cli): make --diff/--staged work when the project is not at the git repo root - #114
Conversation
…t repo root git diff --name-only reports repo-root-relative paths, but Result.location is resolved relative to the analyzed cwd. When a SvelteKit project lives in a monorepo subdirectory (e.g. apps/web/), the mismatch made changed.has(location) always false, silently dropping every finding and exiting 0 -- the worst failure mode for a CI/pre-commit gate. Add --relative to both git diff calls so paths match the cwd-relative basis of Result.location; git ls-files --others was already cwd-relative and needed no change.
📝 WalkthroughWalkthroughThis PR fixes ChangesChanged-files path resolution fix
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/test/changed-files.test.ts (1)
62-123: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting shared commit-setup helper.
The "create dir, write file,
git add,git commit" sequence is repeated near-identically across all four tests. A smallcommitFile(repo, projectDir, relPath, contents)helper would reduce duplication.🤖 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/cli/test/changed-files.test.ts` around lines 62 - 123, The four tests in changed-files.test.ts repeat the same repository setup and commit sequence, making the fixture noisy and harder to maintain. Extract the shared “create directories, write file, git add, git commit” flow into a small helper such as commitFile used by the getChangedFiles test cases. Keep the helper flexible enough to cover both the subdirectory projectDir cases and the repo-root case, and update each test to call it with the appropriate repo, projectDir, relative path, and contents.
🤖 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/cli/test/changed-files.test.ts`:
- Around line 62-123: The four tests in changed-files.test.ts repeat the same
repository setup and commit sequence, making the fixture noisy and harder to
maintain. Extract the shared “create directories, write file, git add, git
commit” flow into a small helper such as commitFile used by the getChangedFiles
test cases. Keep the helper flexible enough to cover both the subdirectory
projectDir cases and the repo-root case, and update each test to call it with
the appropriate repo, projectDir, relative path, and contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92dd4f13-a6fb-4c1c-beb5-b7b20547a537
📒 Files selected for processing (3)
.changeset/fix-diff-staged-repo-subdirectory.mdpackages/cli/src/changed-files.tspackages/cli/test/changed-files.test.ts
There was a problem hiding this comment.
Pull request overview
This PR fixes svelte-vitals --diff / --staged failing silently (reporting no findings) when run from a project directory that is a subdirectory of a git repository (e.g., monorepos). It aligns git diff --name-only output with the CLI’s Result.location path basis by making git emit cwd-relative paths.
Changes:
- Add
--relativetogit diff --name-onlyinvocations so changed-file paths arecwd-relative when running from a subdirectory. - Add regression tests that create real temporary git repositories to cover tracked, staged, untracked, and repo-root scenarios.
- Add a changeset for a patch release documenting the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/cli/src/changed-files.ts | Uses git diff --relative so changed paths match Result.location when the project is not at the repo root. |
| packages/cli/test/changed-files.test.ts | Adds regression tests using temp git repos to verify subdirectory behavior for tracked/staged/untracked changes and root behavior. |
| .changeset/fix-diff-staged-repo-subdirectory.md | Patch changeset describing the behavioral fix for monorepos/subdirectory projects. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
svelte-vitals --diff/--stagedsilently reported zero findings (exit 0) whenever the analyzed SvelteKit project was not at the git repository root (e.g. a monorepo'sapps/web/).Root cause: a path-basis mismatch.
git diff --name-onlyoutputs repo-root-relative paths (apps/web/src/routes/+page.svelte) while findings carry cwd-relative locations (src/routes/+page.svelte), sochanged.has(location)never matched — the worst failure mode for a CI/pre-commit gate (silent false pass). Subtly, untracked files merged viagit ls-files --otherswere already cwd-relative, so only tracked changes were affected.Fix
Add
--relativeto bothgit diffinvocations ingetChangedFiles(packages/cli/src/changed-files.ts), making the output cwd-relative and scoped to the analyzed directory — the same basis asResult.location.git ls-files --othersis untouched (already cwd-relative).Tests
New regression tests in
packages/cli/test/changed-files.test.tsusing real temporary git repositories:--stagedin a subdirectoryls-filespathVerification
pnpm typecheck/pnpm lint— cleanpnpm test— 719 tests pass across all packages (core 343 / vite 79 / cli 288 incl. 4 new / mcp 9)svelte-vitals, patch)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
--diffand--stagednow report the correct changed files instead of missing results.Tests