refactor: consume /v1/analysis/dead-code API endpoint#9
Conversation
Replace local graph analysis with the dedicated dead code analysis endpoint. The API now handles parse graph + call graph combination, transitive dead code propagation, symbol-level import analysis, entry point detection, and confidence scoring server-side. - Switch from generateSupermodelGraph to generateDeadCodeAnalysis - Add async polling for job completion (202 → poll → 200) - Remove local findDeadCode, entry point heuristics, pattern constants - Keep filterByIgnorePatterns for client-side post-filtering - Enrich PR comment with Type, Confidence columns and metadata summary - Bump @supermodeltools/sdk ^0.4.1 → ^0.9.3 - Bump version 0.1.0 → 0.2.0 Closes #8
Dead Code HunterNo dead code found! Your codebase is clean. |
WalkthroughThis PR refactors the action to consume Supermodel's /v1/analysis/dead-code API with async polling, removes local graph heuristics in favor of client-side filtering/formatting of API results, updates PR comment formatting and metadata, adds a Markdown escape helper, updates tests, and bumps package/sdk version. Changes
Sequence DiagramsequenceDiagram
rect rgba(200,200,255,0.5)
participant Action as GitHub Action
end
rect rgba(200,255,200,0.5)
participant API as Supermodel API
end
rect rgba(255,200,200,0.5)
participant Result as DeadCodeAnalysisResponse
end
Action->>Action: Prepare repo zip + idempotency key (analysis:deadcode:...)
Action->>API: POST /v1/analysis/dead-code (zip, Idempotency-Key)
API-->>Action: 202 Accepted {status: processing, jobId, retryAfter}
loop Poll until completion or timeout
Action->>Action: sleep(DEFAULT_RETRY_INTERVAL_MS)
Action->>API: GET/POST with same idempotency key
API-->>Action: 202 or 200 {status, (result)}
end
API-->>Result: 200 {status: completed, result: DeadCodeAnalysisResponse}
Action->>Action: filterByIgnorePatterns(result.deadCodeCandidates, ignorePatterns)
Action->>Action: formatPrComment(filteredCandidates, result.metadata)
Action->>Action: set outputs (dead-code-count, dead-code-json, metadata) + post PR comment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
…ection Add uncalled functions in dead-code.ts (truncateString, groupByDirectory, fileSeverity) and a new markdown.ts module (badge, barChart, numberedList, collapsible, escapeTableCell) to verify the API actually finds dead code. Integration tests now assert: - Response shape is valid (metadata, candidates, aliveCode, entryPoints) - Dead code count > 0 - Every candidate has file, name, line, type, confidence, reason - Known dead functions are found by name (at least 3 of 6) - ignore-patterns filtering works against real results
Dead Code HunterFound 3 potentially unused code elements:
Analysis summary
Powered by Supermodel dead code analysis |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/index.ts`:
- Around line 168-171: The ignore-patterns input is parsed blindly with
JSON.parse which throws on plain glob strings; change the initialization of
ignorePatterns (currently using core.getInput('ignore-patterns')) to first read
the raw string, return [] if empty, then attempt JSON.parse in a try/catch and
on parse failure fall back to treating the raw input as a single-pattern array
(e.g., [raw]) or [] if raw is blank; update the code around commentOnPr,
failOnDeadCode, and ignorePatterns (references:
core.getInput('ignore-patterns'), ignorePatterns) accordingly to avoid unhandled
exceptions.
🧹 Nitpick comments (1)
src/dead-code.ts (1)
32-40: Consider escaping table cells to avoid broken Markdown.Example: a symbol named
foo|barwill split the table columns. Escaping|(and backticks) keeps the table stable.Suggested tweak
- const rows = candidates + const escapeCell = (value: string) => + value.replace(/\|/g, '\\|').replace(/`/g, '\\`'); + + const rows = candidates .slice(0, 50) .map(dc => { const lineInfo = dc.line ? `L${dc.line}` : ''; - const fileLink = dc.line ? `${dc.file}#L${dc.line}` : dc.file; + const fileLink = dc.line ? `${dc.file}#L${dc.line}` : dc.file; const badge = dc.confidence === 'high' ? ':red_circle:' : dc.confidence === 'medium' ? ':orange_circle:' : ':yellow_circle:'; - return `| \`${dc.name}\` | ${dc.type} | ${fileLink} | ${lineInfo} | ${badge} ${dc.confidence} |`; + return `| \`${escapeCell(dc.name)}\` | ${escapeCell(dc.type)} | ${escapeCell(fileLink)} | ${lineInfo} | ${badge} ${dc.confidence} |`; })
| const commentOnPr = core.getBooleanInput('comment-on-pr'); | ||
| const failOnDeadCode = core.getBooleanInput('fail-on-dead-code'); | ||
| const ignorePatterns = JSON.parse(core.getInput('ignore-patterns') || '[]'); | ||
| const ignorePatterns: string[] = JSON.parse(core.getInput('ignore-patterns') || '[]'); | ||
|
|
There was a problem hiding this comment.
Guard JSON parsing for ignore patterns.
Right now a non-JSON input (e.g., **/*.test.ts) will throw and fail the action. A friendly fallback keeps UX smooth.
Suggested fix
- const ignorePatterns: string[] = JSON.parse(core.getInput('ignore-patterns') || '[]');
+ let ignorePatterns: string[] = [];
+ try {
+ const raw = core.getInput('ignore-patterns') || '[]';
+ const parsed = JSON.parse(raw);
+ if (!Array.isArray(parsed)) throw new Error('ignore-patterns must be an array');
+ ignorePatterns = parsed;
+ } catch {
+ core.warning('ignore-patterns should be a JSON array, e.g. ["**/*.test.ts"]');
+ ignorePatterns = [];
+ }📝 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.
| const commentOnPr = core.getBooleanInput('comment-on-pr'); | |
| const failOnDeadCode = core.getBooleanInput('fail-on-dead-code'); | |
| const ignorePatterns = JSON.parse(core.getInput('ignore-patterns') || '[]'); | |
| const ignorePatterns: string[] = JSON.parse(core.getInput('ignore-patterns') || '[]'); | |
| const commentOnPr = core.getBooleanInput('comment-on-pr'); | |
| const failOnDeadCode = core.getBooleanInput('fail-on-dead-code'); | |
| let ignorePatterns: string[] = []; | |
| try { | |
| const raw = core.getInput('ignore-patterns') || '[]'; | |
| const parsed = JSON.parse(raw); | |
| if (!Array.isArray(parsed)) throw new Error('ignore-patterns must be an array'); | |
| ignorePatterns = parsed; | |
| } catch { | |
| core.warning('ignore-patterns should be a JSON array, e.g. ["**/*.test.ts"]'); | |
| ignorePatterns = []; | |
| } |
🤖 Prompt for AI Agents
In `@src/index.ts` around lines 168 - 171, The ignore-patterns input is parsed
blindly with JSON.parse which throws on plain glob strings; change the
initialization of ignorePatterns (currently using
core.getInput('ignore-patterns')) to first read the raw string, return [] if
empty, then attempt JSON.parse in a try/catch and on parse failure fall back to
treating the raw input as a single-pattern array (e.g., [raw]) or [] if raw is
blank; update the code around commentOnPr, failOnDeadCode, and ignorePatterns
(references: core.getInput('ignore-patterns'), ignorePatterns) accordingly to
avoid unhandled exceptions.
markdown.ts was never imported, so the API treated it as a root file (entry point) and didn't flag its exports. Now dead-code.ts imports and uses escapeTableCell, pulling markdown.ts into the import graph. The remaining unused exports (badge, barChart, collapsible, numberedList) should now be flagged as dead via symbol-level import analysis. Also adds diagnostic logging of full analysis results to CI output.
Dead Code HunterFound 7 potentially unused code elements:
Analysis summary
Powered by Supermodel dead code analysis |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/markdown.ts`:
- Around line 24-28: The barChart function can throw RangeError when max === 0
or value > max; fix by clamping inputs: if max === 0 treat as no capacity (set
filled = 0), otherwise clamp value to the range [0, max], compute filled =
Math.round((clampedValue / max) * width) and set empty = Math.max(0, width -
filled) so neither repeat() receives a negative or non-finite count; update the
barChart function accordingly.
🧹 Nitpick comments (4)
src/markdown.ts (1)
15-19: Heads up: shields.io has special escaping rules for dashes and underscores.
encodeURIComponentwon't help here because shields.io uses-as a path separator. If yourlabelorvaluecontains a literal dash (like"dead-code"), it'll get misinterpreted as multiple path segments, and your badge will look funky.For example:
- Input:
badge("dead-code", "5", "red")- Output URL:
.../dead-code-5-red→ shields.io sees 4 segments instead of 2Shields.io wants
--for a literal dash and__for a literal underscore.💡 Optional fix if you expect special characters
export function badge(label: string, value: string, color: string): string { - const encodedLabel = encodeURIComponent(label); - const encodedValue = encodeURIComponent(value); + // shields.io uses - as separator; escape -- for dash, __ for underscore + const escapeShields = (s: string) => + s.replace(/-/g, '--').replace(/_/g, '__'); + const encodedLabel = encodeURIComponent(escapeShields(label)); + const encodedValue = encodeURIComponent(escapeShields(value)); return ``; }src/dead-code.ts (2)
9-11: Edge case with smallmaxLenvaluesQuick heads up - if someone passes
maxLen <= 1, you get some quirky behavior:
maxLen = 1→ returns just…(the ellipsis)maxLen = 0→ returns all but last char plus…(becauseslice(0, -1))Probably not a real-world issue since you're likely using this with reasonable lengths like 50+, but a guard like
if (maxLen < 2) return '…'would make it bulletproof.
69-73: File links won't be clickable in GitHubRight now
fileLinkends up as something likesrc/foo.ts#L42which will just render as plain text in the markdown table. If you want clickable links, you'd need proper markdown syntax.That said, maybe plain text is intentional here? GitHub PR comments don't always have access to link to the actual file lines anyway (depends on whether it's in the context of the PR). If you do want links, you'd need to construct full URLs which requires repo context.
Just flagging this in case it wasn't intentional - if plain text is fine for your use case, carry on!
src/__tests__/integration.test.ts (1)
12-43: Solid polling implementation with one timing quirkThe logic looks good overall. Nice touches:
- Respects
retryAfterfrom the API with a sensible default- Handles both
failedandcompletedstates- Has both timeout and max attempts as safety nets
One minor thing: the timeout check on line 34 happens after the API call. So if you're at 119 seconds and the API call takes 30 seconds to respond, you'd be at 149 seconds before the timeout triggers. For integration tests this is probably fine, but if you reuse this helper in production code, you might want to check the timeout before making the call too.
for (let attempt = 1; attempt <= 30; attempt++) { + if (Date.now() - startTime >= timeoutMs) { + throw new Error(`Polling timed out after ${timeoutMs}ms`); + } + const response: DeadCodeAnalysisResponseAsync = await api.generateDeadCodeAnalysis({Not a big deal for test code though - just something to keep in mind!
src/markdown.ts
Outdated
| export function barChart(value: number, max: number, width = 20): string { | ||
| const filled = Math.round((value / max) * width); | ||
| const empty = width - filled; | ||
| return '\u2588'.repeat(filled) + '\u2591'.repeat(empty); | ||
| } |
There was a problem hiding this comment.
Edge cases will throw RangeError at runtime.
Two scenarios that'll crash:
max === 0: You getvalue / 0 = Infinity, and'█'.repeat(Infinity)throws.value > max: SaybarChart(15, 10)→filled = 30,empty = -10, and'░'.repeat(-10)throws.
Both cause RangeError: Invalid count value.
🐛 Proposed fix with clamping
export function barChart(value: number, max: number, width = 20): string {
+ if (max <= 0) {
+ return '\u2591'.repeat(width); // empty bar when max is invalid
+ }
- const filled = Math.round((value / max) * width);
+ const ratio = Math.max(0, Math.min(1, value / max)); // clamp to [0, 1]
+ const filled = Math.round(ratio * width);
const empty = width - filled;
return '\u2588'.repeat(filled) + '\u2591'.repeat(empty);
}📝 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.
| export function barChart(value: number, max: number, width = 20): string { | |
| const filled = Math.round((value / max) * width); | |
| const empty = width - filled; | |
| return '\u2588'.repeat(filled) + '\u2591'.repeat(empty); | |
| } | |
| export function barChart(value: number, max: number, width = 20): string { | |
| if (max <= 0) { | |
| return '\u2591'.repeat(width); // empty bar when max is invalid | |
| } | |
| const ratio = Math.max(0, Math.min(1, value / max)); // clamp to [0, 1] | |
| const filled = Math.round(ratio * width); | |
| const empty = width - filled; | |
| return '\u2588'.repeat(filled) + '\u2591'.repeat(empty); | |
| } |
🤖 Prompt for AI Agents
In `@src/markdown.ts` around lines 24 - 28, The barChart function can throw
RangeError when max === 0 or value > max; fix by clamping inputs: if max === 0
treat as no capacity (set filled = 0), otherwise clamp value to the range [0,
max], compute filled = Math.round((clampedValue / max) * width) and set empty =
Math.max(0, width - filled) so neither repeat() receives a negative or
non-finite count; update the barChart function accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/dead-code.ts`:
- Around line 8-12: truncateString currently misbehaves for tiny limits; add
guards at the start of truncateString to handle maxLen <= 0 and maxLen === 1: if
maxLen <= 0 return an empty string, if maxLen === 1 return the first character
of str (no ellipsis), otherwise preserve the existing logic (for maxLen > 1 use
str.slice(0, maxLen - 1) + '\u2026'); ensure you still handle short strings
where str.length <= maxLen early and reference the truncateString function and
the maxLen parameter in your change.
src/dead-code.ts
Outdated
| * Truncates a string to the given max length, appending an ellipsis if needed. | ||
| */ | ||
| export function isEntryPointFile(filePath: string): boolean { | ||
| return ENTRY_POINT_PATTERNS.some(pattern => minimatch(filePath, pattern)); | ||
| export function truncateString(str: string, maxLen: number): string { | ||
| if (str.length <= maxLen) return str; | ||
| return str.slice(0, maxLen - 1) + '\u2026'; |
There was a problem hiding this comment.
Guard truncateString when maxLen is 0 or 1.
Right now maxLen=0 on "abc" returns "ab", which breaks the “max length” contract.
🔧 Small guard for tiny limits
export function truncateString(str: string, maxLen: number): string {
+ if (maxLen <= 0) return '';
+ if (maxLen === 1) return str.length > 1 ? '\u2026' : str;
if (str.length <= maxLen) return str;
return str.slice(0, maxLen - 1) + '\u2026';
}🤖 Prompt for AI Agents
In `@src/dead-code.ts` around lines 8 - 12, truncateString currently misbehaves
for tiny limits; add guards at the start of truncateString to handle maxLen <= 0
and maxLen === 1: if maxLen <= 0 return an empty string, if maxLen === 1 return
the first character of str (no ellipsis), otherwise preserve the existing logic
(for maxLen > 1 use str.slice(0, maxLen - 1) + '\u2026'); ensure you still
handle short strings where str.length <= maxLen early and reference the
truncateString function and the maxLen parameter in your change.
Dead Code HunterFound 7 potentially unused code elements:
Analysis summary
Powered by Supermodel dead code analysis |
Remove test dead code (truncateString, groupByDirectory, fileSeverity, collapsible, badge, barChart, numberedList) that was used to verify orphaned file detection. Update README and action.yml to reflect the new API-driven architecture with confidence levels and broader type detection. Add escapeTableCell and code type coverage to unit tests.
Dead Code HunterNo dead code found! Your codebase is clean. |
1 participant
Summary
POST /v1/analysis/dead-codeAPI endpointfindDeadCode, entry point patterns, etc.)@supermodeltools/sdkto^0.9.3and version to0.2.0Test plan
nccbuild compiles cleanlyuses: ./Closes #8
Summary by CodeRabbit
Chores
New Features
Documentation