Feat(analyzer): Add smart monorepo scope detection#43
Conversation
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Monorepo detection utilities src/analyzer/scopeDetector.ts |
Introduces isMonorepo(rootDir) to detect marker files (nx.json, lerna.json, turbo.json, workspace.yaml, pnpm-workspace.yaml) and detectMonorepoPackage(file) to extract package names from apps/, packages/, libs/, or services/ path segments. |
Scope detection integration src/analyzer/scopeDetector.ts |
Updates detectScope(filePaths, rootDir) signature to accept an optional rootDir, adds a hasMonorepoPathPattern() heuristic, and prefers monorepo package-derived scopes when active, falling back to existing detectFileScope logic while keeping counting and the "core" default. |
Test coverage src/analyzer/__tests__/scopeDetector.monorepo.test.ts, src/index.test.ts |
Adds tests that create temporary monorepo markers to validate isMonorepo, exercise detectMonorepoPackage edge cases (nested/absent segments), and verify detectScope behavior including filesystem-confirmed monorepo scoping, fallback paths, and most-frequent-package selection. |
Sequence Diagram(s)
sequenceDiagram
participant Caller
participant isMonorepo
participant detectMonorepoPackage
participant detectFileScope
Caller->>isMonorepo: check rootDir for markers
isMonorepo-->>Caller: true/false
Caller->>detectMonorepoPackage: extract package from file path
alt Monorepo active
detectMonorepoPackage-->>Caller: package name|null
else Monorepo inactive
Caller->>detectFileScope: extract scope from filename/path
detectFileScope-->>Caller: scope|null
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- nirvik34/gitbun#48: Also modifies
src/analyzer/scopeDetector.ts(Windows path parsing fix indetectFileScope) and may interact with monorepo fallback behavior.
Suggested labels
level: advanced, quality:clean, type:testing
Suggested reviewers
- 2PieRadian
Poem
🐰 I dug through folders, small and wide,
I sniffed for markers where they hide.
From apps and libs a name I found,
Scope declared — hop, safe and sound!
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The PR title 'Feat(analyzer): Add smart monorepo scope detection' clearly describes the main change of adding monorepo-aware scope detection to the analyzer. |
| Linked Issues check | ✅ Passed | The PR fully implements all objectives from issue #32: monorepo detection via marker files and path patterns, package scope extraction from standard directories, fallback behavior, and backward compatibility. |
| Out of Scope Changes check | ✅ Passed | All changes are directly related to implementing smart monorepo scope detection as specified in issue #32; no unrelated modifications are present. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands and usage tips.
|
Hi @nirvik34 I’ve completed the implementation for the smart monorepo-aware scope detection enhancement for Gitbun. Implemented improvements include:
I also added comprehensive test coverage including:
Additionally:
The changes have been pushed and the PR is ready for review. Thank you for assigning this issue and giving me the opportunity to contribute to Gitbun. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/analyzer/__tests__/scopeDetector.monorepo.test.ts (1)
69-75: ⚡ Quick winAdd regression coverage for nested
services/(orapps/) directories.This suite should assert that nested folders like
src/services/...do not get treated as monorepo package roots when monorepo mode is enabled.Proposed test addition
describe("detectScope with filesystem-confirmed monorepo", () => { @@ it("falls back to standard scope detection when rootDir is not a monorepo and no monorepo path pattern", () => { const files = ["src/analyzer/scopeDetector.ts"]; expect(detectScope(files, tmpDir)).toBe("analyzer"); }); + + it("does not treat nested services/ as monorepo package root", () => { + fs.writeFileSync(path.join(tmpDir, "nx.json"), "{}"); + const files = ["src/services/auth/handler.ts"]; + expect(detectScope(files, tmpDir)).toBe("services"); + });Also applies to: 89-104
🤖 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 `@src/analyzer/__tests__/scopeDetector.monorepo.test.ts` around lines 69 - 75, Add regression tests to src/analyzer/__tests__/scopeDetector.monorepo.test.ts that assert detectMonorepoPackage returns null for nested "services" or "apps" directories (e.g., "src/services/foo/package.json", "src/services/foo/src/index.ts", "src/apps/bar/package.json") so nested folders like src/services/... or src/apps/... are not treated as monorepo package roots; place these new it() cases alongside the existing tests that check for null (the ones referencing detectMonorepoPackage) and ensure they run under the same monorepo-mode test file/describe block.
🤖 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 `@src/analyzer/scopeDetector.ts`:
- Around line 31-35: The current loop using MONOREPO_ROOT_SEGMENTS returns the
next path segment at any depth, causing false positives (e.g., matching
src/services/...). Update the check inside the loop in the function that
computes parts (the code using MONOREPO_ROOT_SEGMENTS and parts) to only accept
a match when the segment is repo-root-relative (i.e., require idx === 0) before
returning parts[idx + 1]; if the segment is found at deeper indexes, skip it so
fallback file-based scope detection still runs.
---
Nitpick comments:
In `@src/analyzer/__tests__/scopeDetector.monorepo.test.ts`:
- Around line 69-75: Add regression tests to
src/analyzer/__tests__/scopeDetector.monorepo.test.ts that assert
detectMonorepoPackage returns null for nested "services" or "apps" directories
(e.g., "src/services/foo/package.json", "src/services/foo/src/index.ts",
"src/apps/bar/package.json") so nested folders like src/services/... or
src/apps/... are not treated as monorepo package roots; place these new it()
cases alongside the existing tests that check for null (the ones referencing
detectMonorepoPackage) and ensure they run under the same monorepo-mode test
file/describe block.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f1032fbf-380f-4209-b4b6-b5f3895cb56b
📒 Files selected for processing (3)
src/analyzer/__tests__/scopeDetector.monorepo.test.tssrc/analyzer/scopeDetector.tssrc/index.test.ts
nirvik34
left a comment
nirvik34
left a comment
There was a problem hiding this comment.
Update detectMonorepoPackage() in src/analyzer/scopeDetector.ts
Change the loop to only match when the segment is at index 0 (root-relative) and Add regression tests in src/analyzer/tests/scopeDetector.monorepo.test.ts
Add these test cases to the detectMonorepoPackage describe block.
|
Hey @nirvik34, |
Sujan075
force-pushed
the
issue-32-monorepo-scope-detection
branch
from
5df89e8 to
f0b5ee1
Compare
|
@Sujan075 is attempting to deploy a commit to the nirvik34's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @nirvik34 Thanks for the review and for pointing out the edge case. I’ve updated I also added regression tests covering:
All tests are passing successfully after the update. Thank you for the feedback. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/analyzer/scopeDetector.ts (1)
26-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize input paths before monorepo segment detection (Windows and
./paths currently miss).Line 29 and Line 65 split only on
/, and Line 83 passes raw file paths. On Windows-style or./-prefixed inputs, monorepo scope detection is skipped and fallback scoping can produce the wrong scope.💡 Proposed fix
export function detectMonorepoPackage( file: string ): string | null { - const parts = file.split("/"); + const normalized = file + .replace(/\\/g, "/") + .replace(/^\.\//, ""); + const parts = normalized.split("/").filter(Boolean); const firstSegment = parts[0]; if (MONOREPO_ROOT_SEGMENTS.includes(firstSegment) && parts[1]) { return parts[1]; } return null; } function hasMonorepoPathPattern(filePaths: string[]): boolean { - return filePaths.some((file) => { - const firstSegment = file.split("/")[0]; - return MONOREPO_ROOT_SEGMENTS.includes(firstSegment); - }); + return filePaths.some((file) => detectMonorepoPackage(file) !== null); } export function detectScope( filePaths: string[], rootDir: string = process.cwd() ): string { @@ for (const file of filePaths) { let scope: string | null = null; + const fileForDetection = path.isAbsolute(file) + ? path.relative(rootDir, file) + : file; if (monorepoActive) { - scope = detectMonorepoPackage(file); + scope = detectMonorepoPackage(fileForDetection); } if (!scope) { - scope = detectFileScope(file); + scope = detectFileScope(fileForDetection); }Also applies to: 63-67, 79-88
🤖 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 `@src/analyzer/scopeDetector.ts` around lines 26 - 33, Paths are not normalized before splitting, so Windows backslashes and leading "./" cause monorepo detection in detectMonorepoPackage (and the other path-splitting checks around the same area) to miss matches; normalize inputs first by converting backslashes to slashes and removing a leading "./" (or use path.normalize/path.posix behavior) before calling split and checking MONOREPO_ROOT_SEGMENTS, then update every other place that splits file paths the same way (the other blocks that currently call file.split("/") around the same scope) to use the same normalized path helper.
🤖 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.
Duplicate comments:
In `@src/analyzer/scopeDetector.ts`:
- Around line 26-33: Paths are not normalized before splitting, so Windows
backslashes and leading "./" cause monorepo detection in detectMonorepoPackage
(and the other path-splitting checks around the same area) to miss matches;
normalize inputs first by converting backslashes to slashes and removing a
leading "./" (or use path.normalize/path.posix behavior) before calling split
and checking MONOREPO_ROOT_SEGMENTS, then update every other place that splits
file paths the same way (the other blocks that currently call file.split("/")
around the same scope) to use the same normalized path helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1131dd9f-48a3-42ef-a3d9-29019103dd50
📒 Files selected for processing (3)
src/analyzer/__tests__/scopeDetector.monorepo.test.tssrc/analyzer/scopeDetector.tssrc/index.test.ts
2 participants
Summary
Adds smart monorepo-aware scope detection to improve commit scope generation for workspace-based repositories such as Nx, Turborepo, Lerna, and pnpm workspaces.
Closes #32
Changes Made
Core Scope Detection Enhancements (
src/analyzer/scopeDetector.ts)Added monorepo detection support using common workspace marker files:
nx.jsonlerna.jsonturbo.jsonworkspace.yamlpnpm-workspace.yamlAdded
isMonorepo()helper for filesystem-based monorepo detectionAdded
detectMonorepoPackage()helper to extract package names from common monorepo structures:apps/packages/libs/services/Added path-pattern heuristic detection to support repositories using monorepo layouts without explicit config files
Updated
detectScope()to:rootDirTest Coverage
Updated
src/index.test.tsAdded test cases for:
apps/scope detectionpackages/scope detectionlibs/scope detectionservices/scope detectionAdded
src/analyzer/__tests__/scopeDetector.monorepo.test.tsAdded dedicated tests covering:
Stability & Compatibility
Validation
Tested using:
npm testnpm run lintAll 47 tests are passing successfully.
Summary by CodeRabbit
New Features
Tests