Repository navigation
Feat: Lockfile First Dependency Graph - #12
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughImplements a lockfile-first dependency-graph workflow: adds lockfile parsers for pnpm/npm/Yarn, integrates lockfile-based tree construction into the collection pipeline with package-manager CLI fallback, extends CLI orchestration, sanitizes HTML report embedding, and adds two runtime dependencies. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Runner
participant npmLs as npmLs Handler
participant LockfileGraph as Lockfile<br/>Graph Builder
participant Fallback as Package Manager<br/>CLI
participant Report as Report<br/>Generator
CLI->>npmLs: runNpmLs(projectPath, tool, lockfileSearchRoot)
rect rgba(100,150,200,0.5)
Note over npmLs,LockfileGraph: Lockfile-first path
npmLs->>LockfileGraph: tryBuildDependencyTreeFromLockfile(projectPath, tool, lockfileSearchRoot)
alt Lockfile parsed successfully
LockfileGraph-->>npmLs: LockfileTreeResult
npmLs->>npmLs: write tree file
npmLs-->>CLI: success
else No valid lockfile
LockfileGraph-->>npmLs: undefined
end
end
rect rgba(200,150,100,0.5)
Note over npmLs,Fallback: CLI fallback path
npmLs->>Fallback: npm/pnpm/yarn list
Fallback-->>npmLs: dependency tree
npmLs->>npmLs: write tree file
npmLs-->>CLI: success
end
CLI->>Report: renderReport(scanResult)
Report->>Report: sanitize CSS/JS
Report->>Report: write HTML
Report-->>CLI: report generated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Contributor
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @JosephMaynard. The following files were modified: * `src/cli.ts` * `src/report.ts` * `src/runners/lockfileGraph.ts` * `src/runners/npmLs.ts` These files were ignored: * `src/runners/npmLs.test.ts` These file types are not supported: * `README.md` * `package.json`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cli.ts (1)
1181-1206: Add a preflight warning whennode_modulesis missing.Runners can return partial results without local installs. Consider warning early so users know to install dependencies before scanning.
🛠️ Suggested warning
const tempDir = path.join(projectPath, ".dependency-radar"); + const nodeModulesPath = path.join(projectPath, "node_modules"); + if (!(await pathExists(nodeModulesPath))) { + console.warn( + "⚠ node_modules not found. Install dependencies for complete results.", + ); + }Based on learnings "Runners rely on project-local
node_modules; ensure the target project is installed before scanning".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 1181 - 1206, Add a preflight check after detecting the workspace (after detectWorkspace(projectPath) and yarnPnP check) to detect if the project-local node_modules directory is missing (e.g., fs.existsSync(path.join(projectPath, "node_modules")) === false) and emit a clear console.warn advising users to run their package manager install (npm/yarn/pnpm) before scanning; keep execution flowing (do not exit) so existing behavior around rootPkg, detectPackageManager, and detectScanManager remains unchanged, and reference workspace.type/yarnPnP in the message when relevant (e.g., if workspace.type === "yarn" or yarnPnP is true) to provide contextual guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runners/lockfileGraph.ts`:
- Around line 507-511: The code currently computes projectRel/rootKey and sets
packageKey = rootKey in packages ? rootKey : '' which silently falls back to the
workspace root when a specific workspace package key is missing; change the
logic so that if rootKey is not present in packages and rootKey is not the empty
string (i.e., not the root package) the function returns undefined instead of
using the root entry. Concretely, in the block using
projectRel/rootKey/packageKey/rootEntry (symbols: projectRel, rootKey,
packageKey, rootEntry, packages, lockDir, projectPath) check membership with
`rootKey in packages`; if false and rootKey !== '' return undefined immediately,
otherwise proceed to read packages[rootKey] for the root case.
---
Nitpick comments:
In `@src/cli.ts`:
- Around line 1181-1206: Add a preflight check after detecting the workspace
(after detectWorkspace(projectPath) and yarnPnP check) to detect if the
project-local node_modules directory is missing (e.g.,
fs.existsSync(path.join(projectPath, "node_modules")) === false) and emit a
clear console.warn advising users to run their package manager install
(npm/yarn/pnpm) before scanning; keep execution flowing (do not exit) so
existing behavior around rootPkg, detectPackageManager, and detectScanManager
remains unchanged, and reference workspace.type/yarnPnP in the message when
relevant (e.g., if workspace.type === "yarn" or yarnPnP is true) to provide
contextual guidance.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli.ts`:
- Around line 1194-1208: The new warning strings in the node_modules check use
double quotes instead of the project's TypeScript style (single quotes); update
the string literals constructed in the hasProjectNodeModules block—specifically
the workspaceHint and yarnHint assignments and the console.warn message that
references projectPath, workspaceHint, and yarnHint—to use single quotes and
keep existing interpolation/templating intact; ensure you preserve semicolons
and existing spacing/indentation while changing only the quote characters.
Comment on lines
+1194
to
+1208
| const hasProjectNodeModules = await pathExists( | ||
| path.join(projectPath, "node_modules"), | ||
| ); | ||
| if (!hasProjectNodeModules) { | ||
| const workspaceHint = | ||
| workspace.type === "none" | ||
| ? "single project" | ||
| : `${workspace.type.toUpperCase()} workspace`; | ||
| const yarnHint = yarnPnP | ||
| ? " Yarn Plug'n'Play appears enabled; Dependency Radar currently requires node_modules linker." | ||
| : ""; | ||
| console.warn( | ||
| `⚠ node_modules was not found at ${projectPath}. Scan completeness may be reduced for this ${workspaceHint}. Run your package manager install (npm install, pnpm install, or yarn install) before scanning.${yarnHint}`, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Use single quotes for the new warning literals.
These new string literals use double quotes; the TS style guide here calls for single quotes.
✍️ Suggested diff
- const hasProjectNodeModules = await pathExists(
- path.join(projectPath, "node_modules"),
- );
+ const hasProjectNodeModules = await pathExists(
+ path.join(projectPath, 'node_modules'),
+ );
if (!hasProjectNodeModules) {
const workspaceHint =
- workspace.type === "none"
- ? "single project"
+ workspace.type === 'none'
+ ? 'single project'
: `${workspace.type.toUpperCase()} workspace`;
const yarnHint = yarnPnP
- ? " Yarn Plug'n'Play appears enabled; Dependency Radar currently requires node_modules linker."
- : "";
+ ? ' Yarn Plug\'n\'Play appears enabled; Dependency Radar currently requires node_modules linker.'
+ : '';As per coding guidelines, "Follow TypeScript style: 2-space indentation, single quotes, semicolons, and explicit return/parameter types under strict mode".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const hasProjectNodeModules = await pathExists( | |
| path.join(projectPath, "node_modules"), | |
| ); | |
| if (!hasProjectNodeModules) { | |
| const workspaceHint = | |
| workspace.type === "none" | |
| ? "single project" | |
| : `${workspace.type.toUpperCase()} workspace`; | |
| const yarnHint = yarnPnP | |
| ? " Yarn Plug'n'Play appears enabled; Dependency Radar currently requires node_modules linker." | |
| : ""; | |
| console.warn( | |
| `⚠ node_modules was not found at ${projectPath}. Scan completeness may be reduced for this ${workspaceHint}. Run your package manager install (npm install, pnpm install, or yarn install) before scanning.${yarnHint}`, | |
| ); | |
| } | |
| const hasProjectNodeModules = await pathExists( | |
| path.join(projectPath, 'node_modules'), | |
| ); | |
| if (!hasProjectNodeModules) { | |
| const workspaceHint = | |
| workspace.type === 'none' | |
| ? 'single project' | |
| : `${workspace.type.toUpperCase()} workspace`; | |
| const yarnHint = yarnPnP | |
| ? ' Yarn Plug\'n\'Play appears enabled; Dependency Radar currently requires node_modules linker.' | |
| : ''; | |
| console.warn( | |
| `⚠ node_modules was not found at ${projectPath}. Scan completeness may be reduced for this ${workspaceHint}. Run your package manager install (npm install, pnpm install, or yarn install) before scanning.${yarnHint}`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli.ts` around lines 1194 - 1208, The new warning strings in the
node_modules check use double quotes instead of the project's TypeScript style
(single quotes); update the string literals constructed in the
hasProjectNodeModules block—specifically the workspaceHint and yarnHint
assignments and the console.warn message that references projectPath,
workspaceHint, and yarnHint—to use single quotes and keep existing
interpolation/templating intact; ensure you preserve semicolons and existing
spacing/indentation while changing only the quote characters.
This was referenced Feb 23, 2026
Merged
Merged
This was referenced Mar 2, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Enhancements
Tests
Documentation