Merged
Conversation
WalkthroughAdds a repo import-graph feature: new Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as "CLI (doc graph)"
participant Scan as "scanRepo()"
participant Build as "buildRepoGraph()"
participant Persist as "persistGraphArtifacts()"
participant Store as ".claude/* (graph.json, graph-index.json, code-map)"
participant Hook as "graph-context-prompt Hook"
participant Claude as "Claude Prompt Context"
User->>CLI: run `doc graph [path]`
CLI->>Scan: scanRepo(repoPath)
Scan-->>CLI: scan results
CLI->>Build: buildRepoGraph(repoPath, scan.languages)
Build-->>CLI: raw import graph
CLI->>Persist: persistGraphArtifacts(repoPath, rawGraph)
Persist->>Store: serialize & write graph.json, graph-index.json, code-map
CLI-->>User: print graph statistics
User->>Hook: UserPromptSubmit (prompt)
Hook->>Store: load graph-index.json
Hook->>Store: load graph.json (if matches)
Hook->>Hook: build neighborhood -> formatNavigationContext()
Hook-->>Claude: inject navigation markdown into prompt context
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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 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: 3
🧹 Nitpick comments (1)
src/lib/graph-persistence.js (1)
184-267: Note: Logic duplicated ingraph-context-prompt.mjs.The subgraph extraction logic here mirrors
buildNeighborhoodingraph-context-prompt.mjs. This is intentional—the.mjshook must be standalone (no aspens imports). Consider adding a comment noting this relationship to help future maintainers keep them in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/graph-persistence.js` around lines 184 - 267, The extractSubgraph function duplicates the neighborhood-building logic found in buildNeighborhood in graph-context-prompt.mjs; add a concise comment at the top of extractSubgraph explaining that this logic intentionally mirrors buildNeighborhood (so the .mjs hook can remain standalone without aspens imports), include the file and function name of the counterpart (graph-context-prompt.mjs::buildNeighborhood) and a short note to keep both implementations in sync when updating behavior; keep the comment minimal and placed immediately above the extractSubgraph declaration.
🤖 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/commands/doc-graph.js`:
- Around line 28-30: persistGraphArtifacts(repoPath, repoGraph) lacks error
handling so failures can still show "Graph saved"; wrap the call in a try/catch
around persistGraphArtifacts, stop or fail the spinner appropriately (use
spinner.stop/spinner.fail) inside both success and catch branches, log the
caught error (with context message mentioning persistGraphArtifacts/repoPath)
via the existing logger or console.error, and ensure the process exits or
returns a non-zero code on failure so the user doesn't see a misleading success
message.
In `@src/templates/hooks/graph-context-prompt.sh`:
- Line 27: The current single-line declaration and assignment local dir="$(cd -P
"$(dirname "$source")" && pwd)" masks the exit status of cd -P; instead, declare
the variable separately (local dir), then perform the command substitution into
dir using cd -P "$(dirname "$source")" && pwd and check/propagate the command's
exit status (or handle failure) so a failing cd doesn't get ignored; reference
variables/commands: dir, source, cd -P, dirname to locate the code to change.
In `@tests/graph-persistence.test.js`:
- Line 18: The test uses import.meta.dirname which is only supported in Node
21+, so replace that usage when constructing FIXTURES_DIR (the join(...) call
referencing import.meta.dirname) with a portable ESM dirname helper: derive
__filename via fileURLToPath(import.meta.url) and then __dirname via
path.dirname(__filename), then use join(__dirname, 'fixtures',
'graph-persistence') to set FIXTURES_DIR; update any imports to ensure
fileURLToPath and dirname come from the appropriate modules.
---
Nitpick comments:
In `@src/lib/graph-persistence.js`:
- Around line 184-267: The extractSubgraph function duplicates the
neighborhood-building logic found in buildNeighborhood in
graph-context-prompt.mjs; add a concise comment at the top of extractSubgraph
explaining that this logic intentionally mirrors buildNeighborhood (so the .mjs
hook can remain standalone without aspens imports), include the file and
function name of the counterpart (graph-context-prompt.mjs::buildNeighborhood)
and a short note to keep both implementations in sync when updating behavior;
keep the comment minimal and placed immediately above the extractSubgraph
declaration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 53205ab9-744e-4f9e-b4ef-46c40dc08165
📒 Files selected for processing (9)
bin/cli.jssrc/commands/doc-graph.jssrc/commands/doc-init.jssrc/commands/doc-sync.jssrc/lib/graph-persistence.jssrc/templates/hooks/graph-context-prompt.mjssrc/templates/hooks/graph-context-prompt.shsrc/templates/settings/settings.jsontests/graph-persistence.test.js
| get_script_dir() { | ||
| local source="${BASH_SOURCE[0]}" | ||
| while [ -h "$source" ]; do | ||
| local dir="$(cd -P "$(dirname "$source")" && pwd)" |
There was a problem hiding this comment.
Declare and assign separately to avoid masking return values.
If cd -P fails, the exit code is lost because local always returns 0.
Proposed fix
- local dir="$(cd -P "$(dirname "$source")" && pwd)"
+ local dir
+ dir="$(cd -P "$(dirname "$source")" && pwd)"📝 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
| local dir="$(cd -P "$(dirname "$source")" && pwd)" | |
| local dir | |
| dir="$(cd -P "$(dirname "$source")" && pwd)" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 27-27: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/templates/hooks/graph-context-prompt.sh` at line 27, The current
single-line declaration and assignment local dir="$(cd -P "$(dirname "$source")"
&& pwd)" masks the exit status of cd -P; instead, declare the variable
separately (local dir), then perform the command substitution into dir using cd
-P "$(dirname "$source")" && pwd and check/propagate the command's exit status
(or handle failure) so a failing cd doesn't get ignored; reference
variables/commands: dir, source, cd -P, dirname to locate the code to change.
| persistGraphArtifacts, | ||
| } from '../src/lib/graph-persistence.js'; | ||
|
|
||
| const FIXTURES_DIR = join(import.meta.dirname, 'fixtures', 'graph-persistence'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking Node engine constraints from package.json files..."
while read -r f; do
python - "$f" <<'PY'
import json, sys
f = sys.argv[1]
with open(f, "r", encoding="utf-8") as fh:
data = json.load(fh)
print(f"== {f} ==")
print(data.get("engines", {}).get("node", "no engines.node"))
PY
done < <(fd -a '^package\.json$' -E node_modules -E .git)
echo
echo "Checking for import.meta.dirname usage..."
rg -n --type=js --type=mjs -C2 'import\.meta\.dirname'
echo
echo "Expected verification result:"
echo "- If supported Node versions include < 20.11, replace import.meta.dirname with fileURLToPath(import.meta.url)+dirname."Repository: aspenkit/aspens
Length of output: 193
🏁 Script executed:
#!/bin/bash
# Check Node engine constraints
echo "=== Checking Node engine constraints ==="
find . -name "package.json" -not -path "*/node_modules/*" -not -path "*/.git/*" | head -20 | while read f; do
echo "File: $f"
cat "$f" | jq -r '.engines.node // "no engines.node"' 2>/dev/null || echo "Error reading $f"
done
echo
echo "=== Checking import.meta.dirname usage ==="
rg -n 'import\.meta\.dirname' --type js --type mjs
echo
echo "=== Checking Node version in learnings context ==="
echo "Learning states: Node >= 18 as minimum supported version"
echo "import.meta.dirname available from: Node 21.2.0"Repository: aspenkit/aspens
Length of output: 371
🏁 Script executed:
head -30 tests/graph-persistence.test.jsRepository: aspenkit/aspens
Length of output: 839
Use a portable ESM dirname helper instead of import.meta.dirname.
import.meta.dirname is only available in Node 21.2.0+, but the package.json engine constraint is >=20, which includes Node 20.x where this will fail. Use fileURLToPath(import.meta.url) + dirname(...) instead for compatibility with all supported Node versions.
Proposed fix
import { mkdirSync, writeFileSync, readFileSync, rmSync, existsSync } from 'fs';
-import { join } from 'path';
+import { join, dirname } from 'path';
+import { fileURLToPath } from 'url';
import {
serializeGraph,
saveGraph,
loadGraph,
extractFileReferences,
extractSubgraph,
formatNavigationContext,
generateCodeMapSkill,
writeCodeMapSkill,
generateGraphIndex,
saveGraphIndex,
persistGraphArtifacts,
} from '../src/lib/graph-persistence.js';
-const FIXTURES_DIR = join(import.meta.dirname, 'fixtures', 'graph-persistence');
+const FIXTURES_DIR = join(dirname(fileURLToPath(import.meta.url)), 'fixtures', 'graph-persistence');📝 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 FIXTURES_DIR = join(import.meta.dirname, 'fixtures', 'graph-persistence'); | |
| import { mkdirSync, writeFileSync, readFileSync, rmSync, existsSync } from 'fs'; | |
| import { join, dirname } from 'path'; | |
| import { fileURLToPath } from 'url'; | |
| import { | |
| serializeGraph, | |
| saveGraph, | |
| loadGraph, | |
| extractFileReferences, | |
| extractSubgraph, | |
| formatNavigationContext, | |
| generateCodeMapSkill, | |
| writeCodeMapSkill, | |
| generateGraphIndex, | |
| saveGraphIndex, | |
| persistGraphArtifacts, | |
| } from '../src/lib/graph-persistence.js'; | |
| const FIXTURES_DIR = join(dirname(fileURLToPath(import.meta.url)), 'fixtures', 'graph-persistence'); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/graph-persistence.test.js` at line 18, The test uses
import.meta.dirname which is only supported in Node 21+, so replace that usage
when constructing FIXTURES_DIR (the join(...) call referencing
import.meta.dirname) with a portable ESM dirname helper: derive __filename via
fileURLToPath(import.meta.url) and then __dirname via path.dirname(__filename),
then use join(__dirname, 'fixtures', 'graph-persistence') to set FIXTURES_DIR;
update any imports to ensure fileURLToPath and dirname come from the appropriate
modules.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/graph-persistence.js (1)
17-21: NormalizerepoPathonce before using it in FS/process calls.
repoPathis consumed directly incwdand write/read paths. Resolving once up front avoids relative-path ambiguity and makes path handling consistent.Suggested fix
-import { join } from 'path'; +import { join, resolve } from 'path'; + +function resolveRepoPath(repoPath) { + return resolve(repoPath); +}export function saveGraph(repoPath, serializedGraph) { - const graphDir = join(repoPath, '.claude'); + const root = resolveRepoPath(repoPath); + const graphDir = join(root, '.claude'); mkdirSync(graphDir, { recursive: true }); writeFileSync( - join(repoPath, GRAPH_PATH), + join(root, GRAPH_PATH), JSON.stringify(serializedGraph, null, 2) + '\n', ); }export function loadGraph(repoPath) { - const fullPath = join(repoPath, GRAPH_PATH); + const root = resolveRepoPath(repoPath); + const fullPath = join(root, GRAPH_PATH);export function writeCodeMap(repoPath, serializedGraph) { + const root = resolveRepoPath(repoPath); const content = generateCodeMap(serializedGraph); - const dir = join(repoPath, '.claude'); + const dir = join(root, '.claude'); mkdirSync(dir, { recursive: true }); - writeFileSync(join(repoPath, CODE_MAP_PATH), content); + writeFileSync(join(root, CODE_MAP_PATH), content); }export function saveGraphIndex(repoPath, index) { - const dir = join(repoPath, '.claude'); + const root = resolveRepoPath(repoPath); + const dir = join(root, '.claude'); mkdirSync(dir, { recursive: true }); writeFileSync( - join(repoPath, INDEX_PATH), + join(root, INDEX_PATH), JSON.stringify(index) + '\n', // compact — no pretty-print for speed ); }As per coding guidelines: "Always resolve file paths to absolute paths using
path.resolve()orpath.join(). ... Never pass raw user-provided paths without resolving first."Also applies to: 85-90, 98-99, 417-419, 464-468
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/graph-persistence.js` around lines 17 - 21, The code uses repoPath directly when calling execSync and file system operations (e.g., the gitHash assignment using execSync with cwd: repoPath and other read/write calls around lines referenced), so resolve repoPath to an absolute path once up front (e.g., const resolvedRepoPath = path.resolve(repoPath)) and replace all raw repoPath usages (cwd in execSync, fs.readFile/fs.writeFile paths, and any other process/FS calls such as those around the referenced regions) with the resolvedRepoPath variable to ensure consistent, absolute path handling.
🤖 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/lib/graph-persistence.js`:
- Around line 437-452: generateGraphIndex currently uses plain objects `exports`
and `hubBasenames` which silently overwrite earlier entries when export names or
hub basenames duplicate; change both to map each key to an array of paths
instead of a single string. Specifically, for `exports` (constructed from
`serializedGraph.files` and the `info.exports` loop) push `path` onto an array
keyed by `exp` (creating the array if missing) rather than assigning
`exports[exp] = path`, and for `hubBasenames` (iterating `serializedGraph.hubs`)
push `h.path` onto an array keyed by `basename` instead of overwriting; keep
deterministic ordering by appending in encountered order and adjust any
downstream lookup logic in `generateGraphIndex` to handle arrays (e.g., choose
first match or disambiguate) so no silent overwrites occur.
- Around line 97-104: The loadGraph function currently returns any parsed JSON
which can be structurally invalid; update loadGraph (the function that reads
GRAPH_PATH) to validate the parsed object has the expected shape (e.g., is an
object and contains the required top-level keys like files, clusters, hubs with
the expected types such as arrays or objects) before returning it, and if
validation fails return null (or handle consistently with existing error
behavior); locate the JSON.parse result in loadGraph and add explicit checks for
the presence and types of files/clusters/hubs (and any other required
properties) to prevent downstream runtime errors.
---
Nitpick comments:
In `@src/lib/graph-persistence.js`:
- Around line 17-21: The code uses repoPath directly when calling execSync and
file system operations (e.g., the gitHash assignment using execSync with cwd:
repoPath and other read/write calls around lines referenced), so resolve
repoPath to an absolute path once up front (e.g., const resolvedRepoPath =
path.resolve(repoPath)) and replace all raw repoPath usages (cwd in execSync,
fs.readFile/fs.writeFile paths, and any other process/FS calls such as those
around the referenced regions) with the resolvedRepoPath variable to ensure
consistent, absolute path handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3d903ebf-e176-4c5b-858f-90c3cf3e0c62
📒 Files selected for processing (3)
src/lib/graph-persistence.jssrc/templates/hooks/graph-context-prompt.mjstests/graph-persistence.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/templates/hooks/graph-context-prompt.mjs
- tests/graph-persistence.test.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/templates/hooks/graph-context-prompt.mjs (1)
374-375: Consider importingwriteFileSyncstatically.The dynamic import here adds unnecessary overhead when
writeFileSynccould simply be added to the existing static import on line 16.Proposed fix
-import { readFileSync, existsSync } from 'fs'; +import { readFileSync, existsSync, writeFileSync } from 'fs';Then at line 374-375:
- const { writeFileSync: wfs } = await import('fs'); - wfs('/tmp/aspens-debug-graph-context.json', JSON.stringify({ + writeFileSync('/tmp/aspens-debug-graph-context.json', JSON.stringify({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/templates/hooks/graph-context-prompt.mjs` around lines 374 - 375, The dynamic import that grabs writeFileSync (const { writeFileSync: wfs } = await import('fs')) introduces unnecessary runtime overhead; instead add writeFileSync to the static import at the top of graph-context-prompt.mjs (the same import that currently exists around line 16) and replace the dynamic import usage (the wfs variable) with a direct call to writeFileSync to write '/tmp/aspens-debug-graph-context.json'. Ensure you remove the dynamic import line and any temporary aliasing so all fs usage uses the statically imported writeFileSync symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/templates/hooks/graph-context-prompt.mjs`:
- Around line 374-375: The dynamic import that grabs writeFileSync (const {
writeFileSync: wfs } = await import('fs')) introduces unnecessary runtime
overhead; instead add writeFileSync to the static import at the top of
graph-context-prompt.mjs (the same import that currently exists around line 16)
and replace the dynamic import usage (the wfs variable) with a direct call to
writeFileSync to write '/tmp/aspens-debug-graph-context.json'. Ensure you remove
the dynamic import line and any temporary aliasing so all fs usage uses the
statically imported writeFileSync symbol.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87b90817-d317-45a8-86c9-e6abdb97378c
📒 Files selected for processing (1)
src/templates/hooks/graph-context-prompt.mjs
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/graph-persistence.js (1)
148-156: Consider adding common module extensions like.mjs,.cjs,.vue.The bare filename regex covers common extensions but misses some increasingly popular ones.
Suggested fix
- const bareRe = /\b([\w.-]+\.(js|ts|tsx|jsx|py|go|rs|rb))\b/g; + const bareRe = /\b([\w.-]+\.(js|mjs|cjs|ts|tsx|jsx|vue|py|go|rs|rb))\b/g;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/graph-persistence.js` around lines 148 - 156, The bare filename regex (bareRe) in graph-persistence.js misses newer module/file types; update the pattern used where bareRe is defined (used with prompt, filename, graphFiles, matches) to include .mjs, .cjs and .vue (e.g. extend the alternation list of extensions) so filenames like example.mjs, example.cjs and example.vue are captured and matched against graphFiles; keep the existing word-boundary and global flags and ensure the rest of the loop logic (getting filename and adding gf to matches) remains 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 `@src/lib/graph-persistence.js`:
- Around line 415-416: The current code assumes serializedGraph.meta exists and
directly accesses serializedGraph.meta.totalFiles, totalEdges, and
meta.generatedAt.split(...), which throws if meta is missing; update the block
that constructs lines (using the serializedGraph object) to defensively check
for serializedGraph.meta (e.g., use optional chaining and default values) and
guard the generatedAt split (provide a fallback date string or omit splitting)
so the push call becomes resilient when meta is undefined—ensure you reference
serializedGraph.meta.totalFiles, serializedGraph.meta.totalEdges, and
serializedGraph.meta.generatedAt when adding the defaults.
- Around line 57-80: The code assumes rawGraph.stats and rawGraph.hubs exist and
will throw if they are undefined; update the return construction (the block that
uses GRAPH_VERSION, rawGraph.stats, rawGraph.hubs, clusters, clusterIndex) to
use defensive defaults: treat rawGraph.stats as an object with totalFiles: 0 and
totalEdges: 0 when missing, and treat rawGraph.hubs as an empty array before
mapping (e.g., map over rawGraph.hubs || []), and ensure coupling and hotspots
also fallback (rawGraph.clusters?.coupling || [] and rawGraph.hotspots || []).
Apply these safe-access patterns where totalFiles, totalEdges, and the hubs
mapping are referenced to prevent runtime errors.
---
Nitpick comments:
In `@src/lib/graph-persistence.js`:
- Around line 148-156: The bare filename regex (bareRe) in graph-persistence.js
misses newer module/file types; update the pattern used where bareRe is defined
(used with prompt, filename, graphFiles, matches) to include .mjs, .cjs and .vue
(e.g. extend the alternation list of extensions) so filenames like example.mjs,
example.cjs and example.vue are captured and matched against graphFiles; keep
the existing word-boundary and global flags and ensure the rest of the loop
logic (getting filename and adding gf to matches) remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a8d3b6e-a920-4b3e-b011-e94f2cde4da1
📒 Files selected for processing (4)
src/commands/doc-graph.jssrc/lib/graph-persistence.jssrc/templates/hooks/graph-context-prompt.mjstests/graph-persistence.test.js
✅ Files skipped from review due to trivial changes (1)
- tests/graph-persistence.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/commands/doc-graph.js
- src/templates/hooks/graph-context-prompt.mjs
Comment on lines
+57
to
+80
| return { | ||
| version: GRAPH_VERSION, | ||
| meta: { | ||
| generatedAt: new Date().toISOString(), | ||
| gitHash, | ||
| totalFiles: rawGraph.stats.totalFiles, | ||
| totalEdges: rawGraph.stats.totalEdges, | ||
| }, | ||
| files, | ||
| hubs: rawGraph.hubs.map(h => ({ | ||
| path: h.path, | ||
| fanIn: h.fanIn, | ||
| exports: h.exports, | ||
| })), | ||
| clusters: clusters.map(c => ({ | ||
| label: c.label, | ||
| size: c.size, | ||
| files: c.files, | ||
| })), | ||
| coupling: rawGraph.clusters?.coupling || [], | ||
| hotspots: rawGraph.hotspots, | ||
| clusterIndex, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Add defensive checks for rawGraph.stats and rawGraph.hubs.
If rawGraph.stats or rawGraph.hubs is undefined, accessing properties or calling .map() will throw.
Suggested fix
return {
version: GRAPH_VERSION,
meta: {
generatedAt: new Date().toISOString(),
gitHash,
- totalFiles: rawGraph.stats.totalFiles,
- totalEdges: rawGraph.stats.totalEdges,
+ totalFiles: rawGraph.stats?.totalFiles ?? 0,
+ totalEdges: rawGraph.stats?.totalEdges ?? 0,
},
files,
- hubs: rawGraph.hubs.map(h => ({
+ hubs: (rawGraph.hubs || []).map(h => ({
path: h.path,
fanIn: h.fanIn,
exports: h.exports,
})),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/graph-persistence.js` around lines 57 - 80, The code assumes
rawGraph.stats and rawGraph.hubs exist and will throw if they are undefined;
update the return construction (the block that uses GRAPH_VERSION,
rawGraph.stats, rawGraph.hubs, clusters, clusterIndex) to use defensive
defaults: treat rawGraph.stats as an object with totalFiles: 0 and totalEdges: 0
when missing, and treat rawGraph.hubs as an empty array before mapping (e.g.,
map over rawGraph.hubs || []), and ensure coupling and hotspots also fallback
(rawGraph.clusters?.coupling || [] and rawGraph.hotspots || []). Apply these
safe-access patterns where totalFiles, totalEdges, and the hubs mapping are
referenced to prevent runtime errors.
Comment on lines
+415
to
+416
| lines.push(`*${serializedGraph.meta.totalFiles} files, ${serializedGraph.meta.totalEdges} edges — updated ${serializedGraph.meta.generatedAt.split('T')[0]}*`); | ||
| lines.push(''); |
There was a problem hiding this comment.
Add defensive check for serializedGraph.meta.
If called with a graph missing meta, this line throws a confusing error.
Suggested fix
- lines.push(`*${serializedGraph.meta.totalFiles} files, ${serializedGraph.meta.totalEdges} edges — updated ${serializedGraph.meta.generatedAt.split('T')[0]}*`);
+ const meta = serializedGraph.meta || {};
+ lines.push(`*${meta.totalFiles ?? '?'} files, ${meta.totalEdges ?? '?'} edges — updated ${(meta.generatedAt || '').split('T')[0] || 'unknown'}*`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/graph-persistence.js` around lines 415 - 416, The current code
assumes serializedGraph.meta exists and directly accesses
serializedGraph.meta.totalFiles, totalEdges, and meta.generatedAt.split(...),
which throws if meta is missing; update the block that constructs lines (using
the serializedGraph object) to defensively check for serializedGraph.meta (e.g.,
use optional chaining and default values) and guard the generatedAt split
(provide a fallback date string or omit splitting) so the push call becomes
resilient when meta is undefined—ensure you reference
serializedGraph.meta.totalFiles, serializedGraph.meta.totalEdges, and
serializedGraph.meta.generatedAt when adding the defaults.
This was referenced Mar 23, 2026
Merged
Merged
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
2 participants
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.
What
Persist the import graph to .claude/graph.json and deliver structural context to Claude via two layers: a static code-map skill (hubs, clusters, hotspots) loaded by the existing skill system, and a smart graph hook that uses a tiny pre-computed index to match export names, hub basenames, and file paths in the user's prompt — only loading the full graph when relevant.
Why
The graph-builder computes a rich import graph during doc init (hub files, domain clusters, priority rankings, hotspots) then throws it away. Claude has zero structural awareness at runtime — it doesn't know which files are hubs, how files relate, or what to read first. This wastes tokens on blind exploration. Now the graph persists across sessions, doc sync rebuilds it on every commit, and Claude gets navigation context when it matters.
Closes #
How I tested
npm testpasses--dry-runoutput looks correct (if applicable)Checklist
Summary by CodeRabbit
New Features
Tests