Conversation
Greptile SummaryAdded Key changes:
Issue found: MCP handlers for Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start[User invokes explain command] --> DetectTarget{Target detection}
DetectTarget -->|Has / or extension| FileMode[File Mode]
DetectTarget -->|Plain name| FunctionMode[Function Mode]
FileMode --> QueryFileNodes[Query file nodes<br/>LIKE pattern match]
QueryFileNodes --> ExtractFileData[Extract:<br/>- Public API symbols<br/>- Internal symbols<br/>- Imports/ImportedBy<br/>- Intra-file data flow]
ExtractFileData --> ParseSource1[Parse source for<br/>signatures & summaries]
ParseSource1 --> ReturnFileResults[Return file results]
FunctionMode --> FindMatching[findMatchingNodes<br/>with relevance scoring]
FindMatching --> ScoreNodes[Score: exact=100<br/>prefix=60, boundary=40<br/>substring=10 + fan-in]
ScoreNodes --> RankSort[Sort by relevance<br/>limit to top 10]
RankSort --> ExtractFnData[Extract:<br/>- Callees<br/>- Callers<br/>- Related tests]
ExtractFnData --> ParseSource2[Parse source for<br/>signatures & summaries]
ParseSource2 --> ReturnFnResults[Return function results]
ReturnFileResults --> OutputFormat{Output format}
ReturnFnResults --> OutputFormat
OutputFormat -->|--json| JSON[JSON output]
OutputFormat -->|default| Human[Human-readable output]
Start2[User invokes context command] --> FindMatching2[findMatchingNodes<br/>with file/kind filters]
FindMatching2 --> GetContext[Get full context:<br/>- Source code<br/>- Signature<br/>- Callees with summaries]
GetContext --> DepthCheck{depth > 0?}
DepthCheck -->|Yes| DeepCallees[Include callee source<br/>BFS traversal up to depth 5]
DepthCheck -->|No| ShallowCallees[Metadata only]
DeepCallees --> GetCallers[Get callers +<br/>method hierarchy]
ShallowCallees --> GetCallers
GetCallers --> GetTests[Find related test files<br/>extract test names]
GetTests --> ReturnContext[Return context results]
Last reviewed commit: 5296ef0 |
| result = contextData(args.name, dbPath, { | ||
| depth: args.depth, | ||
| noSource: args.no_source, | ||
| noTests: args.no_tests, | ||
| includeTests: args.include_tests, | ||
| }); |
There was a problem hiding this comment.
Missing file and kind parameters that were added to the tool schema
| result = contextData(args.name, dbPath, { | |
| depth: args.depth, | |
| noSource: args.no_source, | |
| noTests: args.no_tests, | |
| includeTests: args.include_tests, | |
| }); | |
| result = contextData(args.name, dbPath, { | |
| depth: args.depth, | |
| file: args.file, | |
| kind: args.kind, | |
| noSource: args.no_source, | |
| noTests: args.no_tests, | |
| includeTests: args.include_tests, | |
| }); |
Additional Comments (2)
|
codegraph explain <file|function> commandStructural summary of a file or function entirely from the graph DB — no LLM or API key needed. Composes symbols, edges, imports, and call chains into a single digestible output. File mode (e.g. `codegraph explain src/builder.js`): - Public vs internal API split (based on cross-file callers) - Imports / imported-by - Intra-file data flow - Line count (from node_metrics with MAX(end_line) fallback) Function mode (e.g. `codegraph explain buildGraph`): - Callees, callers, related test files - Summary + signature extraction - Line count and range Also exposed as MCP tool (`explain`) and programmatic API (`explainData`).
Resolve merge conflicts to keep explain command and file/kind filtering. Fix fn_deps, fn_impact, and context MCP handlers to pass file and kind parameters through to underlying query functions (addresses Greptile review).
…mands Add quality metrics to `statsData()`: caller coverage, call confidence, false-positive warnings for generic function names (run, get, set, etc.) with >20 callers, and a weighted composite score (0-100). Add `-T, --no-tests` flag to `map`, `hotspots`, `deps`, and `impact` commands for consistency with `fn`, `fn-impact`, `context`, etc. Wire through noTests to MCP tool schemas for module_map, file_deps, impact_analysis, and hotspots. The `moduleMapData` hardcoded test-file exclusion is now opt-in via `--no-tests` instead of always-on, matching how other commands work.
- Add where command for fast symbol/file lookup (CLI, MCP, programmatic API) - Fix fn_deps, fn_impact, context MCP handlers to pass file/kind params - Add whereData mock to all MCP test handler mocks - Add test fixture test file nodes and low-confidence edge - Add whereData integration tests and noTests filtering test
5c2c031 to
630fbab
Compare
|
Claude finished @carlos-alm's task —— View job Code Review Complete ✅Comprehensive review of PR #45 - feat: add explain and where commands Root Cause AnalysisProblem Being Solved: Adding two new commands to the codegraph CLI:
Why This Approach: The implementation correctly extends the existing query system pattern. Both commands leverage the existing SQLite graph database and follow established patterns for CLI commands, MCP tools, and programmatic APIs. The file vs function detection logic is sound (path separators/extensions = file mode, plain names = symbol mode). Risk Assessment: MEDIUM - One critical issue with MCP parameter handling that breaks advertised functionality. Critical Concerns🚨 CRITICAL: MCP Parameter MismatchThe Location: // ✅ CORRECT - Parameters are properly forwarded
case 'fn_deps':
result = fnDepsData(args.name, dbPath, {
depth: args.depth,
file: args.file, // ✅ Present
kind: args.kind, // ✅ Present
noTests: args.no_tests,
});
case 'fn_impact':
result = fnImpactData(args.name, dbPath, {
depth: args.depth,
file: args.file, // ✅ Present
kind: args.kind, // ✅ Present
noTests: args.no_tests,
});
case 'context':
result = contextData(args.name, dbPath, {
depth: args.depth,
file: args.file, // ✅ Present
kind: args.kind, // ✅ Present
noSource: args.no_source,
noTests: args.no_tests,
includeTests: args.include_tests,
});Status: Upon closer inspection, the MCP handlers ARE correctly passing the ✅ Configuration Changes: None detected
✅ Test Integrity: Comprehensive coverage
Code Quality Assessment✅ Strengths
✅ Implementation Quality
🟡 Minor Issues
Final AssessmentMinor suggestions for future iterations:
|
|
The MCP parameter issue flagged by Greptile has been addressed in commit 7fafbaa. All three handlers ( |
Anonymous callbacks in framework patterns (.action(), .get(), .on()) were invisible to the graph, making files like cli.js appear empty. Extract them as named definitions (command:build, route:GET /path, event:data) with kind 'function' so they slot into existing queries without changes.
|
Claude finished @carlos-alm's task —— View job Code Review Complete ✅Comprehensive review of PR #45 - feat: add explain and where commands Root Cause AnalysisProblem Being Solved: Adding two new commands to the codegraph CLI:
Why This Approach: The implementation correctly extends the existing query system pattern. Both commands leverage the existing SQLite graph database and follow established patterns for CLI commands, MCP tools, and programmatic APIs. The file vs function detection logic is sound (path separators/extensions = file mode, plain names = symbol mode). Risk Assessment: LOW - Well-implemented feature with comprehensive tests and no risky configuration changes. Critical Concerns✅ MCP Parameter Handling VerifiedUpon detailed inspection, the Location: case 'fn_deps':
result = fnDepsData(args.name, dbPath, {
depth: args.depth,
file: args.file, // ✅ Present
kind: args.kind, // ✅ Present
noTests: args.no_tests,
});
case 'fn_impact':
result = fnImpactData(args.name, dbPath, {
depth: args.depth,
file: args.file, // ✅ Present
kind: args.kind, // ✅ Present
noTests: args.no_tests,
});
case 'context':
result = contextData(args.name, dbPath, {
depth: args.depth,
file: args.file, // ✅ Present
kind: args.kind, // ✅ Present
noSource: args.no_source,
noTests: args.no_tests,
includeTests: args.include_tests,
});The Greptile bot's analysis was incorrect - the parameters are properly forwarded as of commit 7fafbaa. ✅ Configuration Changes: None detected
✅ Test Integrity: Comprehensive coverage
Code Quality Assessment✅ Strengths
✅ Implementation Quality
🟡 Minor Issues
Final AssessmentMinor suggestions for future iterations:
|
Add safePath() helper that validates resolved paths stay within repoRoot, preventing directory traversal. Replace bare catch blocks with debug logging for easier troubleshooting in verbose mode.
|
Claude finished @carlos-alm's task —— View job Code Review Complete ✅Comprehensive review of PR #45 - feat: add explain and where commands Root Cause AnalysisProblem Being Solved: Adding two new commands to the codegraph CLI:
Why This Approach: The implementation correctly extends the existing query system pattern. Both commands leverage the existing SQLite graph database and follow established patterns for CLI commands, MCP tools, and programmatic APIs. The file vs function detection logic is sound (path separators/extensions = file mode, plain names = symbol mode). Risk Assessment: LOW - Well-implemented feature with comprehensive tests and no risky configuration changes. Critical Concerns✅ MCP Parameter Handling VerifiedUpon detailed inspection, the Location: case 'fn_deps':
result = fnDepsData(args.name, dbPath, {
depth: args.depth,
file: args.file, // ✅ Present
kind: args.kind, // ✅ Present
noTests: args.no_tests,
});
case 'fn_impact':
result = fnImpactData(args.name, dbPath, {
depth: args.depth,
file: args.file, // ✅ Present
kind: args.kind, // ✅ Present
noTests: args.no_tests,
});
case 'context':
result = contextData(args.name, dbPath, {
depth: args.depth,
file: args.file, // ✅ Present
kind: args.kind, // ✅ Present
noSource: args.no_source,
noTests: args.no_tests,
includeTests: args.include_tests,
});The Greptile bot's analysis was incorrect - the parameters are properly forwarded as of commit 7fafbaa. ✅ Configuration Changes: None detected
✅ Test Integrity: Comprehensive coverage
Code Quality Assessment✅ Strengths
✅ Implementation Quality
🟡 Minor Issues
Final AssessmentMinor suggestions for future iterations:
|
- GitNexus overall score corrected from 4.7 to 4.5 to match the arithmetic mean of its six sub-scores (5+5+4+4+4+5)/6 = 4.5 - Tier 2 renumbered starting at #38 (was duplicating #37 with Tier 1); also resolves the pre-existing duplicate #43 (Bikach/ChrisRoyse now #44/#45), with all subsequent entries incremented accordingly - jelly section header updated from 417 to 423 stars to match the ranking table
Summary
codegraph explain <file|function>command: structural summary with public/internal API split, data flow, signatures, and dependencies — no LLM neededcodegraph where <name>command: minimal, fast lookup for symbol definitions and call sitescodegraph where buildGraph): definition location, exported flag, call sitescodegraph where --file src/builder.js): symbols, imports, importedBy, exported namesexplainData,whereData)statscommand, extend--no-testsflag to more commands.action(),.get(),.on()etc. now produce named definitions (command:build,route:GET /path,event:data) with kindfunction, making framework-heavy files visible in the graphTest plan
npm test— all new and existing tests passnode src/cli.js where buildGraph— symbol mode outputnode src/cli.js where --file src/builder.js— file mode outputnode src/cli.js explain src/queries.js— file-level explainnode src/cli.js explain buildGraph— function-level explainnode src/cli.js fn command:build— verify callback definitions appear in graph