Skip to content

Commit d355511

Browse files
committed
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.
1 parent 5c9eeda commit d355511

6 files changed

Lines changed: 11 additions & 149 deletions

File tree

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

Lines changed: 2 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@ import z from "zod"
22
import { Tool } from "../../tool/tool"
33
import { Dispatcher } from "../native"
44
import type { Telemetry } from "../telemetry"
5-
// altimate_change start — auto-pull schema from cache when not provided
6-
import { getCache } from "../native/schema/cache"
7-
// altimate_change end
85

96
export const AltimateCoreValidateTool = Tool.define("altimate_core_validate", {
107
description:
@@ -15,20 +12,11 @@ export const AltimateCoreValidateTool = Tool.define("altimate_core_validate", {
1512
schema_context: z.record(z.string(), z.any()).optional().describe("Inline schema definition"),
1613
}),
1714
async execute(args, ctx) {
18-
let hasSchema = !!(args.schema_path || (args.schema_context && Object.keys(args.schema_context).length > 0))
19-
// altimate_change start — auto-pull schema from cache when not provided
20-
if (!hasSchema) {
21-
const cachedSchema = await tryGetSchemaFromCache()
22-
if (cachedSchema) {
23-
args = { ...args, schema_context: cachedSchema }
24-
hasSchema = true
25-
}
26-
}
27-
// altimate_change end
15+
const hasSchema = !!(args.schema_path || (args.schema_context && Object.keys(args.schema_context).length > 0))
2816
const noSchema = !hasSchema
2917
if (noSchema) {
3018
const error =
31-
"No schema provided. Provide schema_context or schema_path so table/column references can be resolved. Tip: run schema_index first to cache your warehouse schema."
19+
"No schema provided. Provide schema_context or schema_path so table/column references can be resolved."
3220
return {
3321
title: "Validate: NO SCHEMA",
3422
metadata: { success: false, valid: false, has_schema: false, error },
@@ -89,41 +77,6 @@ function classifyValidationError(message: string): string {
8977
return "validation_error"
9078
}
9179

92-
// altimate_change start — auto-pull schema from cache when not provided
93-
const CACHE_TTL_MS = 24 * 60 * 60 * 1000
94-
95-
async function tryGetSchemaFromCache(): Promise<Record<string, any> | null> {
96-
try {
97-
const cache = await getCache()
98-
const status = cache.cacheStatus()
99-
const warehouse = status.warehouses[0]
100-
if (!warehouse?.last_indexed) return null
101-
102-
const cacheAge = Date.now() - new Date(warehouse.last_indexed).getTime()
103-
if (cacheAge > CACHE_TTL_MS) return null
104-
105-
const columns = cache.listColumns(warehouse.name, 10_000)
106-
if (columns.length === 0) return null
107-
108-
const schemaContext: Record<string, any> = {}
109-
for (const col of columns) {
110-
const tableName = col.schema_name ? `${col.schema_name}.${col.table}` : col.table
111-
if (!schemaContext[tableName]) {
112-
schemaContext[tableName] = []
113-
}
114-
schemaContext[tableName].push({
115-
name: col.name,
116-
type: col.data_type || "VARCHAR",
117-
nullable: col.nullable,
118-
})
119-
}
120-
return schemaContext
121-
} catch {
122-
return null
123-
}
124-
}
125-
// altimate_change end
126-
12780
function formatValidate(data: Record<string, any>): string {
12881
if (data.error) return `Error: ${data.error}`
12982
if (data.valid) return "SQL is valid."

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,12 @@ export const SqlExecuteTool = Tool.define("sql_execute", {
3737
}
3838
// altimate_change end
3939

40-
// altimate_change start — pre-execution SQL validation via cached schema
41-
const preValidation = await preValidateSql(args.query, args.warehouse)
42-
if (preValidation.blocked) {
43-
return {
44-
title: "SQL: VALIDATION ERROR",
45-
metadata: { rowCount: 0, truncated: false, error: preValidation.error },
46-
output: preValidation.error!,
47-
}
48-
}
40+
// altimate_change start — shadow-mode pre-execution SQL validation
41+
// Runs validation against cached schema and emits sql_pre_validation telemetry,
42+
// but does NOT block execution. Used to measure catch rate before deciding
43+
// whether to enable blocking in a future release. Fire-and-forget so it
44+
// doesn't add latency to the sql_execute hot path.
45+
preValidateSql(args.query, args.warehouse).catch(() => {})
4946
// altimate_change end
5047

5148
try {

packages/opencode/src/altimate/tools/warehouse-test.ts

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,10 @@ export const WarehouseTestTool = Tool.define("warehouse_test", {
2020
}
2121
}
2222

23-
// altimate_change start — actionable error guidance for common auth failures
24-
const errorDetail = result.error ?? "Unknown error"
25-
const guidance = getConnectionGuidance(errorDetail)
26-
// altimate_change end
2723
return {
2824
title: `Connection '${args.name}': FAILED`,
2925
metadata: { connected: false },
30-
output: `Failed to connect to warehouse '${args.name}'.\nError: ${errorDetail}${guidance}`,
26+
output: `Failed to connect to warehouse '${args.name}'.\nError: ${result.error ?? "Unknown error"}`,
3127
}
3228
} catch (e) {
3329
const msg = e instanceof Error ? e.message : String(e)
@@ -39,36 +35,3 @@ export const WarehouseTestTool = Tool.define("warehouse_test", {
3935
}
4036
},
4137
})
42-
43-
// altimate_change start — actionable error guidance for common auth failures
44-
function getConnectionGuidance(error: string): string {
45-
const lower = error.toLowerCase()
46-
47-
if (lower.includes("password") && (lower.includes("incorrect") || lower.includes("authentication failed"))) {
48-
return "\n\nHow to fix: Check the password in your connection config. Verify the username has access from your current IP address. Use `warehouse_remove` then `warehouse_add` to re-enter credentials."
49-
}
50-
if (lower.includes("password must be a string") || lower.includes("scram")) {
51-
return "\n\nHow to fix: The password field is missing or not a string. Check your connection config — the password may be empty or set to a non-string value. Use `warehouse_remove` then `warehouse_add` to re-configure."
52-
}
53-
if (lower.includes("private key") || lower.includes("decrypt")) {
54-
return "\n\nHow to fix: Key pair authentication failed. Verify: (1) the key file is PEM/PKCS#8 format, (2) the passphrase is correct, (3) the key has not expired, (4) the public key is registered in your warehouse."
55-
}
56-
if (lower.includes("missing") && lower.includes("password")) {
57-
return "\n\nHow to fix: No password was provided. Use `warehouse_remove` then `warehouse_add` to configure credentials. For Snowflake, you can also use key pair or SSO authentication."
58-
}
59-
if (lower.includes("browser") && lower.includes("timed out")) {
60-
return "\n\nHow to fix: SSO browser authentication timed out. Ensure your default browser opened the auth page. If running in a headless environment, switch to password or key pair authentication instead."
61-
}
62-
if (lower.includes("not installed") || lower.includes("cannot find module")) {
63-
return "\n\nHow to fix: The database driver is not installed. Run `npm install` with the appropriate driver package for your database type."
64-
}
65-
if (lower.includes("econnrefused") || lower.includes("enotfound")) {
66-
return "\n\nHow to fix: Cannot reach the database server. Check: (1) the hostname and port are correct, (2) the server is running, (3) any firewalls or VPNs are configured to allow the connection."
67-
}
68-
if (lower.includes("schema") && (lower.includes("does not exist") || lower.includes("not authorized"))) {
69-
return "\n\nHow to fix: The specified schema does not exist or your user lacks access. Check: (1) the schema name is spelled correctly, (2) your user/role has USAGE privilege on the schema."
70-
}
71-
72-
return ""
73-
}
74-
// altimate_change end

packages/opencode/src/session/prompt/anthropic.txt

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -69,27 +69,9 @@ I've found some existing telemetry code. Let me mark the first todo as in_progre
6969

7070
# Doing tasks
7171
The user will primarily request you perform software engineering tasks. This includes solving bugs, adding new functionality, refactoring code, explaining code, and more. For these tasks the following steps are recommended:
72-
-
72+
-
7373
- Use the TodoWrite tool to plan the task if required
7474

75-
<!-- altimate_change start — bias toward action, reduce over-questioning, enforce read-before-edit -->
76-
# Agent behavior rules
77-
78-
## Act first, ask later
79-
- Start working immediately. Do NOT ask clarifying questions unless the request is genuinely ambiguous with multiple incompatible interpretations.
80-
- If you can make a reasonable assumption, make it and state what you assumed. The user can correct you.
81-
- Prefer taking a small action (scan the project, read a file, check the schema) over asking the user what to do.
82-
83-
## Always read before editing
84-
- NEVER call edit, write, or apply_patch on a file you have not read in the current session.
85-
- If apply_patch fails with "Failed to find expected lines", re-read the file immediately and generate a new patch from the fresh content. Do NOT retry the same patch.
86-
- After a file_not_found error on a specific path, do NOT attempt to read that same path again. Move on or ask the user.
87-
88-
## Limit retries
89-
- If a tool call fails twice on the same input, do NOT retry a third time. Instead, explain what went wrong and try an alternative approach.
90-
- If apply_patch or edit fails on the same file twice, re-read the file, then try once more with fresh content. If that also fails, stop and report the issue.
91-
<!-- altimate_change end -->
92-
9375
- Tool results and user messages may include <system-reminder> tags. <system-reminder> tags contain useful information and reminders. They are automatically added by the system, and bear no direct relation to the specific tool results or user messages in which they appear.
9476

9577

packages/opencode/src/tool/apply_patch.ts

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -100,30 +100,13 @@ export const ApplyPatchTool = Tool.define("apply_patch", {
100100
const oldContent = await fs.readFile(filePath, "utf-8")
101101
let newContent = oldContent
102102

103-
// altimate_change start — retry patch application with re-read on mismatch
104103
// Apply the update chunks to get new content
105104
try {
106105
const fileUpdate = Patch.deriveNewContentsFromChunks(filePath, hunk.chunks)
107106
newContent = fileUpdate.content
108107
} catch (error) {
109-
// Re-read the file and retry once — the file may have changed since the LLM last saw it
110-
const freshContent = await fs.readFile(filePath, "utf-8")
111-
if (freshContent !== oldContent) {
112-
try {
113-
const retryUpdate = Patch.deriveNewContentsFromChunks(filePath, hunk.chunks)
114-
newContent = retryUpdate.content
115-
} catch (retryError) {
116-
throw new Error(
117-
`apply_patch verification failed: ${retryError}\n\nThe file has been modified since you last read it. Please re-read the file and generate a new patch.`,
118-
)
119-
}
120-
} else {
121-
throw new Error(
122-
`apply_patch verification failed: ${error}\n\nThe expected lines were not found in the file. Please re-read the file and generate a new patch based on its current contents.`,
123-
)
124-
}
108+
throw new Error(`apply_patch verification failed: ${error}`)
125109
}
126-
// altimate_change end
127110

128111
const diff = trimDiff(createTwoFilesPatch(filePath, filePath, oldContent, newContent))
129112

packages/opencode/src/tool/read.ts

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,6 @@ import { assertExternalDirectory } from "./external-directory"
1212
import { InstructionPrompt } from "../session/instruction"
1313
import { Filesystem } from "../util/filesystem"
1414

15-
// altimate_change start — cache file-not-found paths to prevent retry loops
16-
const _notFoundCache = new Set<string>()
17-
const NOT_FOUND_CACHE_MAX = 500
18-
// altimate_change end
19-
2015
const DEFAULT_READ_LIMIT = 2000
2116
const MAX_LINE_LENGTH = 2000
2217
const MAX_LINE_SUFFIX = `... (line truncated to ${MAX_LINE_LENGTH} chars)`
@@ -55,17 +50,6 @@ export const ReadTool = Tool.define("read", {
5550
})
5651

5752
if (!stat) {
58-
// altimate_change start — cache file-not-found to prevent retry loops
59-
if (_notFoundCache.has(filepath)) {
60-
throw new Error(
61-
`File not found: ${filepath}\n\nThis path was already checked and does not exist. Do NOT retry. Use a different path or ask the user for the correct location.`,
62-
)
63-
}
64-
if (_notFoundCache.size < NOT_FOUND_CACHE_MAX) {
65-
_notFoundCache.add(filepath)
66-
}
67-
// altimate_change end
68-
6953
const dir = path.dirname(filepath)
7054
const base = path.basename(filepath)
7155

0 commit comments

Comments
 (0)