Skip to content

Commit 453096c

Browse files
Phoenix1808Phoenix1808
committed
fix: show absolute HEAD cloud costs in PR bot, remove delta-only reporting
1 parent 0e2761d commit 453096c

1 file changed

Lines changed: 103 additions & 88 deletions

File tree

‎src/routes/bot.ts‎

Lines changed: 103 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { FastifyInstance, FastifyRequest } from 'fastify';
22
import { Octokit } from '@octokit/rest';
3-
import { config } from '../config.js';
43
import { computeCostDiff } from '../services/diff/cost-diff.js';
54

65
interface BotPayload {
@@ -15,159 +14,175 @@ interface BotPayload {
1514
function getLanguageFromFilename(filename: string): string | null {
1615
if (filename.endsWith('.ts') || filename.endsWith('.tsx')) return 'typescript';
1716
if (filename.endsWith('.js') || filename.endsWith('.jsx')) return 'javascript';
18-
if (filename.endsWith('.py')) return 'python';
19-
if (filename.endsWith('.go')) return 'go';
2017
return null;
2118
}
2219

2320
export async function botRoutes(app: FastifyInstance) {
2421
app.post('/api/bot/analyze-pr', async (request: FastifyRequest<{ Body: BotPayload }>, reply) => {
25-
const { owner, repo, prNumber, baseSha, headSha, files } = request.body;
22+
const { owner, repo, baseSha, headSha, files } = request.body;
2623

27-
// Use the backend server's GitHub token for fetching raw code
2824
const token = process.env.GITHUB_TOKEN || '';
2925
const octokit = new Octokit({ auth: token });
3026

31-
let totalBaseCost = 0;
32-
let totalHeadCost = 0;
33-
const fileReports: any[] = [];
34-
35-
let inLoop = 0;
36-
let handler = 0;
27+
let totalBaseCost = 0;
28+
let totalHeadCost = 0;
29+
let inLoop = 0;
30+
let handler = 0;
31+
const fetchErrors: string[] = [];
32+
33+
// Per-detection breakdown rows: one row per AST detection in HEAD
34+
interface DetectionRow {
35+
service: string;
36+
model?: string;
37+
snippet: string;
38+
headCents: number; // absolute monthly cost in HEAD
39+
deltaCents: number; // change introduced by this PR
40+
inLoop: boolean;
41+
}
42+
const detectionRows: DetectionRow[] = [];
3743

3844
// --- DYNAMIC AST ANALYSIS ---
3945
for (const file of files) {
4046
if (file.status === 'removed') continue;
4147
const language = getLanguageFromFilename(file.filename);
4248
if (!language) continue;
4349

50+
// Fetch BASE content (what was there before the PR)
4451
let baseContent = '';
4552
if (file.status !== 'added') {
4653
try {
4754
const { data } = await octokit.rest.repos.getContent({ owner, repo, path: file.filename, ref: baseSha });
4855
if ('content' in (data as any) && !Array.isArray(data)) {
4956
baseContent = Buffer.from((data as any).content, 'base64').toString('utf-8');
5057
}
51-
} catch (e) { }
58+
} catch (e: any) {
59+
// Non-fatal: treat as empty (new file scenario)
60+
fetchErrors.push(`[base] ${file.filename}: ${e.message}`);
61+
}
5262
}
5363

64+
// Fetch HEAD content (what the PR introduces)
5465
let headContent = '';
5566
try {
5667
const { data } = await octokit.rest.repos.getContent({ owner, repo, path: file.filename, ref: headSha });
5768
if ('content' in (data as any) && !Array.isArray(data)) {
5869
headContent = Buffer.from((data as any).content, 'base64').toString('utf-8');
5970
}
60-
} catch (e) { continue; }
61-
62-
// Check for loops to trigger Criticality
63-
if (headContent.includes('for (') || headContent.includes('while (')) {
64-
inLoop += 1;
71+
} catch (e: any) {
72+
fetchErrors.push(`[head] ${file.filename}: ${e.message}`);
73+
continue; // Cannot analyze — skip this file
6574
}
6675

67-
// REAL AST COST CALCULATION
76+
if (!headContent.trim()) continue;
77+
78+
// Detect loop patterns in head file
79+
const fileHasLoop = /for\s*\(|while\s*\(|\.forEach\s*\(|\.map\s*\(/.test(headContent);
80+
if (fileHasLoop) inLoop += 1;
81+
82+
// Run AST cost diff
6883
const diffResult = await computeCostDiff(baseContent, headContent, language);
6984
totalBaseCost += diffResult.baseTotal;
7085
totalHeadCost += diffResult.headTotal;
7186

72-
// Extract snippet
73-
let snippet = 'await client.chat.completions.create(...)';
74-
const lines = headContent.split('\n');
75-
const apiLine = lines.find(l =>
76-
l.includes('openai') || l.includes('chat.completions') ||
77-
l.includes('aws') || l.includes('new Lambda') || l.includes('InvokeCommand')
78-
);
79-
if (apiLine) snippet = apiLine.trim();
80-
81-
if (diffResult.deltaCents !== 0 || diffResult.addedServices.length > 0 || diffResult.removedServices.length > 0) {
82-
// Pick the most informative snippet: prefer AST-extracted snippets, then fall back to line scan
83-
const astSnippet = diffResult.headLines[0]?.snippet || apiLine?.trim() || 'await client.chat.completions.create(...)';
84-
85-
fileReports.push({
86-
filename: file.filename,
87-
deltaCents: diffResult.deltaCents,
88-
added: diffResult.addedServices,
89-
removed: diffResult.removedServices,
90-
models: diffResult.headLines, // real per-detection service+model data
91-
snippet: astSnippet,
92-
});
87+
// ── Key fix: Report ALL detections in HEAD (not just services that changed) ──
88+
// headLines contains every cloud API call found in the PR's version of the file.
89+
if (diffResult.headLines.length > 0) {
9390
handler += 1;
91+
92+
// Compute per-detection cost delta: headCents proportional share of total delta
93+
const totalHeadFile = diffResult.headTotal || 1;
94+
const fileDelta = diffResult.deltaCents;
95+
96+
for (const line of diffResult.headLines) {
97+
const share = line.monthlyCents / totalHeadFile;
98+
const lineDelta = Math.round(fileDelta * share);
99+
100+
detectionRows.push({
101+
service: line.service,
102+
model: line.model,
103+
snippet: line.snippet,
104+
headCents: line.monthlyCents,
105+
deltaCents: lineDelta,
106+
inLoop: fileHasLoop,
107+
});
108+
}
94109
}
95110
}
96111

97-
// If no cloud cost patterns were detected, report a clean bill of health
98-
// (no fake fallback — real results only)
112+
// ── Build markdown report ──────────────────────────────────────────────────
99113

100-
const totalDelta = totalHeadCost - totalBaseCost;
101-
const formatDollar = (cents: number) => `$${(Math.abs(cents) / 100).toLocaleString('en-US', { minimumFractionDigits: 2 })}`;
102-
const sign = totalDelta > 0 ? '+' : totalDelta < 0 ? '-' : '';
114+
const totalDelta = totalHeadCost - totalBaseCost;
115+
const fmt = (cents: number) =>
116+
`$${(Math.abs(cents) / 100).toLocaleString('en-US', { minimumFractionDigits: 2 })}`;
117+
const sign = (cents: number) => cents > 0 ? '+' : cents < 0 ? '-' : '';
103118

104-
// Code Quality Billing Criticality
105-
let qualityStatement = "";
106-
let criticalityBadge = "**Criticality: Info**";
119+
// Criticality
120+
let criticalityBadge = '🟢 **Criticality: Low**';
121+
let qualityStatement = '';
107122

108123
if (inLoop > 0) {
109-
qualityStatement = "\n> **CRITICAL CODE QUALITY WARNING:** Cloud API calls detected inside a loop. This is an anti-pattern that causes exponentially multiplying cloud costs. Consider batching requests or pulling the API call outside the loop.";
110-
criticalityBadge = "**🔴 Criticality: Major**";
111-
} else if (totalDelta > 5000) {
112-
qualityStatement = "\n> **NOTICE:** This PR introduces significant new cloud infrastructure costs. Please ensure these additions align with your current billing budget.";
113-
criticalityBadge = "**🟡 Criticality: Minor**";
114-
} else {
115-
criticalityBadge = "**🟢 Criticality: Low**";
124+
criticalityBadge = '🔴 **Criticality: Major**';
125+
qualityStatement = '\n> ⚠️ **CRITICAL:** Cloud API calls detected inside a loop — costs scale with every iteration. Consider batching or moving calls outside the loop.\n';
126+
} else if (totalHeadCost > 5_000) {
127+
criticalityBadge = '🟡 **Criticality: Minor**';
128+
qualityStatement = '\n> 💡 **NOTICE:** Significant cloud costs detected in changed files. Ensure these align with your budget.\n';
116129
}
117130

131+
// Unique service count
132+
const uniqueServices = [...new Set(detectionRows.map(r => r.service))];
133+
118134
let markdown = `## 📊 CloudGauge Cost Impact Analysis\n\n`;
135+
136+
// ── Headline numbers ──
119137
markdown += `### 💰 ESTIMATED MONTHLY COST DELTA\n`;
120-
markdown += `# **${sign}${formatDollar(totalDelta)}/mo**\n`;
138+
markdown += `# **${sign(totalDelta)}${fmt(totalDelta)}/mo**\n`;
121139
markdown += `${criticalityBadge}\n\n`;
122-
markdown += `*Detected ${fileReports.length} cost-impacting pattern(s) across ${fileReports.length} service(s).*\n`;
123-
markdown += `${qualityStatement}\n\n`;
140+
markdown += `> Total cloud cost **in changed files**: **${fmt(totalHeadCost)}/mo**`;
141+
if (totalBaseCost > 0) markdown += ` *(was ${fmt(totalBaseCost)}/mo before this PR)*`;
142+
markdown += `\n\n`;
143+
markdown += `*Detected **${detectionRows.length}** cloud API call(s) across **${uniqueServices.length}** service(s).*\n`;
144+
markdown += qualityStatement + '\n';
124145
markdown += `---\n\n`;
146+
147+
// ── Execution context table ──
125148
markdown += `### ⚙️ EXECUTION CONTEXT IMPACT\n`;
126149
markdown += `| 🔄 In Loop | 🌐 Handler | ⏱️ Scheduled | 📦 Batch | 📌 Direct |\n`;
127150
markdown += `|:---:|:---:|:---:|:---:|:---:|\n`;
128151
markdown += `| **${inLoop}**<br>^(High Impact)^ | **${handler}**<br>^(Per Request)^ | **0**<br>^(Recurring)^ | **0**<br>^(Concurrent)^ | **0**<br>^(Baseline)^ |\n\n`;
129152
markdown += `---\n\n`;
130153

131-
if (fileReports.length > 0) {
154+
// ── Per-detection cost breakdown ──
155+
if (detectionRows.length > 0) {
132156
markdown += `### 📋 COST BREAKDOWN\n`;
133-
markdown += `| Service / Model | Detected Code Snippet | Monthly Delta |\n`;
134-
markdown += `|:---|:---|---:|\n`;
135-
for (const report of fileReports) {
136-
const sgn = report.deltaCents > 0 ? '+' : report.deltaCents < 0 ? '-' : '';
137-
const snippet = report.snippet || 'await client.chat.completions.create(...)';
138-
139-
// Build a precise service label from the actual detected services + models
140-
let serviceName: string;
141-
if (report.models && report.models.length > 0) {
142-
// Use actual model names returned by the AST diff engine
143-
serviceName = report.models
144-
.map((m: { service: string; model?: string }) =>
145-
m.model ? `**${m.service}** \`${m.model}\`` : `**${m.service}**`
146-
)
147-
.join(', ');
148-
} else if (report.added.includes('openai')) {
149-
serviceName = '**OpenAI**';
150-
} else if (report.added.includes('anthropic')) {
151-
serviceName = '**Anthropic**';
152-
} else if (report.added.includes('aws-lambda') || report.added.includes('aws')) {
153-
serviceName = '**AWS Lambda**';
154-
} else if (report.added.includes('s3')) {
155-
serviceName = '**AWS S3**';
156-
} else {
157-
serviceName = report.added.length ? `**${report.added.join(', ')}**` : '**Unknown**';
158-
}
159-
160-
markdown += `| ${serviceName} | \`${snippet}\` | **${sgn}${formatDollar(report.deltaCents)}/mo** |\n`;
157+
markdown += `| Service / Model | Detected Snippet | Est. Cost/mo | PR Delta |\n`;
158+
markdown += `|:---|:---|---:|---:|\n`;
159+
160+
for (const row of detectionRows) {
161+
const serviceLabel = row.model
162+
? `**${row.service}** \`${row.model}\``
163+
: `**${row.service}**`;
164+
const loopBadge = row.inLoop ? ' 🔄' : '';
165+
const deltaStr = row.deltaCents !== 0
166+
? `**${sign(row.deltaCents)}${fmt(row.deltaCents)}/mo**`
167+
: `*(existing)*`;
168+
169+
markdown += `| ${serviceLabel}${loopBadge} | \`${row.snippet.slice(0, 70)}\` | **${fmt(row.headCents)}/mo** | ${deltaStr} |\n`;
161170
}
162171
} else {
163-
markdown += `*No cost-impacting patterns detected in this PR.*\n`;
172+
markdown += `*No cloud API calls detected in the changed files.*\n`;
173+
174+
// Surface fetch errors if that's why we got nothing
175+
if (fetchErrors.length > 0) {
176+
markdown += `\n> ⚠️ **Note:** Some files could not be fetched (GITHUB_TOKEN may lack repo read access):\n`;
177+
for (const e of fetchErrors) markdown += `> - \`${e}\`\n`;
178+
}
164179
}
165180

166181
markdown += `\n\n> *Powered by SentinelEngine CodeReview Bot.*`;
167182

168183
return reply.send({
169184
markdown,
170-
totalDeltaCents: totalDelta
185+
totalDeltaCents: totalDelta,
171186
});
172187
});
173188
}

0 commit comments

Comments
 (0)