Repository navigation
Conversation
The run builds the call graph of each file in the workers, for the index. A cached plugin that reads the call graph, as the file plugin of vet, built it again in Analyze. The extra plugins run on the goroutine of the run, so that second build is serial work. On gitea it took 8.9 s of the 21 s that the plugin of vet spent on that goroutine, and the goroutine set the wall time of the run. The worker now keeps the call graph of the index in its outcome. A plugin that implements the new GraphPlugin gets it through AnalyzeGraph. A file with no call graph, as one whose index failed, still goes to Analyze. The index is complete before the plugin runs, so the plugin cannot change the index. The callgraph plugin gets AnalyzeGraph too, so a caller with a graph runs its callback and its partial check without a second build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm Signed-off-by: Claude <noreply@anthropic.com>
A review found three gaps in the shared call graph. Each outcome kept its call graph, also in a run with no GraphPlugin. The pool keeps two outcomes per worker in flight, so a run with no plugin held graphs that it never read. The pool now keeps a graph only when the run has a GraphPlugin. The GraphPlugin doc said that a file whose index failed goes to Analyze. A file that the deadline cut has a failed index and a partial graph, and it goes to AnalyzeGraph. The doc now says that a file whose call graph did not build goes to Analyze. It also says that a file that the workers finished before the deadline gets its full graph. The run test checked only the file name of the graph. It now compares the graph with the one that callgraph.Build gives for the tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm Signed-off-by: Claude <noreply@anthropic.com>
SafeDep Report Summary
No dependency changes detected. Nothing to scan. This report is generated by SafeDep GitHub App |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Graph plugin contract and callback behavior plugin/indexer/cache.go, plugin/callgraph/plugin.go, plugin/callgraph/context_test.go, docs/projectgraph.md |
GraphPlugin adds AnalyzeGraph with a call-graph argument. The call-graph plugin runs its callback and reports an error when the graph is partial. The test covers full and partial graphs. |
Build and retain per-file call graphs plugin/indexer/project.go, plugin/indexer/plugin.go, plugin/indexer/indexer_pool.go, plugin/indexer/gounit.go |
project returns the call graph with the index. fileIndexPlugin stores the graph, and pool workers retain it when requested. skeletonIndex handles the additional return value from project. |
Select and dispatch graph-aware analysis plugin/indexer/indexer.go, plugin/indexer/cache_test.go |
Run enables graph retention when a configured plugin implements GraphPlugin. Cached analysis calls AnalyzeGraph when a graph is available and otherwise calls Analyze. Tests cover graph delivery, graph retention, and warm-run artifact restoration. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Refactor
Sequence Diagram(s)
sequenceDiagram
participant IndexerRun
participant IndexerPool
participant Project
participant AnalyzeCached
participant GraphPlugin
IndexerRun->>IndexerPool: Enable graph retention when GraphPlugin is configured
IndexerPool->>Project: Build file index and call graph
Project-->>IndexerPool: Return index and call graph
IndexerPool->>AnalyzeCached: Pass outcome with retained graph
AnalyzeCached->>GraphPlugin: Call AnalyzeGraph with tree and graph
Merge Risk: 
f314b
-
No identified issue prevents merging after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title names two changes: sharing the index's call graph with graph plugins (the main change, via the new GraphPlugin interface and AnalyzeGraph) and logging no line per call. The changeset centers… Full details: Title check
-
Explanation
The title names two changes: sharing the index's call graph with graph plugins (the main change, via the new GraphPlugin interface and AnalyzeGraph) and logging no line per call. The changeset centers on passing the call graph to cached plugins, so the title is related to the main change. The 'log no line per call' part is not evident in the provided summaries, which is a minor mismatch but does not make the title off-topic. The title follows the conventional commit style and stays specific.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 9 functions across 9 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
==========================================
- Coverage 77.25% 77.24% -0.01%
==========================================
Files 174 174
Lines 17140 17142 +2
==========================================
Hits 13241 13241
- Misses 3115 3116 +1
- Partials 784 785 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The call graph build wrote a debug line for each call, each definition and each resolved call or constructor. A debug index run of gitea wrote 185,860 lines, and 160,836 of them were "Go call". The log took 2.3 s of a 15.8 s run. The lines named what the graph holds, so they did not help to find a fault. dry/log has no trace level, so the build no longer writes them. It keeps the line for each file and the lines that report a problem, as a node with no name or a limit that the build hit. The debug run of gitea writes 5,566 lines and takes 13.8 s. A new test builds a file with one call and with 50 calls in Go, JavaScript, Python and Java, and expects the same number of log lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm Signed-off-by: Claude <noreply@anthropic.com>
abhisek
changed the title
safedep/code#58 now also stops the call graph build from writing a debug line for each call, definition and resolved call. A scan with --debug wrote 160,836 "Go call" lines for gitea. The index of gitea now writes 5,566 debug lines in place of 185,860. This pins the new head of the branch of #58. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm
What was wrong
The indexer builds the call graph of each file in its workers, for the index. vet's file plugin is a
CachedPluginthat reads the call graph, so it built the graph again inAnalyze. The indexer runs the extra plugins on the goroutine of the run, so the second build was serial work.A CPU profile of a cold vet code analysis of gitea, on 4 cores:
collect.callgraph.Build.The call graph build also wrote a debug line for each call, each definition and each resolved call. A debug index run of gitea wrote 185,860 lines, and 160,836 of them were
Go call.What this changes
indexer.GraphPluginextendsCachedPluginwithAnalyzeGraph(ctx, file, tree, cg). The run gives that graph to aGraphPluginin place of a call toAnalyze.Analyze.GraphPlugin. A run with no such plugin drops each graph after the index, as before.AnalyzeGraph(ctx, cg). It runs the callback and the check for a partial graph without a second build.docs/projectgraph.mddescribesGraphPlugin.Under a deadline, this changes one case. A file that the workers finished before the deadline passed now gets its full graph. Before,
Analyzeran with a done context, so it built no graph and the file failed. The output is more complete, and the index of the file does not change.Measurements
vet code analysis, cold, no cache.db, 4 cores. vet runs with its file plugin as a
GraphPlugin. The analysis output is byte-identical before and after on each project.Tests
TestIndexerGivesAGraphPluginTheCallGraphOfTheIndex:GraphPlugingets the call graph of each file, equal to whatcallgraph.Buildgives for its tree.Analyze.Analyze.TestAnalyzeGraphFailsOnAPartialGraphAsAnalyzeTreeDoes:context.DeadlineExceeded.TestAnalyzeKeepsTheCallGraphOnlyForAGraphPlugin: the outcome keeps the graph only when the run asks for it.TestBuildLogsNoLineForEachCall: a file with 50 calls writes as many log lines as a file with one call, in Go, JavaScript, Python and Java.go test ./...andgolangci-lintpass.go test -race ./plugin/indexer/passes.Known gap, not in this PR
A review found a gap that this PR did not cause.
skeletonIndexingounit.godoes not check the deadline after it projects the skeleton of a generated Go table. A deadline during that build can store a partial skeleton index as complete. FOU-595 tracks it.Part of FOU-594.
🤖 Generated with Claude Code
https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm