Skip to content

Commit 1a9c6fe

Browse files
anandgupta42claude
andauthored
feat: shadow-mode SQL pre-validation telemetry (#643)
* fix: resolve all 5 Verdaccio sanity test failures - altimate-core NAPI binding: set `NODE_PATH` to global npm root so `require('@altimateai/altimate-core')` resolves after `npm install -g` - upstream branding: replace "opencode" with "altimate-code" in user-facing `describe` strings (uninstall, tui, pr commands, config, server API docs) - driver resolvability: set `NODE_PATH` in driver check loop and install `duckdb` alongside the main package so at least one peer dep is present - hardcoded CI paths: restrict grep to JS/JSON files only — compiled Bun binaries embed build-machine paths in debug info which is unavoidable - NAPI module exports: already had correct `NODE_PATH` in extended test; root cause was the base test (fix 1) which is now resolved Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: pre-execution SQL validation, apply_patch retry, agent behavior rules, and error UX - Wire datafusion validation into sql_execute — catches column/table errors locally before hitting the warehouse (uses schema cache with 24h TTL) - Add sql_pre_validation telemetry event to measure catch rate and latency - Add apply_patch retry-with-re-read on verification failure — re-reads the file and retries once before giving up, with actionable error messages - Add file-not-found cache in read tool — prevents retry loops on missing paths (capped at 500 entries) - Add agent behavior rules to system prompt: act first/ask later, enforce read-before-edit, limit retries to 2 per input - Add actionable connection error guidance in warehouse_test — maps common auth failures (wrong password, missing key, SSO timeout) to fix instructions - Auto-pull schema cache in altimate_core_validate when no schema provided Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: ship SQL pre-validation in shadow mode, defer other fixes Reduces PR scope to telemetry-only based on deep analysis: the broader fixes (prompt rules, warehouse_test guidance, apply_patch retry, read file-not-found cache, altimate_core_validate auto-pull) were speculative against an 8-machine / 1-day telemetry sample. This PR now ships only what's needed to measure whether pre-execution SQL validation is worth it: - Keep: sql_pre_validation telemetry event + preValidateSql function - Change: pre-validation runs fire-and-forget (shadow mode) — emits telemetry with outcome=skipped|passed|blocked|error but never blocks sql_execute. Zero user-facing latency impact. - Revert: read.ts, apply_patch.ts, warehouse-test.ts, altimate-core-validate.ts, anthropic.txt system prompt changes — to be re-evaluated as separate PRs once real telemetry data validates need. After 2 weeks of shadow telemetry, we can decide whether the blocking behavior is worth the latency and false-positive risk. * fix: address cubic review feedback on SQL pre-validation - P1: mask validator error via `Telemetry.maskString()` before emitting `sql_pre_validation` telemetry. Raw schema identifiers (table/column names, paths) no longer leak to App Insights. - P2: resolve fallback warehouse via `Registry.list().warehouses[0]` (same path `sql.execute` uses) instead of the cache's first warehouse. Keeps shadow validation aligned with actual execution. - P2: raise column-scan cap from 10k to 500k and add `schema_truncated` boolean to the event. Avoids false structural errors on large warehouses and lets analysis flag biased samples. --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 56484c7 commit 1a9c6fe

File tree

2 files changed

+177
-0
lines changed

2 files changed

+177
-0
lines changed

packages/opencode/src/altimate/telemetry/index.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,22 @@ export namespace Telemetry {
637637
total_cost: number
638638
}
639639
// altimate_change end
640+
// altimate_change start — pre-execution SQL validation telemetry
641+
| {
642+
type: "sql_pre_validation"
643+
timestamp: number
644+
session_id: string
645+
/** skipped = no cache or stale, passed = valid SQL, blocked = invalid SQL caught, error = validation itself failed */
646+
outcome: "skipped" | "passed" | "blocked" | "error"
647+
/** why: no_cache, stale_cache, empty_cache, valid, non_structural, structural_error, validation_exception */
648+
reason: string
649+
schema_columns: number
650+
/** true when schema scan hit the column-scan cap — flags samples biased by large-warehouse truncation */
651+
schema_truncated: boolean
652+
duration_ms: number
653+
error_message?: string
654+
}
655+
// altimate_change end
640656

641657
/** SHA256 hash a masked error message for anonymous grouping. */
642658
export function hashError(maskedMessage: string): string {

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

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ import { Telemetry } from "../telemetry"
99
// altimate_change start — progressive disclosure suggestions
1010
import { PostConnectSuggestions } from "./post-connect-suggestions"
1111
// altimate_change end
12+
// altimate_change start — pre-execution SQL validation via cached schema
13+
import { getCache } from "../native/schema/cache"
14+
import * as Registry from "../native/connections/registry"
15+
// altimate_change end
1216

1317
export const SqlExecuteTool = Tool.define("sql_execute", {
1418
description: "Execute SQL against a connected data warehouse. Returns results as a formatted table.",
@@ -34,6 +38,14 @@ export const SqlExecuteTool = Tool.define("sql_execute", {
3438
}
3539
// altimate_change end
3640

41+
// altimate_change start — shadow-mode pre-execution SQL validation
42+
// Runs validation against cached schema and emits sql_pre_validation telemetry,
43+
// but does NOT block execution. Used to measure catch rate before deciding
44+
// whether to enable blocking in a future release. Fire-and-forget so it
45+
// doesn't add latency to the sql_execute hot path.
46+
preValidateSql(args.query, args.warehouse).catch(() => {})
47+
// altimate_change end
48+
3749
try {
3850
const result = await Dispatcher.call("sql.execute", {
3951
sql: args.query,
@@ -91,6 +103,155 @@ export const SqlExecuteTool = Tool.define("sql_execute", {
91103
},
92104
})
93105

106+
// altimate_change start — pre-execution SQL validation via cached schema
107+
const CACHE_TTL_MS = 24 * 60 * 60 * 1000 // 24 hours
108+
// High ceiling so large warehouses aren't arbitrarily truncated; we emit
109+
// schema_truncated in telemetry when the cap is reached so the shadow sample
110+
// can be interpreted correctly.
111+
const COLUMN_SCAN_LIMIT = 500_000
112+
113+
interface PreValidationResult {
114+
blocked: boolean
115+
error?: string
116+
}
117+
118+
async function preValidateSql(sql: string, warehouse?: string): Promise<PreValidationResult> {
119+
const startTime = Date.now()
120+
try {
121+
// Resolve the warehouse the same way sql.execute's fallback path does:
122+
// when caller omits `warehouse`, sql.execute uses Registry.list()[0].
123+
// Matching that here keeps the shadow validation aligned with actual
124+
// execution (dbt-routed queries are a known gap — they short-circuit
125+
// before this fallback, so validation may use a different warehouse
126+
// than the one dbt selects).
127+
let warehouseName = warehouse
128+
if (!warehouseName) {
129+
const registered = Registry.list().warehouses
130+
warehouseName = registered[0]?.name
131+
}
132+
if (!warehouseName) {
133+
trackPreValidation("skipped", "no_cache", 0, Date.now() - startTime, false)
134+
return { blocked: false }
135+
}
136+
137+
const cache = await getCache()
138+
const status = cache.cacheStatus()
139+
140+
const warehouseStatus = status.warehouses.find((w) => w.name === warehouseName)
141+
if (!warehouseStatus?.last_indexed) {
142+
trackPreValidation("skipped", "no_cache", 0, Date.now() - startTime, false)
143+
return { blocked: false }
144+
}
145+
146+
// Check cache freshness
147+
const cacheAge = Date.now() - new Date(warehouseStatus.last_indexed).getTime()
148+
if (cacheAge > CACHE_TTL_MS) {
149+
trackPreValidation("skipped", "stale_cache", 0, Date.now() - startTime, false)
150+
return { blocked: false }
151+
}
152+
153+
// Build schema context from cached columns
154+
const columns = cache.listColumns(warehouseName, COLUMN_SCAN_LIMIT)
155+
const schemaTruncated = columns.length >= COLUMN_SCAN_LIMIT
156+
if (columns.length === 0) {
157+
trackPreValidation("skipped", "empty_cache", 0, Date.now() - startTime, false)
158+
return { blocked: false }
159+
}
160+
161+
const schemaContext: Record<string, any> = {}
162+
for (const col of columns) {
163+
const tableName = col.schema_name ? `${col.schema_name}.${col.table}` : col.table
164+
if (!schemaContext[tableName]) {
165+
schemaContext[tableName] = []
166+
}
167+
schemaContext[tableName].push({
168+
name: col.name,
169+
type: col.data_type || "VARCHAR",
170+
nullable: col.nullable,
171+
})
172+
}
173+
174+
// Validate SQL against cached schema
175+
const validationResult = await Dispatcher.call("altimate_core.validate", {
176+
sql,
177+
schema_path: "",
178+
schema_context: schemaContext,
179+
})
180+
181+
const data = (validationResult.data ?? {}) as Record<string, any>
182+
const errors = Array.isArray(data.errors) ? data.errors : []
183+
const isValid = data.valid !== false && errors.length === 0
184+
185+
if (isValid) {
186+
trackPreValidation("passed", "valid", columns.length, Date.now() - startTime, schemaTruncated)
187+
return { blocked: false }
188+
}
189+
190+
// Only block on high-confidence structural errors
191+
const structuralErrors = errors.filter((e: any) => {
192+
const msg = (e.message ?? "").toLowerCase()
193+
return msg.includes("column") || msg.includes("table") || msg.includes("not found") || msg.includes("does not exist")
194+
})
195+
196+
if (structuralErrors.length === 0) {
197+
// Non-structural errors (ambiguous cases) — let them through
198+
trackPreValidation("passed", "non_structural", columns.length, Date.now() - startTime, schemaTruncated)
199+
return { blocked: false }
200+
}
201+
202+
// Build helpful error with available columns
203+
const errorMsgs = structuralErrors.map((e: any) => e.message).join("\n")
204+
const referencedTables = Object.keys(schemaContext).slice(0, 10)
205+
const availableColumns = referencedTables
206+
.map((t) => `${t}: ${schemaContext[t].map((c: any) => c.name).join(", ")}`)
207+
.join("\n")
208+
209+
const errorOutput = [
210+
`Pre-execution validation failed (validated against cached schema):`,
211+
``,
212+
errorMsgs,
213+
``,
214+
`Available tables and columns:`,
215+
availableColumns,
216+
``,
217+
`Fix the query and retry. If the schema cache is outdated, run schema_index to refresh it.`,
218+
].join("\n")
219+
220+
trackPreValidation("blocked", "structural_error", columns.length, Date.now() - startTime, schemaTruncated, errorMsgs)
221+
return { blocked: true, error: errorOutput }
222+
} catch {
223+
// Validation failure should never block execution
224+
trackPreValidation("error", "validation_exception", 0, Date.now() - startTime, false)
225+
return { blocked: false }
226+
}
227+
}
228+
229+
function trackPreValidation(
230+
outcome: "skipped" | "passed" | "blocked" | "error",
231+
reason: string,
232+
schema_columns: number,
233+
duration_ms: number,
234+
schema_truncated: boolean,
235+
error_message?: string,
236+
) {
237+
// Mask schema identifiers (table / column names, paths, user IDs) from the
238+
// validator error BEFORE it leaves the process — these are PII-adjacent and
239+
// must not land in App Insights as raw strings.
240+
const masked = error_message ? Telemetry.maskString(error_message).slice(0, 500) : undefined
241+
Telemetry.track({
242+
type: "sql_pre_validation",
243+
timestamp: Date.now(),
244+
session_id: Telemetry.getContext().sessionId,
245+
outcome,
246+
reason,
247+
schema_columns,
248+
schema_truncated,
249+
duration_ms,
250+
...(masked && { error_message: masked }),
251+
})
252+
}
253+
// altimate_change end
254+
94255
function formatResult(result: SqlExecuteResult): string {
95256
if (result.row_count === 0) return "(0 rows)"
96257

0 commit comments

Comments
 (0)