Skip to content

Commit e982af7

Browse files
invalidclaude
andcommitted
fix: address all 11 code review issues
- Move _walkNodes/_findChildOfType/_stripQuotes to LanguageAdapter base class, removing duplication across 6 adapters - Add try/finally in parseFile to prevent WASM resource leaks on error - Accept optional sourceCode param in parseFile; scanner now reads each file once (eliminates double-read and TOCTOU race) - Build normalised path lookup Map for O(1) import resolution instead of O(n) linear scan per import - Detect C++ projects and reclassify .h files as cpp when .cpp/.cc/.cxx files are present in the same project - Rename misleading calls/calledBy fields to fileImports/importedBy in query results to accurately reflect semantics - Extract getGitCommitHash and isEntryPoint into graph.js, eliminating duplication between scan and update commands - Export buildModuleSlice from slicer.js; slice command now builds only the target module slice instead of generating all slices - Unify error handling: all commands use process.exit(1) for fatal errors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e2771c7 commit e982af7

17 files changed

Lines changed: 168 additions & 264 deletions

File tree

cli/src/commands/query.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ export function registerQueryCommand(program) {
5959
console.log(` File: ${result.file}`);
6060
console.log(` Module: ${result.module}`);
6161
console.log(` Lines: ${result.lines.start}-${result.lines.end}`);
62-
if (result.calledBy.length > 0) {
63-
console.log(` Called by: ${result.calledBy.join(', ')}`);
62+
if (result.importedBy.length > 0) {
63+
console.log(` Imported by: ${result.importedBy.join(', ')}`);
6464
}
6565
}
6666
});

cli/src/commands/scan.js

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,8 @@
11
import path from 'path';
22
import { scanProject } from '../scanner.js';
3-
import { saveGraph, computeFileHash } from '../graph.js';
3+
import { saveGraph, computeFileHash, getGitCommitHash } from '../graph.js';
44
import { saveSlices } from '../slicer.js';
55

6-
/**
7-
* Try to get the current git commit hash from the given directory.
8-
* Returns null if the directory is not a git repo or simple-git is unavailable.
9-
*
10-
* @param {string} dir - Directory to check for git.
11-
* @returns {Promise<string|null>} The current commit hash or null.
12-
*/
13-
async function getGitCommitHash(dir) {
14-
try {
15-
const { simpleGit } = await import('simple-git');
16-
const git = simpleGit(dir);
17-
const isRepo = await git.checkIsRepo();
18-
if (!isRepo) return null;
19-
const log = await git.log({ maxCount: 1 });
20-
return log.latest ? log.latest.hash : null;
21-
} catch {
22-
return null;
23-
}
24-
}
25-
266
/**
277
* Register the `scan` command on the given Commander program.
288
*

cli/src/commands/slice.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import path from 'path';
22
import { loadGraph } from '../graph.js';
3-
import { generateOverview, generateSlices, getModuleSliceWithDeps } from '../slicer.js';
3+
import { generateOverview, buildModuleSlice, getModuleSliceWithDeps } from '../slicer.js';
44

55
/**
66
* Register the `slice` command on the given Commander program.
@@ -40,13 +40,11 @@ export function registerSliceCommand(program) {
4040
}
4141

4242
if (options.withDeps) {
43-
// Include dependency info
4443
const sliceWithDeps = getModuleSliceWithDeps(graph, moduleName);
4544
console.log(JSON.stringify(sliceWithDeps, null, 2));
4645
} else {
47-
// Simple module slice
48-
const slices = generateSlices(graph);
49-
const slice = slices[moduleName];
46+
// Build only the target module's slice instead of all modules
47+
const slice = buildModuleSlice(graph, moduleName, graph.modules[moduleName]);
5048
console.log(JSON.stringify(slice, null, 2));
5149
}
5250
});

cli/src/commands/status.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ export function registerStatusCommand(program) {
1919
try {
2020
await fs.access(outputDir);
2121
} catch {
22-
console.log('No code graph found. Run "codegraph scan" first.');
23-
return;
22+
console.error('No code graph found. Run "codegraph scan" first.');
23+
process.exit(1);
2424
}
2525

2626
let graph, meta;
@@ -29,7 +29,7 @@ export function registerStatusCommand(program) {
2929
meta = await loadMeta(outputDir);
3030
} catch (err) {
3131
console.error(`Error loading code graph: ${err.message}`);
32-
return;
32+
process.exit(1);
3333
}
3434

3535
// Print status

cli/src/commands/update.js

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,12 @@
11
import path from 'path';
22
import fs from 'fs/promises';
3-
import { loadGraph, loadMeta, saveGraph, computeFileHash } from '../graph.js';
3+
import { loadGraph, loadMeta, saveGraph, computeFileHash, getGitCommitHash, isEntryPoint } from '../graph.js';
44
import { traverseFiles, detectLanguage } from '../traverser.js';
55
import { initParser, parseFile } from '../parser.js';
66
import { detectModuleName } from '../scanner.js';
77
import { detectChangedFiles, mergeGraphUpdate } from '../differ.js';
88
import { saveSlices } from '../slicer.js';
99

10-
/**
11-
* Try to get the current git commit hash from the given directory.
12-
* Returns null if the directory is not a git repo or simple-git is unavailable.
13-
*
14-
* @param {string} dir - Directory to check for git.
15-
* @returns {Promise<string|null>} The current commit hash or null.
16-
*/
17-
async function getGitCommitHash(dir) {
18-
try {
19-
const { simpleGit } = await import('simple-git');
20-
const git = simpleGit(dir);
21-
const isRepo = await git.checkIsRepo();
22-
if (!isRepo) return null;
23-
const log = await git.log({ maxCount: 1 });
24-
return log.latest ? log.latest.hash : null;
25-
} catch {
26-
return null;
27-
}
28-
}
29-
3010
/**
3111
* Register the `update` command on the given Commander program.
3212
*
@@ -68,7 +48,6 @@ export function registerUpdateCommand(program) {
6848
const content = await fs.readFile(absPath, 'utf-8');
6949
currentHashes[relPath] = computeFileHash(content);
7050
} catch {
71-
// Skip files that can't be read
7251
continue;
7352
}
7453
}
@@ -81,7 +60,7 @@ export function registerUpdateCommand(program) {
8160
return;
8261
}
8362

84-
// Step 5: Re-parse only changed/added files
63+
// Step 5: Re-parse only changed/added files (single read per file)
8564
const updatedFiles = {};
8665
const changedPaths = [...changes.added, ...changes.modified];
8766

@@ -90,18 +69,23 @@ export function registerUpdateCommand(program) {
9069
const language = detectLanguage(absPath);
9170
if (!language) continue;
9271

72+
let content;
73+
try {
74+
content = await fs.readFile(absPath, 'utf-8');
75+
} catch {
76+
continue;
77+
}
78+
79+
const hash = computeFileHash(content);
80+
9381
let parsed;
9482
try {
95-
parsed = await parseFile(absPath, language);
83+
parsed = await parseFile(absPath, language, content);
9684
} catch {
9785
continue;
9886
}
9987

10088
const moduleName = detectModuleName(absPath, rootDir);
101-
const content = await fs.readFile(absPath, 'utf-8');
102-
const hash = computeFileHash(content);
103-
const baseName = path.basename(absPath, path.extname(absPath)).toLowerCase();
104-
const isEntryPoint = ['main', 'index', 'server', 'app', 'entry', 'bootstrap'].includes(baseName);
10589

10690
updatedFiles[relPath] = {
10791
language,
@@ -113,7 +97,7 @@ export function registerUpdateCommand(program) {
11397
types: parsed.types,
11498
imports: parsed.imports,
11599
exports: parsed.exports,
116-
isEntryPoint,
100+
isEntryPoint: isEntryPoint(absPath),
117101
};
118102
}
119103

cli/src/graph.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import crypto from 'crypto';
22
import fs from 'fs/promises';
3+
import path from 'path';
34

45
export function createEmptyGraph(projectName, rootDir) {
56
return {
@@ -21,6 +22,34 @@ export function computeFileHash(content) {
2122
return 'sha256:' + crypto.createHash('sha256').update(content).digest('hex').slice(0, 16);
2223
}
2324

25+
/** File basenames (without extension) that mark an entry point. */
26+
const ENTRY_POINT_NAMES = new Set([
27+
'main', 'index', 'server', 'app', 'entry', 'bootstrap',
28+
]);
29+
30+
/** Check whether a file path represents an entry point. */
31+
export function isEntryPoint(filePath) {
32+
const baseName = path.basename(filePath, path.extname(filePath)).toLowerCase();
33+
return ENTRY_POINT_NAMES.has(baseName);
34+
}
35+
36+
/**
37+
* Try to get the current git commit hash from the given directory.
38+
* Returns null if the directory is not a git repo or simple-git is unavailable.
39+
*/
40+
export async function getGitCommitHash(dir) {
41+
try {
42+
const { simpleGit } = await import('simple-git');
43+
const git = simpleGit(dir);
44+
const isRepo = await git.checkIsRepo();
45+
if (!isRepo) return null;
46+
const log = await git.log({ maxCount: 1 });
47+
return log.latest ? log.latest.hash : null;
48+
} catch {
49+
return null;
50+
}
51+
}
52+
2453
export async function saveGraph(outputDir, graph, meta) {
2554
await fs.mkdir(outputDir, { recursive: true });
2655
await fs.writeFile(`${outputDir}/graph.json`, JSON.stringify(graph, null, 2), 'utf-8');

cli/src/languages/base.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,37 @@ export class LanguageAdapter {
6262
extractTypes(tree, sourceCode) {
6363
return [];
6464
}
65+
66+
// ---------------------------------------------------------------------------
67+
// Shared helpers
68+
// ---------------------------------------------------------------------------
69+
70+
/**
71+
* Walk all nodes depth-first, calling `visitor(node)` for each.
72+
* Uses an explicit stack (not the cursor API) for simplicity and reliability.
73+
*/
74+
_walkNodes(root, visitor) {
75+
const stack = [root];
76+
while (stack.length > 0) {
77+
const node = stack.pop();
78+
visitor(node);
79+
for (let i = node.childCount - 1; i >= 0; i--) {
80+
stack.push(node.child(i));
81+
}
82+
}
83+
}
84+
85+
/** Find the first direct child with the given type. */
86+
_findChildOfType(node, type) {
87+
for (let i = 0; i < node.childCount; i++) {
88+
const child = node.child(i);
89+
if (child.type === type) return child;
90+
}
91+
return null;
92+
}
93+
94+
/** Strip surrounding quotes from a string literal. */
95+
_stripQuotes(text) {
96+
return text.replace(/^['"`]|['"`]$/g, '');
97+
}
6598
}

cli/src/languages/cpp.js

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -261,27 +261,6 @@ export class CppAdapter extends LanguageAdapter {
261261
// Helpers
262262
// ---------------------------------------------------------------------------
263263

264-
/** Walk all nodes depth-first, calling visitor(node) for each. */
265-
_walkNodes(root, visitor) {
266-
const stack = [root];
267-
while (stack.length > 0) {
268-
const node = stack.pop();
269-
visitor(node);
270-
for (let i = node.childCount - 1; i >= 0; i--) {
271-
stack.push(node.child(i));
272-
}
273-
}
274-
}
275-
276-
/** Find the first direct child with the given type. */
277-
_findChildOfType(node, type) {
278-
for (let i = 0; i < node.childCount; i++) {
279-
const child = node.child(i);
280-
if (child.type === type) return child;
281-
}
282-
return null;
283-
}
284-
285264
/** Find the first descendant (BFS) with the given type. */
286265
_findDescendantOfType(node, type) {
287266
const queue = [];

cli/src/languages/go.js

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -249,27 +249,10 @@ export class GoAdapter extends LanguageAdapter {
249249
// Helpers
250250
// ---------------------------------------------------------------------------
251251

252-
/** Walk all nodes depth-first, calling visitor(node) for each. */
253-
_walkNodes(root, visitor) {
254-
const stack = [root];
255-
while (stack.length > 0) {
256-
const node = stack.pop();
257-
visitor(node);
258-
for (let i = node.childCount - 1; i >= 0; i--) {
259-
stack.push(node.child(i));
260-
}
261-
}
262-
}
263-
264252
/** Check whether a Go identifier is exported (starts with uppercase). */
265253
_isExported(name) {
266254
if (!name || name.length === 0) return false;
267255
const first = name.charCodeAt(0);
268256
return first >= 65 && first <= 90; // A-Z
269257
}
270-
271-
/** Strip surrounding quotes from a string literal. */
272-
_stripQuotes(text) {
273-
return text.replace(/^['"`]|['"`]$/g, '');
274-
}
275258
}

cli/src/languages/java.js

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -168,20 +168,6 @@ export class JavaAdapter extends LanguageAdapter {
168168
// Helpers
169169
// ---------------------------------------------------------------------------
170170

171-
/**
172-
* Walk all nodes depth-first, calling `visitor(node)` for each.
173-
*/
174-
_walkNodes(root, visitor) {
175-
const stack = [root];
176-
while (stack.length > 0) {
177-
const node = stack.pop();
178-
visitor(node);
179-
for (let i = node.childCount - 1; i >= 0; i--) {
180-
stack.push(node.child(i));
181-
}
182-
}
183-
}
184-
185171
/**
186172
* Find the name of the enclosing class/interface/enum declaration.
187173
*/

0 commit comments

Comments
 (0)