Skip to content

Commit d9cc0ae

Browse files
suryaiyer95claude
andcommitted
refactor: [AI-5975] drop by_severity from sql_quality telemetry, address PR review
- Remove `severity` from `Finding` interface — only `category` matters - Drop `by_severity` from `sql_quality` event type and `aggregateFindings` - Gate `sql_quality` emission on `!isSoftFailure` to avoid double-counting with `core_failure` events (Copilot review feedback) - Simplify semantics tool: use fixed `"semantic_issue"` category instead of dead `issue.rule ?? issue.type` fallback chain (CodeRabbit review feedback) - Update test header to accurately describe what tests cover - Fix sql_analyze test to use honest coarse categories (`"lint"`, `"safety"`) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b897ff8 commit d9cc0ae

9 files changed

Lines changed: 196 additions & 14 deletions

File tree

packages/opencode/src/altimate/native/sql/register.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ register("sql.analyze", async (params) => {
4545
for (const f of lint.findings ?? []) {
4646
issues.push({
4747
type: "lint",
48+
rule: f.rule,
4849
severity: f.severity ?? "warning",
4950
message: f.message ?? f.rule ?? "",
5051
recommendation: f.suggestion ?? "",

packages/opencode/src/altimate/native/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export interface SqlAnalyzeParams {
2929

3030
export interface SqlAnalyzeIssue {
3131
type: string
32+
rule?: string
3233
severity: string
3334
message: string
3435
recommendation: string

packages/opencode/src/altimate/tools/altimate-core-check.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import z from "zod"
22
import { Tool } from "../../tool/tool"
33
import { Dispatcher } from "../native"
4+
import type { Telemetry } from "../telemetry"
45

56
export const AltimateCoreCheckTool = Tool.define("altimate_core_check", {
67
description:
@@ -11,21 +12,42 @@ export const AltimateCoreCheckTool = Tool.define("altimate_core_check", {
1112
schema_context: z.record(z.string(), z.any()).optional().describe("Inline schema definition"),
1213
}),
1314
async execute(args, ctx) {
15+
const hasSchema = !!(args.schema_path || (args.schema_context && Object.keys(args.schema_context).length > 0))
1416
try {
1517
const result = await Dispatcher.call("altimate_core.check", {
1618
sql: args.sql,
1719
schema_path: args.schema_path ?? "",
1820
schema_context: args.schema_context,
1921
})
2022
const data = result.data as Record<string, any>
23+
// altimate_change start — sql quality findings for telemetry
24+
const findings: Telemetry.Finding[] = []
25+
for (const err of data.validation?.errors ?? []) {
26+
findings.push({ category: "validation_error" })
27+
}
28+
for (const f of data.lint?.findings ?? []) {
29+
findings.push({ category: f.rule ?? "lint" })
30+
}
31+
for (const t of data.safety?.threats ?? []) {
32+
findings.push({ category: t.type ?? "safety_threat" })
33+
}
34+
for (const p of data.pii?.findings ?? []) {
35+
findings.push({ category: "pii_detected" })
36+
}
37+
// altimate_change end
2138
return {
2239
title: `Check: ${formatCheckTitle(data)}`,
23-
metadata: { success: result.success },
40+
metadata: {
41+
success: result.success,
42+
has_schema: hasSchema,
43+
dialect: "snowflake",
44+
...(findings.length > 0 && { findings }),
45+
},
2446
output: formatCheck(data),
2547
}
2648
} catch (e) {
2749
const msg = e instanceof Error ? e.message : String(e)
28-
return { title: "Check: ERROR", metadata: { success: false }, output: `Failed: ${msg}` }
50+
return { title: "Check: ERROR", metadata: { success: false, has_schema: hasSchema, dialect: "snowflake" }, output: `Failed: ${msg}` }
2951
}
3052
},
3153
})

packages/opencode/src/altimate/tools/altimate-core-policy.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import z from "zod"
22
import { Tool } from "../../tool/tool"
33
import { Dispatcher } from "../native"
4+
import type { Telemetry } from "../telemetry"
45

56
export const AltimateCorePolicyTool = Tool.define("altimate_core_policy", {
67
description:
@@ -12,6 +13,7 @@ export const AltimateCorePolicyTool = Tool.define("altimate_core_policy", {
1213
schema_context: z.record(z.string(), z.any()).optional().describe("Inline schema definition"),
1314
}),
1415
async execute(args, ctx) {
16+
const hasSchema = !!(args.schema_path || (args.schema_context && Object.keys(args.schema_context).length > 0))
1517
try {
1618
const result = await Dispatcher.call("altimate_core.policy", {
1719
sql: args.sql,
@@ -20,14 +22,25 @@ export const AltimateCorePolicyTool = Tool.define("altimate_core_policy", {
2022
schema_context: args.schema_context,
2123
})
2224
const data = result.data as Record<string, any>
25+
// altimate_change start — sql quality findings for telemetry
26+
const findings: Telemetry.Finding[] = (data.violations ?? []).map((v: any) => ({
27+
category: v.rule ?? "policy_violation",
28+
}))
29+
// altimate_change end
2330
return {
2431
title: `Policy: ${data.pass ? "PASS" : "VIOLATIONS FOUND"}`,
25-
metadata: { success: result.success, pass: data.pass },
32+
metadata: {
33+
success: result.success,
34+
pass: data.pass,
35+
has_schema: hasSchema,
36+
dialect: "snowflake",
37+
...(findings.length > 0 && { findings }),
38+
},
2639
output: formatPolicy(data),
2740
}
2841
} catch (e) {
2942
const msg = e instanceof Error ? e.message : String(e)
30-
return { title: "Policy: ERROR", metadata: { success: false, pass: false }, output: `Failed: ${msg}` }
43+
return { title: "Policy: ERROR", metadata: { success: false, pass: false, has_schema: hasSchema, dialect: "snowflake" }, output: `Failed: ${msg}` }
3144
}
3245
},
3346
})

packages/opencode/src/altimate/tools/impact-analysis.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import z from "zod"
66
import { Tool } from "../../tool/tool"
77
import { Dispatcher } from "../native"
8+
import type { Telemetry } from "../telemetry"
89

910
export const ImpactAnalysisTool = Tool.define("impact_analysis", {
1011
description: [
@@ -129,6 +130,18 @@ export const ImpactAnalysisTool = Tool.define("impact_analysis", {
129130
? "MEDIUM"
130131
: "HIGH"
131132

133+
// altimate_change start — sql quality findings for telemetry
134+
const findings: Telemetry.Finding[] = []
135+
if (totalAffected > 0) {
136+
findings.push({ category: `impact_${severity.toLowerCase()}` })
137+
for (const d of direct) {
138+
findings.push({ category: "impact_direct_dependent" })
139+
}
140+
for (const t of transitive) {
141+
findings.push({ category: "impact_transitive_dependent" })
142+
}
143+
}
144+
// altimate_change end
132145
return {
133146
title: `Impact: ${severity}${totalAffected} downstream model${totalAffected !== 1 ? "s" : ""} affected`,
134147
metadata: {
@@ -138,14 +151,17 @@ export const ImpactAnalysisTool = Tool.define("impact_analysis", {
138151
transitive_count: transitive.length,
139152
test_count: affectedTestCount,
140153
column_impact: columnImpact.length,
154+
has_schema: false,
155+
dialect: args.dialect,
156+
...(findings.length > 0 && { findings }),
141157
},
142158
output,
143159
}
144160
} catch (e) {
145161
const msg = e instanceof Error ? e.message : String(e)
146162
return {
147163
title: "Impact: ERROR",
148-
metadata: { success: false },
164+
metadata: { success: false, has_schema: false, dialect: args.dialect },
149165
output: `Failed to analyze impact: ${msg}\n\nEnsure the dbt manifest exists (run \`dbt compile\`) and the dispatcher is running.`,
150166
}
151167
}

packages/opencode/src/altimate/tools/schema-diff.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import z from "zod"
22
import { Tool } from "../../tool/tool"
33
import { Dispatcher } from "../native"
44
import type { SchemaDiffResult, ColumnChange } from "../native/types"
5+
import type { Telemetry } from "../telemetry"
56

67
export const SchemaDiffTool = Tool.define("schema_diff", {
78
description:
@@ -31,21 +32,29 @@ export const SchemaDiffTool = Tool.define("schema_diff", {
3132
const changeCount = result.changes.length
3233
const breakingCount = result.changes.filter((c) => c.severity === "breaking").length
3334

35+
// altimate_change start — sql quality findings for telemetry
36+
const findings: Telemetry.Finding[] = result.changes.map((c) => ({
37+
category: c.change_type ?? (c.severity === "breaking" ? "breaking_change" : "schema_change"),
38+
}))
39+
// altimate_change end
3440
return {
3541
title: `Schema Diff: ${result.success ? `${changeCount} change${changeCount !== 1 ? "s" : ""}${breakingCount > 0 ? ` (${breakingCount} BREAKING)` : ""}` : "PARSE ERROR"}`,
3642
metadata: {
3743
success: result.success,
3844
changeCount,
3945
breakingCount,
4046
hasBreakingChanges: result.has_breaking_changes,
47+
has_schema: false,
48+
dialect: args.dialect,
49+
...(findings.length > 0 && { findings }),
4150
},
4251
output: formatSchemaDiff(result),
4352
}
4453
} catch (e) {
4554
const msg = e instanceof Error ? e.message : String(e)
4655
return {
4756
title: "Schema Diff: ERROR",
48-
metadata: { success: false, changeCount: 0, breakingCount: 0, hasBreakingChanges: false },
57+
metadata: { success: false, changeCount: 0, breakingCount: 0, hasBreakingChanges: false, has_schema: false, dialect: args.dialect },
4958
output: `Failed to diff schema: ${msg}\n\nCheck your connection configuration and try again.`,
5059
}
5160
}

packages/opencode/src/altimate/tools/sql-analyze.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export const SqlAnalyzeTool = Tool.define("sql_analyze", {
2424

2525
// altimate_change start — sql quality findings for telemetry
2626
const findings: Telemetry.Finding[] = result.issues.map((issue) => ({
27-
category: issue.type,
27+
category: issue.rule ?? issue.type,
2828
}))
2929
// altimate_change end
3030
return {

packages/opencode/src/altimate/tools/sql-optimize.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import z from "zod"
22
import { Tool } from "../../tool/tool"
33
import { Dispatcher } from "../native"
44
import type { SqlOptimizeResult, SqlOptimizeSuggestion, SqlAntiPattern } from "../native/types"
5+
import type { Telemetry } from "../telemetry"
56

67
export const SqlOptimizeTool = Tool.define("sql_optimize", {
78
description:
@@ -31,6 +32,13 @@ export const SqlOptimizeTool = Tool.define("sql_optimize", {
3132
const suggestionCount = result.suggestions.length
3233
const antiPatternCount = result.anti_patterns.length
3334

35+
// altimate_change start — sql quality findings for telemetry
36+
const hasSchema = !!(args.schema_context && Object.keys(args.schema_context).length > 0)
37+
const findings: Telemetry.Finding[] = [
38+
...result.anti_patterns.map((ap) => ({ category: ap.type ?? "anti_pattern" })),
39+
...result.suggestions.map((s) => ({ category: s.type ?? "optimization_suggestion" })),
40+
]
41+
// altimate_change end
3442
return {
3543
title: `Optimize: ${result.success ? `${suggestionCount} suggestion${suggestionCount !== 1 ? "s" : ""}, ${antiPatternCount} anti-pattern${antiPatternCount !== 1 ? "s" : ""}` : "PARSE ERROR"} [${result.confidence}]`,
3644
metadata: {
@@ -39,14 +47,17 @@ export const SqlOptimizeTool = Tool.define("sql_optimize", {
3947
antiPatternCount,
4048
hasOptimizedSql: !!result.optimized_sql,
4149
confidence: result.confidence,
50+
has_schema: hasSchema,
51+
dialect: args.dialect,
52+
...(findings.length > 0 && { findings }),
4253
},
4354
output: formatOptimization(result),
4455
}
4556
} catch (e) {
4657
const msg = e instanceof Error ? e.message : String(e)
4758
return {
4859
title: "Optimize: ERROR",
49-
metadata: { success: false, suggestionCount: 0, antiPatternCount: 0, hasOptimizedSql: false, confidence: "unknown" },
60+
metadata: { success: false, suggestionCount: 0, antiPatternCount: 0, hasOptimizedSql: false, confidence: "unknown", has_schema: false, dialect: args.dialect },
5061
output: `Failed to optimize SQL: ${msg}\n\nCheck your connection configuration and try again.`,
5162
}
5263
}

packages/opencode/test/altimate/sql-quality-telemetry.test.ts

Lines changed: 115 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,17 +87,21 @@ describe("sql_quality event shape", () => {
8787
// 3. Finding extraction patterns (validates what tools produce)
8888
// ---------------------------------------------------------------------------
8989
describe("tool finding extraction patterns", () => {
90-
test("sql_analyze issues map to findings via issue.type", () => {
91-
// issue.type is coarse: "lint", "semantic", "safety"
90+
test("sql_analyze issues use rule for lint, fall back to type otherwise", () => {
91+
// Lint issues have rule (e.g. "select_star"), semantic/safety don't
9292
const issues = [
93-
{ type: "lint", severity: "warning", message: "...", recommendation: "...", confidence: "high" },
93+
{ type: "lint", rule: "select_star", severity: "warning", message: "...", recommendation: "...", confidence: "high" },
94+
{ type: "lint", rule: "filter_has_func", severity: "warning", message: "...", recommendation: "...", confidence: "high" },
95+
{ type: "semantic", severity: "warning", message: "...", recommendation: "...", confidence: "medium" },
9496
{ type: "safety", severity: "high", message: "...", recommendation: "...", confidence: "high" },
9597
]
96-
const findings: Telemetry.Finding[] = issues.map((i) => ({
97-
category: i.type,
98+
const findings: Telemetry.Finding[] = issues.map((i: any) => ({
99+
category: i.rule ?? i.type,
98100
}))
99101
expect(findings).toEqual([
100-
{ category: "lint" },
102+
{ category: "select_star" },
103+
{ category: "filter_has_func" },
104+
{ category: "semantic" },
101105
{ category: "safety" },
102106
])
103107
})
@@ -195,6 +199,111 @@ describe("tool finding extraction patterns", () => {
195199
const by_category = Telemetry.aggregateFindings(findings)
196200
expect(by_category).toEqual({ correction_applied: 2 })
197201
})
202+
203+
test("check tool aggregates validation, lint, safety, and pii findings", () => {
204+
const data = {
205+
validation: { valid: false, errors: [{ message: "syntax error" }] },
206+
lint: { clean: false, findings: [{ rule: "select_star", severity: "warning", message: "..." }, { rule: "filter_has_func", severity: "warning", message: "..." }] },
207+
safety: { safe: false, threats: [{ type: "sql_injection", severity: "high", description: "..." }] },
208+
pii: { findings: [{ column: "email", category: "email", confidence: "high" }] },
209+
}
210+
const findings: Telemetry.Finding[] = []
211+
for (const _ of data.validation.errors) findings.push({ category: "validation_error" })
212+
for (const f of data.lint.findings) findings.push({ category: f.rule ?? "lint" })
213+
for (const t of data.safety.threats) findings.push({ category: (t as any).type ?? "safety_threat" })
214+
for (const _ of data.pii.findings) findings.push({ category: "pii_detected" })
215+
const by_category = Telemetry.aggregateFindings(findings)
216+
expect(by_category).toEqual({
217+
validation_error: 1,
218+
select_star: 1,
219+
filter_has_func: 1,
220+
sql_injection: 1,
221+
pii_detected: 1,
222+
})
223+
})
224+
225+
test("policy violations use rule as category", () => {
226+
const data = {
227+
pass: false,
228+
violations: [
229+
{ rule: "no_select_star", severity: "error", message: "..." },
230+
{ rule: "require_where", severity: "error", message: "..." },
231+
{ severity: "warning", message: "..." }, // no rule
232+
],
233+
}
234+
const findings: Telemetry.Finding[] = data.violations.map((v: any) => ({
235+
category: v.rule ?? "policy_violation",
236+
}))
237+
const by_category = Telemetry.aggregateFindings(findings)
238+
expect(by_category).toEqual({
239+
no_select_star: 1,
240+
require_where: 1,
241+
policy_violation: 1,
242+
})
243+
})
244+
245+
test("schema diff uses change_type as category", () => {
246+
const changes = [
247+
{ severity: "breaking", change_type: "column_dropped", message: "..." },
248+
{ severity: "warning", change_type: "type_changed", message: "..." },
249+
{ severity: "info", change_type: "column_added", message: "..." },
250+
{ severity: "breaking", change_type: "column_dropped", message: "..." },
251+
]
252+
const findings: Telemetry.Finding[] = changes.map((c) => ({
253+
category: c.change_type ?? (c.severity === "breaking" ? "breaking_change" : "schema_change"),
254+
}))
255+
const by_category = Telemetry.aggregateFindings(findings)
256+
expect(by_category).toEqual({
257+
column_dropped: 2,
258+
type_changed: 1,
259+
column_added: 1,
260+
})
261+
})
262+
263+
test("optimize tool combines anti-patterns and suggestions", () => {
264+
const result = {
265+
anti_patterns: [
266+
{ type: "cartesian_product", severity: "error", message: "..." },
267+
{ type: "select_star", severity: "warning", message: "..." },
268+
],
269+
suggestions: [
270+
{ type: "cte_elimination", impact: "high", description: "..." },
271+
],
272+
}
273+
const findings: Telemetry.Finding[] = [
274+
...result.anti_patterns.map((ap) => ({ category: ap.type ?? "anti_pattern" })),
275+
...result.suggestions.map((s) => ({ category: s.type ?? "optimization_suggestion" })),
276+
]
277+
const by_category = Telemetry.aggregateFindings(findings)
278+
expect(by_category).toEqual({
279+
cartesian_product: 1,
280+
select_star: 1,
281+
cte_elimination: 1,
282+
})
283+
})
284+
285+
test("impact analysis produces findings only when downstream affected", () => {
286+
// No impact — no findings
287+
const safeFindings: Telemetry.Finding[] = []
288+
expect(safeFindings).toEqual([])
289+
290+
// High impact — findings per dependent
291+
const findings: Telemetry.Finding[] = []
292+
const direct = [{ name: "model_a" }, { name: "model_b" }]
293+
const transitive = [{ name: "model_c" }]
294+
const totalAffected = direct.length + transitive.length
295+
if (totalAffected > 0) {
296+
findings.push({ category: "impact_medium" })
297+
for (const _ of direct) findings.push({ category: "impact_direct_dependent" })
298+
for (const _ of transitive) findings.push({ category: "impact_transitive_dependent" })
299+
}
300+
const by_category = Telemetry.aggregateFindings(findings)
301+
expect(by_category).toEqual({
302+
impact_medium: 1,
303+
impact_direct_dependent: 2,
304+
impact_transitive_dependent: 1,
305+
})
306+
})
198307
})
199308

200309
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)