Conversation
WalkthroughAdds a repository import-graph builder and integrates it into scan and doc-init flows, introduces parallelized domain and architecture discovery, updates CLI/options and package metadata, adds contributor and GitHub templates, and includes comprehensive graph and scanner tests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as "bin/cli.js"
participant Scan as "src/commands/scan.js"
participant Scanner as "src/lib/scanner.js"
participant Graph as "src/lib/graph-builder.js"
User->>CLI: run scan (with --domains/--verbose)
CLI->>Scan: scanCommand(path, options)
Scan->>Scanner: scanRepo(repoPath, { extraDomains })
Scanner-->>Scan: result (languages, domains, health)
Scan->>Graph: buildRepoGraph(repoPath, result.languages)
Graph-->>Scan: repoGraph (hubs, clusters, hotspots)
Scan-->>CLI: formatted graph + health output
CLI-->>User: display results
sequenceDiagram
actor User
participant CLI as "bin/cli.js"
participant DocInit as "src/commands/doc-init.js"
participant Graph as "src/lib/graph-builder.js"
participant ClaudeA as "Claude (Domains)"
participant ClaudeB as "Claude (Architecture)"
participant DocGen as "Documentation Generator"
User->>CLI: run doc init
CLI->>DocInit: docInitCommand(path, options)
DocInit->>Graph: buildRepoGraph(repoPath, languages)
Graph-->>DocInit: repoGraph
DocInit->>ClaudeA: discover-domains prompt (parallel)
DocInit->>ClaudeB: discover-architecture prompt (parallel)
ClaudeA-->>DocInit: domain findings
ClaudeB-->>DocInit: architecture findings
DocInit->>DocInit: merge findings, select effective domains
DocInit->>ClaudeA: generateChunked per-domain (batched, limit=3)
ClaudeA-->>DocInit: per-domain docs
DocInit->>DocGen: assemble CLAUDE.md
DocGen-->>CLI: complete
CLI-->>User: show elapsed time & token summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/graph-builder.js (1)
508-528: Consider adding a timeout error message for verbose mode.
execSynchas a 5-second timeout, but failures are silently caught. In very large repos, users might wonder why churn data is missing. However, this is acceptable since churn is supplementary data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/graph-builder.js` around lines 508 - 528, The analyzeGitChurn function currently swallows all errors from execSync; update its catch block to emit a clear timeout/exec error when running in verbose mode so users know why churn is missing. In the catch for analyzeGitChurn, inspect the caught error (e.g., check err.code === 'ERR_CHILD_PROCESS_TIMED_OUT' or err.killed/signal) and, when a verbose flag is set (use process.env.VERBOSE or an existing logger), log a concise warning that includes repoPath and the error message/stack; otherwise keep returning the empty churn object.src/lib/scanner.js (1)
33-67: Minor:sourceFileCountfallback.Line 55 uses
modules.length || undefined, but0is a valid count. This won't cause bugs since the domain was matched, butsourceFileCount: modules.lengthwould be more accurate.Suggested fix
- sourceFileCount: modules.length || undefined, + sourceFileCount: modules.length,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/scanner.js` around lines 33 - 67, In mergeExtraDomains, the matched domain sets sourceFileCount using "modules.length || undefined" which converts a valid 0 to undefined; change it to set sourceFileCount to modules.length so zero is preserved (update the object constructed in mergeExtraDomains where sourceFileCount is assigned); keep the rest of the structure unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 103-122: The fenced code block containing the project directory
listing (starts with entries like bin/cli.js, src/, commands/, tests/) lacks a
language tag; change the opening fence from ``` to ```text so the block is
explicitly marked as plain text (e.g., modify the CONTRIBUTING.md fenced block
that lists bin/cli.js, src/, commands/, etc., to begin with ```text).
In `@tests/graph-builder.test.js`:
- Around line 42-117: Tests fail because es-module-lexer isn’t initialized
before parseJsImports is used; update the code so parseJsImports always runs
after init. Either (a) export and call the existing init from graph-builder.js
(the same init used by buildRepoGraph) at the top of the test file before
invoking parseJsImports, or (b) make parseJsImports async and call await init()
inside parseJsImports before calling parse(), ensuring parseJsImports and any
callers (tests) await it; reference parseJsImports, init, and buildRepoGraph
when locating the code to change.
---
Nitpick comments:
In `@src/lib/graph-builder.js`:
- Around line 508-528: The analyzeGitChurn function currently swallows all
errors from execSync; update its catch block to emit a clear timeout/exec error
when running in verbose mode so users know why churn is missing. In the catch
for analyzeGitChurn, inspect the caught error (e.g., check err.code ===
'ERR_CHILD_PROCESS_TIMED_OUT' or err.killed/signal) and, when a verbose flag is
set (use process.env.VERBOSE or an existing logger), log a concise warning that
includes repoPath and the error message/stack; otherwise keep returning the
empty churn object.
In `@src/lib/scanner.js`:
- Around line 33-67: In mergeExtraDomains, the matched domain sets
sourceFileCount using "modules.length || undefined" which converts a valid 0 to
undefined; change it to set sourceFileCount to modules.length so zero is
preserved (update the object constructed in mergeExtraDomains where
sourceFileCount is assigned); keep the rest of the structure unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e13c237a-cd7b-4d39-afbf-0f79a3b1972c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.github/ISSUE_TEMPLATE/bug_report.yml.github/ISSUE_TEMPLATE/config.yml.github/ISSUE_TEMPLATE/feature_request.yml.github/pull_request_template.mdCONTRIBUTING.mdbin/cli.jspackage.jsonsrc/commands/doc-init.jssrc/commands/scan.jssrc/lib/graph-builder.jssrc/lib/scanner.jssrc/prompts/discover-architecture.mdsrc/prompts/discover-domains.mdtests/graph-builder.test.jstests/scanner.test.js
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/graph-builder.test.js (2)
13-37: Use a unique temp fixture root to avoid parallel-test collisions.A shared fixed directory can cause flaky cleanup/race behavior across workers/files. Prefer per-run temp dirs.
Proposed refactor
-import { mkdirSync, writeFileSync, rmSync, existsSync } from 'fs'; +import { mkdirSync, writeFileSync, rmSync, existsSync, mkdtempSync } from 'fs'; import { join } from 'path'; +import { tmpdir } from 'os'; @@ -const FIXTURES_DIR = join(import.meta.dirname, 'fixtures', 'graph-builder'); +const FIXTURES_DIR = mkdtempSync(join(tmpdir(), 'aspens-graph-builder-'));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/graph-builder.test.js` around lines 13 - 37, Replace the shared FIXTURES_DIR with a unique per-run temp directory: create a temp root using os/tmp mkdtemp (or fs.mkdtempSync) at test startup (referencing beforeAll and init) and use that temp root in createFixture instead of the fixed FIXTURES_DIR constant; ensure createFixture still creates directories and files under the temp root, and update afterAll cleanup to rmSync that temp directory (and handle the existing try/catch) so parallel test workers won't collide with each other's fixture directories.
111-118: Strengthen therequire()test assertion (current check is too weak).
expect(result).toBeDefined()passes even if behavior regresses. Assert the returned shape and rename the case to match the actual contract.Proposed refactor
- it('handles require() calls as imports', () => { + it('does not crash on require() calls', () => { const code = `const foo = require('./bar');`; const result = parseJsImports(code, 'src/app.js'); - // es-module-lexer may or may not pick up require() — it focuses on ESM. - // If it does, great; if not, this is expected behavior. - // The implementation relies on es-module-lexer which only parses ESM syntax. - expect(result).toBeDefined(); + expect(result).toMatchObject({ + imports: expect.any(Array), + exports: expect.any(Array), + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/graph-builder.test.js` around lines 111 - 118, Rename the test to something like "handles require() calls (may produce no ESM imports)" and strengthen assertions around parseJsImports: call parseJsImports(code, 'src/app.js') and assert Array.isArray(result) (or assert the returned object's imports array if parseJsImports returns an object), then assert that if result.length > 0 each item has the expected shape (e.g., item.specifier is a string and item.start/item.end are numbers) so the test fails on regressions; use parseJsImports and item property names (specifier, start, end) as the unique identifiers to locate and validate the output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/graph-builder.test.js`:
- Around line 13-37: Replace the shared FIXTURES_DIR with a unique per-run temp
directory: create a temp root using os/tmp mkdtemp (or fs.mkdtempSync) at test
startup (referencing beforeAll and init) and use that temp root in createFixture
instead of the fixed FIXTURES_DIR constant; ensure createFixture still creates
directories and files under the temp root, and update afterAll cleanup to rmSync
that temp directory (and handle the existing try/catch) so parallel test workers
won't collide with each other's fixture directories.
- Around line 111-118: Rename the test to something like "handles require()
calls (may produce no ESM imports)" and strengthen assertions around
parseJsImports: call parseJsImports(code, 'src/app.js') and assert
Array.isArray(result) (or assert the returned object's imports array if
parseJsImports returns an object), then assert that if result.length > 0 each
item has the expected shape (e.g., item.specifier is a string and
item.start/item.end are numbers) so the test fails on regressions; use
parseJsImports and item property names (specifier, start, end) as the unique
identifiers to locate and validate the output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7936c293-edbb-4386-a8e1-1149f7fdc49d
📒 Files selected for processing (1)
tests/graph-builder.test.js
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 `@README.md`:
- Around line 83-124: The fenced code block in README.md that begins with ```
and contains the aspens doc init output is missing a language specifier
(triggering MD040); update the opening fence to include a language (for example
change ``` to ```text) so the block is explicitly marked as plain text while
keeping the existing closing ``` unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca262a3a-50f1-4af9-85b8-709220da29a8
📒 Files selected for processing (5)
CHANGELOG.mdREADME.mdbin/cli.jssrc/lib/graph-builder.jstests/graph-builder.test.js
✅ Files skipped from review due to trivial changes (2)
- CHANGELOG.md
- src/lib/graph-builder.js
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/graph-builder.test.js
This was referenced Mar 21, 2026
Merged
Merged
Merged
Merged
6 tasks
This was referenced Apr 7, 2026
Merged
6 tasks
This was referenced Aug 15, 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
Documentation
Chores
Tests