AltimateAI / AltimateAI/altimate-code

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

オープン
#1,242 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
TypeScript
スター
811
フォーク
134
平均マージ
3日 2時間
マージ済み PR(30日)
50

説明

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

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

評価

技術スタック
sql, typescript
領域
backend, data-engineering, observability
issue の種類
バグ
難易度
3/5
見積もり時間
1〜2日
活発さ
活発
明瞭さ
明確に書かれている
初心者へのやさしさ
72/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。