Repository navigation
Feat: CLI Summary - #17
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a CLI flag Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Aggregator
participant Runner as "Runner(s)"
participant Disk as "Disk (.dependency-radar/)"
participant User
CLI->>Aggregator: parse options (includes --no-report)
Aggregator->>Runner: run tasks with persistToDisk flag
alt persistToDisk = true
Runner->>Disk: write JSON/HTML/artifacts
Runner-->>Aggregator: result + file path
else persistToDisk = false
Runner-->>Aggregator: result (no file)
end
Aggregator->>CLI: buildCliSummary(AggregatedData)
CLI-->>User: print summary & status (colorized)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![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 (3)
src/runners/npmOutdated.ts (1)
102-107: Consider standardizing runner persistence APIs on an options object.This runner uses a positional boolean while other runners use
{ persistToDisk }; unifying reduces call-site mistakes and keeps runner contracts consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runners/npmOutdated.ts` around lines 102 - 107, The runPackageOutdated function currently accepts a positional boolean for persistence; change its signature to accept an options object (e.g., replace the positional parameter with options: { persistToDisk?: boolean } = {}) and default persistToDisk to true inside that options object, update references inside runPackageOutdated to use options.persistToDisk, and update all callers of runPackageOutdated to pass an options object ({ persistToDisk }) instead of a positional boolean to match other runners' APIs.README.md (1)
224-229: Add one line about--keep-tempwith--no-report.Consider adding a short note that
--keep-temphas no effect when--no-reportis set, so users don’t expect.dependency-radar/output in that mode.Based on learnings, "Temporary files in
.dependency-radar/may contain dependency metadata; avoid committing them and remove unless debugging with--keep-temp."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 224 - 229, Update the README section showing "npx dependency-radar --no-report" to add a single-line note stating that the --keep-temp flag has no effect when --no-report is used and that temporary files in .dependency-radar/ may contain dependency metadata and should not be committed (remove them unless debugging with --keep-temp); place this sentence directly beneath the example so readers immediately see the caveat.src/runners/npmLs.ts (1)
24-32: Update the function contract docs for conditionalfileoutput.The current comment still implies
fileis always present; now it’s conditional onoptions.persistToDisk !== false.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runners/npmLs.ts` around lines 24 - 32, Update the function JSDoc to state that `file` is conditional: when `options.persistToDisk === false` the returned object will omit `file` (or set it to undefined), otherwise `file` is the path written to disk; keep the rest of the return description for `data` and `error` intact and reference `options.persistToDisk` and the `data`, `file`, `error` fields so callers understand the conditional presence of `file`.
🤖 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 1241-1243: The code counts a dependency as "unused" when
importUsage is missing because it uses a falsy check; change the condition to
only treat an explicitly false importUsage as unused. Update the if in
src/cli.ts (the block using dep.usage.direct, dep.usage.scope and
dep.usage.importUsage) so the last clause is dep.usage.importUsage === false
(and keep the existing dep.usage.direct and dep.usage.scope === "runtime"
checks) to avoid inflating metrics when importUsage is undefined.
---
Nitpick comments:
In `@README.md`:
- Around line 224-229: Update the README section showing "npx dependency-radar
--no-report" to add a single-line note stating that the --keep-temp flag has no
effect when --no-report is used and that temporary files in .dependency-radar/
may contain dependency metadata and should not be committed (remove them unless
debugging with --keep-temp); place this sentence directly beneath the example so
readers immediately see the caveat.
In `@src/runners/npmLs.ts`:
- Around line 24-32: Update the function JSDoc to state that `file` is
conditional: when `options.persistToDisk === false` the returned object will
omit `file` (or set it to undefined), otherwise `file` is the path written to
disk; keep the rest of the return description for `data` and `error` intact and
reference `options.persistToDisk` and the `data`, `file`, `error` fields so
callers understand the conditional presence of `file`.
In `@src/runners/npmOutdated.ts`:
- Around line 102-107: The runPackageOutdated function currently accepts a
positional boolean for persistence; change its signature to accept an options
object (e.g., replace the positional parameter with options: { persistToDisk?:
boolean } = {}) and default persistToDisk to true inside that options object,
update references inside runPackageOutdated to use options.persistToDisk, and
update all callers of runPackageOutdated to pass an options object ({
persistToDisk }) instead of a positional boolean to match other runners' APIs.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
dist/cli.jsis excluded by!**/dist/**dist/runners/importGraphRunner.jsis excluded by!**/dist/**dist/runners/npmAudit.jsis excluded by!**/dist/**dist/runners/npmLs.jsis excluded by!**/dist/**dist/runners/npmOutdated.jsis excluded by!**/dist/**
📒 Files selected for processing (6)
README.mdsrc/cli.tssrc/runners/importGraphRunner.tssrc/runners/npmAudit.tssrc/runners/npmLs.tssrc/runners/npmOutdated.ts
Comment on lines
+1241
to
+1243
| if (dep.usage.direct && dep.usage.scope === "runtime" && !dep.usage.importUsage) { | ||
| unusedInstalledDeps += 1; | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid treating missing import-usage data as “unused dependency.”
This currently counts dependencies as unused when importUsage is absent, which can happen from unavailable/partial import graph data and inflate the metric.
💡 Proposed fix
- if (dep.usage.direct && dep.usage.scope === "runtime" && !dep.usage.importUsage) {
+ const importFileCount = dep.usage.importUsage?.fileCount;
+ if (
+ dep.usage.direct &&
+ dep.usage.scope === "runtime" &&
+ typeof importFileCount === "number" &&
+ importFileCount === 0
+ ) {
unusedInstalledDeps += 1;
}📝 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
| if (dep.usage.direct && dep.usage.scope === "runtime" && !dep.usage.importUsage) { | |
| unusedInstalledDeps += 1; | |
| } | |
| const importFileCount = dep.usage.importUsage?.fileCount; | |
| if ( | |
| dep.usage.direct && | |
| dep.usage.scope === "runtime" && | |
| typeof importFileCount === "number" && | |
| importFileCount === 0 | |
| ) { | |
| unusedInstalledDeps += 1; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli.ts` around lines 1241 - 1243, The code counts a dependency as
"unused" when importUsage is missing because it uses a falsy check; change the
condition to only treat an explicitly false importUsage as unused. Update the if
in src/cli.ts (the block using dep.usage.direct, dep.usage.scope and
dep.usage.importUsage) so the last clause is dep.usage.importUsage === false
(and keep the existing dep.usage.direct and dep.usage.scope === "runtime"
checks) to avoid inflating metrics when importUsage is undefined.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/cli.ts (1)
1246-1252:⚠️ Potential issue | 🟡 MinorMake unused-dependency counting explicit with
fileCount === 0.On Line 1250, using
!dep.usage.importUsagecan still blur “unknown” vs “unused”. Prefer counting only when a numeric import file count is explicitly zero.💡 Suggested fix
- if ( - options.importGraphComplete && - dep.usage.direct && - dep.usage.scope === "runtime" && - !dep.usage.importUsage - ) { + const importFileCount = dep.usage.importUsage?.fileCount; + if ( + options.importGraphComplete && + dep.usage.direct && + dep.usage.scope === "runtime" && + typeof importFileCount === "number" && + importFileCount === 0 + ) { unusedInstalledDeps += 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 1246 - 1252, The current unused-dependency increment uses a truthy check (!dep.usage.importUsage) which conflates "unknown" with "unused"; change the condition in the block that updates unusedInstalledDeps to explicitly test that importUsage.fileCount is numeric zero (e.g., use dep.usage.importUsage?.fileCount === 0) so only dependencies with a known import file count of 0 are counted; keep the other conditions (options.importGraphComplete, dep.usage.direct, dep.usage.scope === "runtime") unchanged and still reference unusedInstalledDeps, dep.usage.direct, dep.usage.scope, and options.importGraphComplete when locating the code to modify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/cli.ts`:
- Around line 1246-1252: The current unused-dependency increment uses a truthy
check (!dep.usage.importUsage) which conflates "unknown" with "unused"; change
the condition in the block that updates unusedInstalledDeps to explicitly test
that importUsage.fileCount is numeric zero (e.g., use
dep.usage.importUsage?.fileCount === 0) so only dependencies with a known import
file count of 0 are counted; keep the other conditions
(options.importGraphComplete, dep.usage.direct, dep.usage.scope === "runtime")
unchanged and still reference unusedInstalledDeps, dep.usage.direct,
dep.usage.scope, and options.importGraphComplete when locating the code to
modify.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
dist/cli.jsis excluded by!**/dist/**dist/runners/npmLs.jsis excluded by!**/dist/**dist/runners/npmOutdated.jsis excluded by!**/dist/**
📒 Files selected for processing (4)
README.mdsrc/cli.tssrc/runners/npmLs.tssrc/runners/npmOutdated.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/runners/npmOutdated.ts
- README.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/cli.ts (1)
1260-1265:⚠️ Potential issue | 🟡 MinorDon’t count unknown import-usage as “unused.”
On Line 1264,
!dep.usage.importUsagetreats missing/unknown usage as unused and can inflateunusedInstalledDeps. This concern was already raised in prior review feedback and still appears unresolved.💡 Proposed fix
- if ( - options.importGraphComplete && - dep.usage.direct && - dep.usage.scope === "runtime" && - !dep.usage.importUsage - ) { + const importFileCount = dep.usage.importUsage?.fileCount; + if ( + options.importGraphComplete && + dep.usage.direct && + dep.usage.scope === "runtime" && + typeof importFileCount === "number" && + importFileCount === 0 + ) { unusedInstalledDeps += 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 1260 - 1265, The condition treating missing/unknown import usage as unused should be tightened: instead of using !dep.usage.importUsage (which treats undefined/null as false), only count a dependency as unused when importUsage is explicitly false; update the conditional that uses options.importGraphComplete && dep.usage.direct && dep.usage.scope === "runtime" && !dep.usage.importUsage to check dep.usage.importUsage === false, so unknown/missing values are not included in unusedInstalledDeps.
🤖 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`:
- Line 1366: Replace the expression-bodied forEach call on blockerDetails with a
for...of loop to satisfy the useIterableCallbackReturn lint rule: iterate over
blockerDetails using for (const line of blockerDetails) and call
console.log(line) inside the loop body so the callback is not an
expression-returning function; update the code where
blockerDetails.forEach((line) => console.log(line)) is present.
---
Duplicate comments:
In `@src/cli.ts`:
- Around line 1260-1265: The condition treating missing/unknown import usage as
unused should be tightened: instead of using !dep.usage.importUsage (which
treats undefined/null as false), only count a dependency as unused when
importUsage is explicitly false; update the conditional that uses
options.importGraphComplete && dep.usage.direct && dep.usage.scope === "runtime"
&& !dep.usage.importUsage to check dep.usage.importUsage === false, so
unknown/missing values are not included in unusedInstalledDeps.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dist/cli.jsis excluded by!**/dist/**
📒 Files selected for processing (2)
README.mdsrc/cli.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
Contributor
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @JosephMaynard. The following files were modified: * `src/cli.ts` * `src/runners/importGraphRunner.ts` * `src/runners/npmAudit.ts` * `src/runners/npmLs.ts` * `src/runners/npmOutdated.ts` These file types are not supported: * `README.md`
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/cli.ts (1)
1341-1347:⚠️ Potential issue | 🟡 MinorAvoid counting unknown import-usage as unused dependencies.
This still treats absent
importUsageas unused, which can inflateunusedInstalledDeps. Count only explicit “unused” signals.💡 Suggested fix
- if ( - options.importGraphComplete && - dep.usage.direct && - dep.usage.scope === "runtime" && - !dep.usage.importUsage - ) { + const importUsage = dep.usage.importUsage; + const importFileCount = + importUsage && typeof importUsage === "object" + ? (importUsage as { fileCount?: number }).fileCount + : undefined; + const explicitlyUnused = + importUsage === false || + (typeof importFileCount === "number" && importFileCount === 0); + if ( + options.importGraphComplete && + dep.usage.direct && + dep.usage.scope === "runtime" && + explicitlyUnused + ) { unusedInstalledDeps += 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 1341 - 1347, The current conditional in src/cli.ts increments unusedInstalledDeps when importUsage is missing; change it to only count explicit "unused" signals by adding a strict check for dep.usage.importUsage === "unused" (while keeping options.importGraphComplete, dep.usage.direct, and dep.usage.scope === "runtime" checks) so unknown/absent importUsage values are excluded from unusedInstalledDeps increments.
🧹 Nitpick comments (1)
src/runners/npmAudit.ts (1)
84-90: Consider using an options object for consistency with other runners.This function uses a positional boolean
persistToDisk = true, whilerunPackageOutdatedusesoptions: { persistToDisk?: boolean }andrunNpmLsusesLsProgressOptions. Positional booleans reduce readability at call sites (e.g.,runPackageAudit(path, temp, 'npm', undefined, false)).♻️ Suggested refactor to align with other runners
+type AuditOptions = { + yarnVersion?: string; + persistToDisk?: boolean; +}; + export async function runPackageAudit( projectPath: string, tempDir: string, tool: "npm" | "pnpm" | "yarn", - yarnVersion?: string, - persistToDisk = true, + options: AuditOptions = {}, ): Promise<ToolResult<any>> { + const persistToDisk = options.persistToDisk !== false; const targetFile = path.join(tempDir, `${tool}-audit.json`); try { - const { cmd, args, lockFiles } = buildAuditCommand(tool, yarnVersion); + const { cmd, args, lockFiles } = buildAuditCommand(tool, options.yarnVersion);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runners/npmAudit.ts` around lines 84 - 90, The runPackageAudit function currently accepts a positional boolean persistToDisk; change its signature to take an options object (e.g., options?: { persistToDisk?: boolean }) to match runPackageOutdated and LsProgressOptions usage, update internal references inside runPackageAudit to read options.persistToDisk with a default of true, and update any call sites that pass the positional boolean (e.g., calls to runPackageAudit(..., false)) to pass an options object { persistToDisk: false } so call-site readability is preserved; keep the tool, yarnVersion, and other parameters unchanged aside from switching the final boolean into the options object.
🤖 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`:
- Line 1412: The summary output prints "Licence mismatches" with British
spelling; change that literal to use the project's standard "License" spelling
so the console.log in this area prints `${bullet} License mismatches:
${summary.licenseMismatches}` instead of "Licence". Locate the console.log call
that emits the summary string (the line with `${bullet} Licence mismatches`) and
update the displayed label only, leaving the summary.licenseMismatches variable
intact.
In `@src/runners/npmLs.ts`:
- Around line 102-111: The JSDoc comment above the runPnpmLsWithFallback
function is missing the closing "*/", causing a syntax error; fix it by adding
the terminating "*/" immediately before the async function declaration for
runPnpmLsWithFallback (or otherwise close the JSDoc block so the comment ends)
so the function signature async function runPnpmLsWithFallback(projectPath:
string, targetFile: string, options: LsProgressOptions):
Promise<ToolResult<any>> is valid and the file compiles.
---
Duplicate comments:
In `@src/cli.ts`:
- Around line 1341-1347: The current conditional in src/cli.ts increments
unusedInstalledDeps when importUsage is missing; change it to only count
explicit "unused" signals by adding a strict check for dep.usage.importUsage ===
"unused" (while keeping options.importGraphComplete, dep.usage.direct, and
dep.usage.scope === "runtime" checks) so unknown/absent importUsage values are
excluded from unusedInstalledDeps increments.
---
Nitpick comments:
In `@src/runners/npmAudit.ts`:
- Around line 84-90: The runPackageAudit function currently accepts a positional
boolean persistToDisk; change its signature to take an options object (e.g.,
options?: { persistToDisk?: boolean }) to match runPackageOutdated and
LsProgressOptions usage, update internal references inside runPackageAudit to
read options.persistToDisk with a default of true, and update any call sites
that pass the positional boolean (e.g., calls to runPackageAudit(..., false)) to
pass an options object { persistToDisk: false } so call-site readability is
preserved; keep the tool, yarnVersion, and other parameters unchanged aside from
switching the final boolean into the options object.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dist/cli.jsis excluded by!**/dist/**
📒 Files selected for processing (5)
src/cli.tssrc/runners/importGraphRunner.tssrc/runners/npmAudit.tssrc/runners/npmLs.tssrc/runners/npmOutdated.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/runners/importGraphRunner.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/cli.ts (1)
1342-1348:⚠️ Potential issue | 🟡 MinorUnused-dependency metric still treats “missing usage data” as unused.
At Line 1346,
!dep.usage.importUsagecounts deps as unused when usage data is absent, and misses explicitfileCount === 0objects.💡 Proposed fix
if ( options.importGraphComplete && dep.usage.direct && dep.usage.scope === "runtime" && - !dep.usage.importUsage + typeof dep.usage.importUsage?.fileCount === "number" && + dep.usage.importUsage.fileCount === 0 ) { unusedInstalledDeps += 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 1342 - 1348, The unused-dependency count wrongly treats absent import usage as unused; update the condition that increments unusedInstalledDeps (the block using options.importGraphComplete, dep.usage.direct, dep.usage.scope === "runtime") to only count a dependency as unused when import usage exists and shows zero files, e.g. require dep.usage.importUsage to be present and check dep.usage.importUsage.fileCount === 0 instead of using !dep.usage.importUsage so missing usage data is not classified as unused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/cli.ts`:
- Around line 1342-1348: The unused-dependency count wrongly treats absent
import usage as unused; update the condition that increments unusedInstalledDeps
(the block using options.importGraphComplete, dep.usage.direct, dep.usage.scope
=== "runtime") to only count a dependency as unused when import usage exists and
shows zero files, e.g. require dep.usage.importUsage to be present and check
dep.usage.importUsage.fileCount === 0 instead of using !dep.usage.importUsage so
missing usage data is not classified as unused.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
dist/cli.jsis excluded by!**/dist/**dist/runners/importGraphRunner.jsis excluded by!**/dist/**dist/runners/npmAudit.jsis excluded by!**/dist/**dist/runners/npmLs.jsis excluded by!**/dist/**dist/runners/npmOutdated.jsis excluded by!**/dist/**
📒 Files selected for processing (3)
src/cli.tssrc/runners/npmAudit.tssrc/runners/npmLs.ts
Merged
This was referenced Apr 27, 2026
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
Behavior
Documentation