AltimateAI / AltimateAI/altimate-code

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

Open
#1,242 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
811
Forks
134
Avg merge
3d 2h
Merged PRs (30d)
50

Description

### 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 #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.

Contributor guide

Open the contributing guide

Research direction

Start in packages/opencode/src/altimate/native/connections/register.ts around the sql.execute handler and trace where connector.execute(...) is actually reached. Then update packages/opencode/src/altimate/tools/sql-execute.ts to use that explicit execution signal for fingerprinting failed results. Extend packages/opencode/test/altimate/telemetry-signals.test.ts so done means success and executed failures are fingerprinted, while never-executed failures are not.

Written by the indexing model from the issue text.

Assessment

Tech stack
sql, typescript
Domain
backend, data-engineering, observability
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.