Merged
Conversation
supermodel share runs the same health audit pipeline and uploads the rendered Markdown report to the Supermodel API, returning a public URL and a ready-to-paste README badge: Report: https://supermodeltools.com/s/abc123 Add this badge to your README: [](…) Implementation: - api.ShareRequest / ShareResponse types in internal/api/types.go - api.Client.Share() method using the existing request() helper - jsonBody() helper for JSON POST bodies - cmd/share.go wires the command; reuses resolveAuditDir, shareAnalyze, and runImpactForShare (same pattern as cmd/audit.go) Note: requires POST /v1/share endpoint on the backend to return URLs. Closes #52 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Command
participant Archive as Archive/Compression
participant AnalyzeAPI as API: Analyze
participant ImpactAPI as API: Impact
participant UploadAPI as API: Share Upload
participant Report as Report Generator
CLI->>Archive: Create archive + compute hash
Archive-->>CLI: archive.zip + cacheKey
CLI->>AnalyzeAPI: POST analyze (archive / key)
AnalyzeAPI-->>CLI: SupermodelIR
CLI->>Report: Convert IR -> health report (Markdown)
Report-->>CLI: Markdown content
alt Impact analysis available
CLI->>Archive: Create second archive + hash
Archive-->>CLI: secondArchive.zip + cacheKey2
CLI->>ImpactAPI: POST impact (archive2 / key2)
ImpactAPI-->>CLI: ImpactResult
CLI->>Report: Enrich health report with impact
else Impact fails
ImpactAPI-->>CLI: Error (warning logged)
end
CLI->>UploadAPI: POST /v1/share (project, status, content)
UploadAPI-->>CLI: ShareResponse { url }
CLI-->>CLI: Cleanup temp files
CLI-->>CLI: Print URL + README badge
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 5❌ Failed checks (4 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
greynewell
changed the title
Staticcheck cannot see that t.Fatal halts goroutine execution, so it flags the nil-checked pointer accesses immediately after as potential nil dereferences. Adding unreachable return statements satisfies it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/share.go (1)
100-137: Reuse one archive/hash across analyze + impact.You currently zip/hash twice. That adds extra cost and can drift results if the working tree changes between steps.
Refactor direction
- ir, err := shareAnalyze(cmd, cfg, rootDir, projectName) + zipPath, err := audit.CreateZip(rootDir) + // defer remove zipPath + hash, err := cache.HashFile(zipPath) + ir, err := shareAnalyzeWithArchive(cmd, cfg, zipPath, hash, projectName) - impact, err := runImpactForShare(cmd, cfg, rootDir) + impact, err := runImpactForShareWithArchive(cmd, cfg, zipPath, hash)This keeps both analyses on the exact same snapshot and removes duplicate archive work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/share.go` around lines 100 - 137, The code duplicates zipping/hashing in Analyze (client.AnalyzeDomains) and Impact (runImpactForShare -> client.Impact); instead, create the archive and hash once and pass them into both calls to avoid extra work and drift. Change the call site to call audit.CreateZip(rootDir) and cache.HashFile(zipPath) once, defer removing zip once, then call client.AnalyzeDomains(ctx, zipPath, "share-"+hash[:16]) and client.Impact(ctx, zipPath, "share-impact-"+hash[:16], "", "") using the same zipPath and hash; update runImpactForShare signature to accept zipPath and hash (or extract a helper that performs the shared logic) and remove its internal CreateZip/HashFile and file deletion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/api/client.go`:
- Around line 371-379: The Share method always sends an empty idempotency key
causing duplicate public links; update Client.Share to accept or compute and
include a deterministic idempotency key in the POST payload/header when calling
c.request (referencing Share, ShareRequest, ShareResponse and c.request), e.g.,
add an IdempotencyKey field to ShareRequest or build a key from the rendered
content and pass it through jsonBody(req) or as a header argument to c.request;
then update the caller (cmd/share.go) to compute and provide a stable key (for
example a hash of the rendered markdown bytes) so retries use the same
idempotency key.
- Around line 381-385: The jsonBody function currently swallows json.Marshal
errors and returns an invalid reader; change jsonBody signature to return
(io.Reader, error), propagate and return the marshal error instead of ignoring
it, and update all call sites (e.g., where ShareRequest builds its request body
and any uses highlighted near build.go usages) to handle the error (check it,
log/return it, and avoid sending a broken body). Ensure callers construct
requests only on successful jsonBody results and adjust any helper functions
that assumed the old signature to accept and forward the error.
---
Nitpick comments:
In `@cmd/share.go`:
- Around line 100-137: The code duplicates zipping/hashing in Analyze
(client.AnalyzeDomains) and Impact (runImpactForShare -> client.Impact);
instead, create the archive and hash once and pass them into both calls to avoid
extra work and drift. Change the call site to call audit.CreateZip(rootDir) and
cache.HashFile(zipPath) once, defer removing zip once, then call
client.AnalyzeDomains(ctx, zipPath, "share-"+hash[:16]) and client.Impact(ctx,
zipPath, "share-impact-"+hash[:16], "", "") using the same zipPath and hash;
update runImpactForShare signature to accept zipPath and hash (or extract a
helper that performs the shared logic) and remove its internal
CreateZip/HashFile and file deletion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf10d9c3-e4dc-4042-ac3c-0bd9e59f89e7
📒 Files selected for processing (3)
cmd/share.gointernal/api/client.gointernal/api/types.go
Comment on lines
+371
to
+379
| // Share uploads a rendered report and returns a public URL. | ||
| func (c *Client) Share(ctx context.Context, req ShareRequest) (string, error) { | ||
| var resp ShareResponse | ||
| if err := c.request(ctx, "POST", "/v1/share", "application/json", | ||
| jsonBody(req), "", &resp); err != nil { | ||
| return "", err | ||
| } | ||
| return resp.URL, nil | ||
| } |
There was a problem hiding this comment.
Add idempotency support for POST /v1/share to prevent duplicate public reports.
Right now Share hardcodes an empty key, so retries/reruns can create multiple links for the same report.
Suggested fix
-func (c *Client) Share(ctx context.Context, req ShareRequest) (string, error) {
+func (c *Client) Share(ctx context.Context, req ShareRequest, idempotencyKey string) (string, error) {
var resp ShareResponse
- if err := c.request(ctx, "POST", "/v1/share", "application/json",
- jsonBody(req), "", &resp); err != nil {
+ body, err := jsonBody(req)
+ if err != nil {
+ return "", fmt.Errorf("encode share payload: %w", err)
+ }
+ if err := c.request(ctx, http.MethodPost, "/v1/share", "application/json",
+ body, idempotencyKey, &resp); err != nil {
return "", err
}
return resp.URL, nil
}And in caller (cmd/share.go), pass a deterministic key (for example from rendered markdown bytes).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/api/client.go` around lines 371 - 379, The Share method always sends
an empty idempotency key causing duplicate public links; update Client.Share to
accept or compute and include a deterministic idempotency key in the POST
payload/header when calling c.request (referencing Share, ShareRequest,
ShareResponse and c.request), e.g., add an IdempotencyKey field to ShareRequest
or build a key from the rendered content and pass it through jsonBody(req) or as
a header argument to c.request; then update the caller (cmd/share.go) to compute
and provide a stable key (for example a hash of the rendered markdown bytes) so
retries use the same idempotency key.
Comment on lines
+381
to
+385
| // jsonBody encodes v as JSON and returns it as an io.Reader. | ||
| func jsonBody(v any) io.Reader { | ||
| b, _ := json.Marshal(v) | ||
| return bytes.NewReader(b) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify ignored json.Marshal errors and current jsonBody usage.
rg -n --type go 'b,\s*_\s*:=\s*json\.Marshal\(' -C2
rg -n --type go '\bjsonBody\s*\(' -C2Repository: supermodeltools/cli
Length of output: 920
🏁 Script executed:
# Find the request method definition to understand its signature
rg -n --type go 'func\s*\(\w+\s+\*?\w+\)\s+request\s*\(' -A5
# Count all jsonBody usages
rg -n --type go '\bjsonBody\s*\('Repository: supermodeltools/cli
Length of output: 642
🏁 Script executed:
sed -n '370,378p' internal/api/client.goRepository: supermodeltools/cli
Length of output: 379
🏁 Script executed:
# Find ShareRequest definition
rg -n --type go 'type\s+ShareRequest\s+struct' -A10
# Check for other similar json.Marshal error ignoring patterns
rg -n --type go 'json\.(Marshal|Unmarshal)\s*\([^)]+\)\s*$' -A1Repository: supermodeltools/cli
Length of output: 1997
Don't ignore JSON marshal errors in jsonBody.
Right now if encoding fails, you're sending an empty/broken body downstream and hiding the root cause. It's not great practice—even though ShareRequest is just strings and unlikely to fail in practice, this pattern elsewhere in the codebase (like build.go lines 459 and 1165) shows inconsistent error handling. Better to fail fast and be explicit.
The suggested fix is incomplete though—changing jsonBody to return an error means you also need to refactor the call site:
Complete fix
// jsonBody function change
-func jsonBody(v any) io.Reader {
- b, _ := json.Marshal(v)
- return bytes.NewReader(b)
+func jsonBody(v any) (io.Reader, error) {
+ b, err := json.Marshal(v)
+ if err != nil {
+ return nil, err
+ }
+ return bytes.NewReader(b), nil
}
// Call site in Share function change
func (c *Client) Share(ctx context.Context, req ShareRequest) (string, error) {
var resp ShareResponse
+ body, err := jsonBody(req)
+ if err != nil {
+ return "", err
+ }
- if err := c.request(ctx, "POST", "/v1/share", "application/json",
- jsonBody(req), "", &resp); err != nil {
+ if err := c.request(ctx, "POST", "/v1/share", "application/json",
+ body, "", &resp); err != nil {
return "", err
}
return resp.URL, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/api/client.go` around lines 381 - 385, The jsonBody function
currently swallows json.Marshal errors and returns an invalid reader; change
jsonBody signature to return (io.Reader, error), propagate and return the
marshal error instead of ignoring it, and update all call sites (e.g., where
ShareRequest builds its request body and any uses highlighted near build.go
usages) to handle the error (check it, log/return it, and avoid sending a broken
body). Ensure callers construct requests only on successful jsonBody results and
adjust any helper functions that assumed the old signature to accept and forward
the error.
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
1 participant
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.
Closes #53 #54
What changed
Complete README rewrite focused on outcomes over features.
Headline: "Your AI agent knows your entire codebase — not just the files it's looking at."
New sections:
watch→.graph files appear→ask anythingRestructured:
supermodel setup(was buried)🤖 Generated with Claude Code
Summary by CodeRabbit
supermodel sharecommand to generate a Markdown health report, upload it, and print a public shareable URL and README badge (optional--dir).