Add impact analysis, test coverage, and circular dependency endpoints#3
Draft
jonathanpopham wants to merge 2 commits intomainfrom
Draft
Add impact analysis, test coverage, and circular dependency endpoints#3jonathanpopham wants to merge 2 commits intomainfrom
jonathanpopham wants to merge 2 commits intomainfrom
Conversation
Calls all three new analysis endpoints in parallel alongside the existing supermodel graph endpoint. Analysis results are used to enrich the generated entity pages with: - Impact analysis: blast radius, risk score, affected entry points - Test coverage: tested/untested status per function, coverage % per file - Circular dependencies: cycle membership, severity, breaking suggestions Adds new pills (impact level, test coverage, dependency health), new body sections on entity pages, and three new taxonomies for browsing by impact level, test coverage status, and dependency health. Analysis endpoints fail gracefully with warnings — the site still generates normally if any of the new endpoints are unavailable. Resolves #1
Renders per-file coverage as styled progress bars with percentage, file path, and tested/total ratio. File entities show function-level breakdown with check/x indicators. Directory/Module entities show sorted child file bars.
WalkthroughThe PR adds concurrent API calls to four endpoints (supermodel, impact, test-coverage, circular-deps), introduces response models for analysis data, and implements a markdown enrichment workflow that injects impact metrics, test coverage, and dependency health information into generated documentation files. Changes
Sequence DiagramsequenceDiagram
participant Client as Main Process
participant SM as Supermodel API
participant IM as Impact API
participant TC as Test Coverage API
participant CD as Circular Deps API
participant FS as File System
rect rgba(100, 150, 255, 0.5)
Note over Client,CD: Parallel API Calls
par Supermodel Call
Client->>SM: POST /supermodel with zip
SM-->>Client: graphJSON response
and Impact Call
Client->>IM: POST /impact with zip
IM-->>Client: impactJSON response
and Test Coverage Call
Client->>TC: POST /test-coverage with zip
TC-->>Client: testCoverageJSON response
and Circular Deps Call
Client->>CD: POST /circular-deps with zip
CD-->>Client: circularDepsJSON response
end
end
rect rgba(150, 200, 100, 0.5)
Note over Client,FS: Enrichment & File Writing
Client->>Client: enrichMarkdown with JSON responses
Client->>FS: Extract frontmatter & inject enrichment data
Client->>FS: Write enhanced markdown files with sections
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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
🤖 Fix all issues with AI agents
In `@main.go`:
- Around line 416-430: The three os.WriteFile calls for impactJSON,
testCoverageJSON, and circularDepsJSON currently ignore errors; update each
block (the branches guarding impactJSON, testCoverageJSON, circularDepsJSON) to
capture the error returned by os.WriteFile and, on error, log a warning with
fmt.Printf (or a logger) that includes the file path (constructed with
filepath.Join(tmpDir, "...")) and the error details instead of silently
swallowing it; keep the existing success printf behavior.
- Line 1246: The os.WriteFile call currently ignores its error; change the call
at os.WriteFile(path, []byte(newContent), info.Mode()) to capture the returned
error and handle it: if err != nil, log a warning that includes the filename
(path) and the error (and optionally the content length or mode for context)
using your project's logger (or log.Printf) so write failures are visible and
actionable; ensure you keep the same variables (path, newContent, info.Mode())
when reporting the error.
🧹 Nitpick comments (5)
templates/_styles.css (1)
189-189: Consider using modern CSS color notation.The static analysis tool flags the legacy
rgba()syntax. Modern CSS prefersrgb(255 255 255 / 6%)overrgba(255,255,255,0.06). It's functionally the same but aligns with current CSS specs.💅 Proposed fix
-.cov-bar { width: 120px; height: 10px; background: rgba(255,255,255,0.06); border-radius: 3px; overflow: hidden; flex-shrink: 0; } +.cov-bar { width: 120px; height: 10px; background: rgb(255 255 255 / 6%); border-radius: 3px; overflow: hidden; flex-shrink: 0; }main.go (4)
1086-1094: The impact level logic works but could be clearer.Right now you have two separate
ifblocks where the second one (> 100) can override the first. It works, but reading it makes you do a double-take.Think of it like: if someone is 25 years old, you don't first say "they're a teenager" then correct yourself to "actually an adult." Just check the thresholds in descending order:
♻️ Cleaner threshold ordering
level := "Low" -if impact.BlastRadius.RiskScore >= 30 { +if impact.BlastRadius.RiskScore > 100 { + level = "Critical" +} else if impact.BlastRadius.RiskScore >= 30 { level = "High" } else if impact.BlastRadius.RiskScore >= 10 { level = "Medium" } -if impact.BlastRadius.RiskScore > 100 { - level = "Critical" -}
1153-1159: Consider escaping HTML in function names.The
namevariable gets dropped directly into HTML. While function names from parsed code are usually safe identifiers, if one somehow contained<script>alert(1)</script>(maybe from a weird edge case or API bug), you'd have broken HTML or worse.Go has
html.EscapeString()in thehtmlpackage for exactly this:🛡️ Escape names before interpolation
Add to imports:
import "html"Then wrap the names:
for _, name := range testedNamesInFile[filePath] { - covLines = append(covLines, fmt.Sprintf(`<span class="cov-func"><span class="cov-check">✓</span> %s</span>`, name)) + covLines = append(covLines, fmt.Sprintf(`<span class="cov-func"><span class="cov-check">✓</span> %s</span>`, html.EscapeString(name))) } for _, name := range untestedNamesInFile[filePath] { - covLines = append(covLines, fmt.Sprintf(`<span class="cov-func"><span class="cov-x">✗</span> %s</span>`, name)) + covLines = append(covLines, fmt.Sprintf(`<span class="cov-func"><span class="cov-x">✗</span> %s</span>`, html.EscapeString(name))) }
1268-1285: Same escaping concern for thelabelparameter.File paths are generally safe, but same principle applies - if a file path contains
&or<, you'd get malformed HTML.🛡️ Escape label in coverageBarHTML
+import "html" func coverageBarHTML(label string, pct float64, tested, total int) string { color := "var(--red)" if pct >= 80 { color = "var(--green)" } else if pct > 0 { color = "var(--orange)" } return fmt.Sprintf( `<div class="cov-row">`+ `<div class="cov-bar"><div class="cov-fill" style="width:%.1f%%;background:%s"></div></div>`+ `<span class="cov-pct">%.1f%%</span>`+ `<span class="cov-label">%s</span>`+ `<span class="cov-ratio">(%d/%d)</span>`+ `</div>`, - pct, color, pct, label, tested, total, + pct, color, pct, html.EscapeString(label), tested, total, ) }
1049-1049:filepath.Walkreturn value is ignored.If
contentDirdoesn't exist or has permission issues, Walk returns an error but this code just moves on. In practice the directory should exist sincemain()creates it, but for defensive coding you might want to at least log it:♻️ Handle Walk error
- filepath.Walk(contentDir, func(path string, info os.FileInfo, err error) error { + if err := filepath.Walk(contentDir, func(path string, info os.FileInfo, err error) error { // ... existing callback ... - }) + }); err != nil { + fmt.Printf("::warning::Error walking content dir: %v\n", err) + }
Comment on lines
+416
to
+430
| if impactJSON != nil { | ||
| p := filepath.Join(tmpDir, "impact.json") | ||
| os.WriteFile(p, impactJSON, 0644) | ||
| fmt.Printf("Impact analysis saved to %s\n", p) | ||
| } | ||
| if testCoverageJSON != nil { | ||
| p := filepath.Join(tmpDir, "test-coverage.json") | ||
| os.WriteFile(p, testCoverageJSON, 0644) | ||
| fmt.Printf("Test coverage saved to %s\n", p) | ||
| } | ||
| if circularDepsJSON != nil { | ||
| p := filepath.Join(tmpDir, "circular-deps.json") | ||
| os.WriteFile(p, circularDepsJSON, 0644) | ||
| fmt.Printf("Circular dependencies saved to %s\n", p) | ||
| } |
There was a problem hiding this comment.
Ignored errors on WriteFile could hide problems.
You're handling the graph.json write error properly (lines 411-413), but the other three writes silently swallow errors. If one of these fails (disk full, permissions, etc.), you'd never know.
Since these are optional/degradable, a warning makes sense rather than failing:
🔧 Proposed fix: log warnings on write failures
if impactJSON != nil {
p := filepath.Join(tmpDir, "impact.json")
- os.WriteFile(p, impactJSON, 0644)
- fmt.Printf("Impact analysis saved to %s\n", p)
+ if err := os.WriteFile(p, impactJSON, 0644); err != nil {
+ fmt.Printf("::warning::Failed to save impact analysis: %v\n", err)
+ } else {
+ fmt.Printf("Impact analysis saved to %s\n", p)
+ }
}
if testCoverageJSON != nil {
p := filepath.Join(tmpDir, "test-coverage.json")
- os.WriteFile(p, testCoverageJSON, 0644)
- fmt.Printf("Test coverage saved to %s\n", p)
+ if err := os.WriteFile(p, testCoverageJSON, 0644); err != nil {
+ fmt.Printf("::warning::Failed to save test coverage: %v\n", err)
+ } else {
+ fmt.Printf("Test coverage saved to %s\n", p)
+ }
}
if circularDepsJSON != nil {
p := filepath.Join(tmpDir, "circular-deps.json")
- os.WriteFile(p, circularDepsJSON, 0644)
- fmt.Printf("Circular dependencies saved to %s\n", p)
+ if err := os.WriteFile(p, circularDepsJSON, 0644); err != nil {
+ fmt.Printf("::warning::Failed to save circular deps: %v\n", err)
+ } else {
+ fmt.Printf("Circular dependencies saved to %s\n", p)
+ }
}🤖 Prompt for AI Agents
In `@main.go` around lines 416 - 430, The three os.WriteFile calls for impactJSON,
testCoverageJSON, and circularDepsJSON currently ignore errors; update each
block (the branches guarding impactJSON, testCoverageJSON, circularDepsJSON) to
capture the error returned by os.WriteFile and, on error, log a warning with
fmt.Printf (or a logger) that includes the file path (constructed with
filepath.Join(tmpDir, "...")) and the error details instead of silently
swallowing it; keep the existing success printf behavior.
| } | ||
|
|
||
| newContent := newFrontmatter + "\n---\n" + newBody | ||
| os.WriteFile(path, []byte(newContent), info.Mode()) |
There was a problem hiding this comment.
Another ignored WriteFile error.
Same pattern as the JSON saves - if this fails, the file doesn't get enriched and you'd never know. At minimum, logging a warning helps debugging:
🔧 Log write failures
- os.WriteFile(path, []byte(newContent), info.Mode())
- enriched++
+ if err := os.WriteFile(path, []byte(newContent), info.Mode()); err != nil {
+ fmt.Printf("::warning::Failed to enrich %s: %v\n", path, err)
+ } else {
+ enriched++
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| os.WriteFile(path, []byte(newContent), info.Mode()) | |
| if err := os.WriteFile(path, []byte(newContent), info.Mode()); err != nil { | |
| fmt.Printf("::warning::Failed to enrich %s: %v\n", path, err) | |
| } else { | |
| enriched++ | |
| } |
🤖 Prompt for AI Agents
In `@main.go` at line 1246, The os.WriteFile call currently ignores its error;
change the call at os.WriteFile(path, []byte(newContent), info.Mode()) to
capture the returned error and handle it: if err != nil, log a warning that
includes the filename (path) and the error (and optionally the content length or
mode for context) using your project's logger (or log.Printf) so write failures
are visible and actionable; ensure you keep the same variables (path,
newContent, info.Mode()) when reporting 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.
Summary
Test plan
Summary by CodeRabbit
Release Notes