fix: eliminate double API call in analyze when file mode is on#63
Conversation
When file mode is enabled (default), `supermodel analyze` was making two separate API calls to the same endpoint: first via analyze.Run() (caching to ~/.supermodel/cache/), then via files.Generate() (caching to .supermodel/graph.json). Both uploaded the same zip to /v1/graphs/supermodel. Fix: when file mode is on, skip analyze.Run() and let files.Generate() handle the entire pipeline (upload + cache + sidecars) in a single API call. When --no-files is set, use the original analyze.Run() path. The --force flag is passed through to files.Generate() so it bypasses the sidecar cache when requested.
WalkthroughThe command control flow in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/analyze.go`:
- Around line 39-45: When cfg.FilesEnabled() && !noFiles causes an early return
to files.Generate, it skips analyze.Run's responsibilities (printing
summary/JSON and writing the content-hash cache). Update the branch so that
after calling files.Generate(cmd.Context(), cfg, dir,
files.GenerateOptions{Force: opts.Force}) you also invoke
analyze.Run(cmd.Context(), cfg, dir, opts) (or otherwise call the functions in
analyze.Run that emit JSON/summary and persist the cache) and ensure the command
output/exit behavior still reflects analyze.Run's result; keep references to
cfg.FilesEnabled, noFiles, files.Generate, files.GenerateOptions, and
analyze.Run to locate and implement the change.
- Around line 39-43: The current branch returns files.Generate(...) early which
trusts .supermodel/graph.json and can serve stale results because files.Generate
only checks file existence when --force is false; instead compute the same repo
fingerprint/content hash and cache validation used by analyze.Run (see
internal/analyze/handler.go fingerprinting logic) before calling files.Generate,
and if the fingerprint indicates the cache is valid let files.Generate proceed,
otherwise invalidate or regenerate the graph (or pass a Force-equivalent) so
files.Generate does not reuse stale .supermodel/graph.json; update
cmd/analyze.go to invoke the same fingerprinting/check routine (or extract it
into a helper used by both analyze.Run and files.Generate) so files.Generate
only runs when content-based validation passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| if cfg.FilesEnabled() && !noFiles { | ||
| return files.Generate(cmd.Context(), cfg, dir, files.GenerateOptions{}) | ||
| // File mode: Generate handles the full pipeline (API call + | ||
| // cache + sidecars) in a single upload. Running analyze.Run | ||
| // first would duplicate the API call. | ||
| return files.Generate(cmd.Context(), cfg, dir, files.GenerateOptions{Force: opts.Force}) | ||
| } | ||
| return nil | ||
| return analyze.Run(cmd.Context(), cfg, dir, opts) |
There was a problem hiding this comment.
Skipping analyze.Run also drops two analyze behaviors users already have.
analyze.Run is the thing that prints the requested summary/JSON and writes the content-hash cache advertised in this command’s help (internal/analyze/handler.go:21-92). files.Generate only renders sidecars and persists .supermodel/graph.json (internal/files/handler.go:82-132). So in the default mode, supermodel analyze -o json no longer behaves like JSON output, and this path never warms the shared cache for follow-up analysis commands.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/analyze.go` around lines 39 - 45, When cfg.FilesEnabled() && !noFiles
causes an early return to files.Generate, it skips analyze.Run's
responsibilities (printing summary/JSON and writing the content-hash cache).
Update the branch so that after calling files.Generate(cmd.Context(), cfg, dir,
files.GenerateOptions{Force: opts.Force}) you also invoke
analyze.Run(cmd.Context(), cfg, dir, opts) (or otherwise call the functions in
analyze.Run that emit JSON/summary and persist the cache) and ensure the command
output/exit behavior still reflects analyze.Run's result; keep references to
cfg.FilesEnabled, noFiles, files.Generate, files.GenerateOptions, and
analyze.Run to locate and implement the change.
| if cfg.FilesEnabled() && !noFiles { | ||
| return files.Generate(cmd.Context(), cfg, dir, files.GenerateOptions{}) | ||
| // File mode: Generate handles the full pipeline (API call + | ||
| // cache + sidecars) in a single upload. Running analyze.Run | ||
| // first would duplicate the API call. | ||
| return files.Generate(cmd.Context(), cfg, dir, files.GenerateOptions{Force: opts.Force}) |
There was a problem hiding this comment.
This makes the default analyze path serve stale results after the first run.
files.Generate only checks whether .supermodel/graph.json exists when --force is false (internal/files/handler.go:50-81). It never fingerprints or hashes the current repo the way analyze.Run does (internal/analyze/handler.go:33-69). Easy repro: run analyze once, edit any source file, run it again, and this branch will still reuse the old sidecar cache instead of analyzing the new repo. To keep the single-upload win, this path needs the same content-based cache validation before trusting .supermodel/graph.json.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/analyze.go` around lines 39 - 43, The current branch returns
files.Generate(...) early which trusts .supermodel/graph.json and can serve
stale results because files.Generate only checks file existence when --force is
false; instead compute the same repo fingerprint/content hash and cache
validation used by analyze.Run (see internal/analyze/handler.go fingerprinting
logic) before calling files.Generate, and if the fingerprint indicates the cache
is valid let files.Generate proceed, otherwise invalidate or regenerate the
graph (or pass a Force-equivalent) so files.Generate does not reuse stale
.supermodel/graph.json; update cmd/analyze.go to invoke the same
fingerprinting/check routine (or extract it into a helper used by both
analyze.Run and files.Generate) so files.Generate only runs when content-based
validation passes.
Proof of double API callOn Two distinct success lines (two API calls): Root cause: When After this PR: file mode skips |
Logs proving fix worksBefore (main) — two API callsProof the cache miss causes the second call: After (this PR) — one API callOne spinner, one success line, one API call. All paths verified
|
Full impact analysisWhat
|
| Command | Uses cache.Get? |
Impact of skipping analyze.Run() |
|---|---|---|
analyze (itself) |
Yes | N/A — we're changing this command |
find |
Yes | First run after analyze will make its own API call instead of cache hit |
focus |
Yes | Same — first run makes its own API call |
mcp |
Yes | Same — first tool use makes its own API call |
graph |
Yes (via GetGraph) |
Same |
Who doesn't care?
Commands that never check cache.Get() — they always create their own zip and hit their own API endpoint regardless:
| Command | Endpoint | Affected? |
|---|---|---|
dead-code |
/v1/analysis/dead-code |
No — always uploads its own zip |
blast-radius |
/v1/analysis/impact |
No — always uploads its own zip |
audit |
Both endpoints | No — always uploads |
docs (archdocs) |
/v1/graphs/supermodel |
No — always uploads |
restore |
/v1/graphs/supermodel |
No — always uploads |
share |
/v1/share |
No — different endpoint |
Do sidecars still work?
Yes, completely unaffected. files.Generate() calls AnalyzeSidecars → returns *api.SidecarIR with full nodes/labels/properties → writes .supermodel/graph.json → renders sidecars. This is the exact same code path as before, just without analyze.Run() running first.
The watch daemon reads from .supermodel/graph.json, not ~/.supermodel/cache/. Incremental updates, sidecar rendering, domain preservation — all use the sidecar cache. None of them depend on the analyze cache.
What's actually lost
If a user runs supermodel analyze then immediately runs supermodel find <symbol>, the find command used to get a cache hit from the graph that analyze pre-warmed. Now it will make its own API call (~30-60s) because the fingerprint cache at ~/.supermodel/cache/ wasn't populated.
This only affects find, focus, graph, and mcp — and only on the first run after analyze. These commands all populate the cache themselves, so subsequent runs are cached either way.
Is this a regression?
Marginal. The pre-warming was a side effect of analyze.Run(), not an intentional design. Before file mode existed (pre-PR #45), analyze only called analyze.Run(). The cache pre-warming was always there. PR #45 added files.Generate() as a second call, doubling the API cost. This PR removes the first call to eliminate the double API cost. The pre-warming was collateral.
The proper fix (future work) would be to have files.Generate() also write to ~/.supermodel/cache/ so both caches stay warm from a single API call. That requires converting SidecarIR → Graph or having the cache accept both types.
Supermodel impact analysisThis PR only changes |
Problem
supermodel analyzemakes two API calls to the same/v1/graphs/supermodelendpoint:analyze.Run()— uploads zip, gets graph, caches to~/.supermodel/cache/files.Generate()— uploads zip again, gets sidecar IR, caches to.supermodel/graph.jsonSame zip, same endpoint, two uploads. Users wait through two "Uploading and analyzing" phases.
Fix
When file mode is on (default), skip
analyze.Run()entirely. Letfiles.Generate()handle the full pipeline in a single API call. When--no-filesis set, use the originalanalyze.Run()path.The
--forceflag is passed through tofiles.Generate()so it bypasses the sidecar cache when requested.1 file changed, 5 lines.
Test plan
go test ./...— all tests passgo vet ./...— cleansupermodel analyze --force— one "Uploading" spinner, writes sidecars, prints summarysupermodel analyze --no-files --force— one "Uploading" spinner, prints summary table, no sidecarssupermodel analyze(cached) — "Using cached graph", renders from cache, no API callSummary by CodeRabbit
Bug Fixes
--forceflag behavior when file output is enabled. The flag now applies consistently across analysis and file generation operations.Refactor