SQL execution telemetry can't distinguish failed-execution from never-executed

[anandgupta42]

What's the problem?

sql_execute's SQL-structure telemetry (sql_fingerprint) can't distinguish "a warehouse ran this query and it failed" from "this query never reached a warehouse at all."

sql.execute (via connections/register.ts) returns the same result shape — { ..., error: string } rather than throwing — for two very different situations:

1. A warehouse actually ran the query and it failed (bad SQL, permission error, connection dropped mid-query). This is exactly the kind of failure the fingerprint telemetry should capture — it's real SQL that a warehouse tried to execute.
2. The query never reached a warehouse at all — no warehouse configured, Registry.get() failed, connector setup/connection failed before any SQL ran.

packages/opencode/src/altimate/tools/sql-execute.ts's result-error branch (if (responseError !== undefined) { ... }) sees both cases identically. There is currently no field on the result that says which one happened.

History

This surfaced across three review rounds on PR #1238 (a follow-up to #1204):

• Round 1: the original ask was just "emit the fingerprint on the result-error branch too, not only on success" (a legitimate gap — failed executions were invisible to the telemetry).
• Round 2 review caught that the thrown-exception catch block (which only fires on a genuine non-execution failure, e.g. dispatcher down) was also being fingerprinted — double-counting/mislabeling never-executed queries as "failed execution."
• Round 3 review caught the deeper issue: even the result-error branch itself can't reliably claim "this was an execution" — register.ts returns that same shape for pre-execution failures.

At that point the fix had gone through three rounds trying to build a correct "was this actually executed" signal purely from the caller's side, without success — the information the caller needs doesn't exist yet at the point sql_execute receives the result.

Decision (PR #1238)

De-scoped to fingerprint-on-success-only — the behavior that predates all of this. It's honest (a fingerprinted query definitely executed) even though it's incomplete (executed-but-failed queries currently aren't captured). This is intentionally the smaller, clearly-correct change rather than building a failed-execution-vs-never-executed taxonomy inside a review-debt cleanup PR.

See the code comment at the result-error branch in packages/opencode/src/altimate/tools/sql-execute.ts (references this issue).

What the real fix needs

An explicit signal from the execution path itself, not something inferred from the result shape after the fact. Something like:

• An executed: boolean (or a small enum: not_attempted / executed / unknown) field on the result register.ts's sql.execute handler returns, set based on whether the code actually reached the point of calling connector.execute(...) — not just whether an error string is present.
• sql-execute.ts then fingerprints on success OR (result-error AND executed === true), and skips fingerprinting for executed === false.

Where to look

• packages/opencode/src/altimate/native/connections/register.ts (lines ~548–576 as of PR fix(drivers): recover #1204 review debt — driver-e2e false-skip, file: URI checks, Windows paths, telemetry, docs #1238) — the sql.execute handler that catches every connection/query error and returns the result-shaped { ..., error } object. This is where a real "did we reach a warehouse" signal would need to originate.
• packages/opencode/src/altimate/tools/sql-execute.ts — the sql_execute tool, where the fingerprint would then branch on that signal instead of guessing from the result shape.
• packages/opencode/test/altimate/telemetry-signals.test.ts — has the structural test asserting the current (success-only) behavior; would need extending once the executed-phase signal exists.