diff --git a/packages/opencode/src/altimate/native/altimate-core.ts b/packages/opencode/src/altimate/native/altimate-core.ts index a41296ee71..137e28d848 100644 --- a/packages/opencode/src/altimate/native/altimate-core.ts +++ b/packages/opencode/src/altimate/native/altimate-core.ts @@ -320,7 +320,13 @@ export function registerAll(): void { register("altimate_core.equivalence", async (params) => { try { const schema = schemaOrEmpty(params.schema_path, params.schema_context) - const raw = await core.checkEquivalence(params.sql1, params.sql2, schema) + // Pass the optional dialect hint so dialect-specific compiled warehouse SQL + // (e.g. Snowflake semi-structured `col:field`) parses and the pair is + // decidable instead of abstaining on a syntax error. Supported since + // altimate-core@0.5.1. Use `|| undefined` (not `??`) so the default empty + // string from ReviewConfig.dialect coerces to "no hint": the engine throws + // on an unknown dialect "", and "" must mean auto-detect, not a real dialect. + const raw = await core.checkEquivalence(params.sql1, params.sql2, schema, params.dialect || undefined) const data = toData(raw) return ok(true, data) } catch (e) { @@ -332,7 +338,7 @@ export function registerAll(): void { register("altimate_core.migration", async (params) => { try { // Build schema from old_ddl, analyze new_ddl against it - const schema = core.Schema.fromDdl(params.old_ddl, params.dialect || undefined) + const schema = core.Schema.fromDdl(params.old_ddl, params.dialect ?? undefined) const raw = core.analyzeMigration(params.new_ddl, schema) const data = toData(raw) return ok(true, data) @@ -426,7 +432,7 @@ export function registerAll(): void { register("altimate_core.column_lineage", async (params) => { try { const schema = resolveSchema(params.schema_path, params.schema_context) - const raw = core.columnLineage(params.sql, params.dialect || undefined, schema ?? undefined) + const raw = core.columnLineage(params.sql, params.dialect ?? undefined, schema ?? undefined) return ok(true, toData(raw)) } catch (e) { return fail(e) @@ -447,7 +453,7 @@ export function registerAll(): void { // 22. altimate_core.format register("altimate_core.format", async (params) => { try { - const raw = core.formatSql(params.sql, params.dialect || undefined) + const raw = core.formatSql(params.sql, params.dialect ?? undefined) const data = toData(raw) return ok(true, data) } catch (e) { @@ -458,7 +464,7 @@ export function registerAll(): void { // 23. altimate_core.metadata register("altimate_core.metadata", async (params) => { try { - const raw = core.extractMetadata(params.sql, params.dialect || undefined) + const raw = core.extractMetadata(params.sql, params.dialect ?? undefined) return ok(true, toData(raw)) } catch (e) { return fail(e) @@ -468,7 +474,7 @@ export function registerAll(): void { // 24. altimate_core.compare register("altimate_core.compare", async (params) => { try { - const raw = core.compareQueries(params.left_sql, params.right_sql, params.dialect || undefined) + const raw = core.compareQueries(params.left_sql, params.right_sql, params.dialect ?? undefined) return ok(true, toData(raw)) } catch (e) { return fail(e) @@ -522,7 +528,7 @@ export function registerAll(): void { // 29. altimate_core.import_ddl — returns Schema, must serialize register("altimate_core.import_ddl", async (params) => { try { - const schema = core.importDdl(params.ddl, params.dialect || undefined) + const schema = core.importDdl(params.ddl, params.dialect ?? undefined) const jsonObj = schema.toJson() return ok(true, { success: true, schema: toData(jsonObj) }) } catch (e) { diff --git a/packages/opencode/src/altimate/native/sql/register.ts b/packages/opencode/src/altimate/native/sql/register.ts index 8a337bdcf9..60b565289f 100644 --- a/packages/opencode/src/altimate/native/sql/register.ts +++ b/packages/opencode/src/altimate/native/sql/register.ts @@ -359,7 +359,9 @@ export function registerAllSql(): void { const sqlA = params.original ?? params.sql_a const sqlB = params.modified ?? params.sql_b - const compareRaw = schema ? await core.checkEquivalence(sqlA, sqlB, schema) : null + // `|| undefined`: coerce a default empty-string dialect to "no hint" — the + // engine throws on an unknown dialect "". + const compareRaw = schema ? await core.checkEquivalence(sqlA, sqlB, schema, params.dialect || undefined) : null const compare = compareRaw ? JSON.parse(JSON.stringify(compareRaw)) : null // Simple line-based diff diff --git a/packages/opencode/src/altimate/native/types.ts b/packages/opencode/src/altimate/native/types.ts index f3e72e562e..ad90c7631e 100644 --- a/packages/opencode/src/altimate/native/types.ts +++ b/packages/opencode/src/altimate/native/types.ts @@ -724,6 +724,8 @@ export interface SqlDiffParams { original: string modified: string context_lines?: number + /** Optional parsing-dialect hint forwarded to the equivalence engine. */ + dialect?: string } export interface SqlDiffResult { diff --git a/packages/opencode/src/altimate/review/runner.ts b/packages/opencode/src/altimate/review/runner.ts index f26b6add65..e4b5e8f0fd 100644 --- a/packages/opencode/src/altimate/review/runner.ts +++ b/packages/opencode/src/altimate/review/runner.ts @@ -481,14 +481,18 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun }) const data = (res.data ?? {}) as Record const validationErrors = asArray(data.validation_errors) - // Undecidable when: call failed, no schema to resolve columns, or the - // engine returned validation errors. Never guess equivalent=true. + // Undecidable when: call failed, no schema to resolve columns, the engine + // returned validation errors, or the engine itself flagged the comparison + // as not decidable (altimate-core@0.5.1 `decidable` field — authoritative, + // catches semantic abstentions that carry no validation error). Never + // guess equivalent=true. const decided = res.success === true && typeof data.equivalent === "boolean" && !res.error && !data.error && validationErrors.length === 0 && + data.decidable !== false && !!schema if (!decided) return { decided: false } // The engine flags column-reorder etc. as `minor` but still sets diff --git a/packages/opencode/test/altimate/altimate-core-native.test.ts b/packages/opencode/test/altimate/altimate-core-native.test.ts index 243425ec8c..78bbb507ac 100644 --- a/packages/opencode/test/altimate/altimate-core-native.test.ts +++ b/packages/opencode/test/altimate/altimate-core-native.test.ts @@ -816,3 +816,101 @@ describe("Real-world equivalence regressions", () => { expect(await structRules("select distinct a from t group by a", "select distinct a from t group by a")).toEqual([]) }) }) + +// --------------------------------------------------------------------------- +// altimate-core@0.5.1: dialect forwarding + authoritative `decidable` field +// --------------------------------------------------------------------------- +// +// 0.5.1 added an optional `dialect` arg to checkEquivalence and a `decidable` +// flag on the result. The native equivalence handler now forwards `params.dialect` +// to the engine so dialect-specific compiled warehouse SQL parses instead of +// abstaining on a syntax error. These tests exercise the REAL engine through the +// Dispatcher (no mocks) to prove the arg is wired end-to-end. +describe("core 0.5.1 dialect forwarding + decidable", () => { + beforeAll(async () => { + registerAll() + // sql.diff lives in the sql/* handler set, registered separately. + const { registerAllSql } = await import("../../src/altimate/native/sql/register") + registerAllSql() + }) + + const variantSchema = { + tables: { t: { columns: [ + { name: "x", type: "INT" }, { name: "payload", type: "VARIANT" }, + ] } }, + } + // Snowflake semi-structured access `payload:f` is a hard PARSE error in the + // default dialect (the ':' is rejected) but parses under the snowflake dialect. + // The validation-error TEXT therefore differs by dialect — a syntax error only + // when the hint is dropped. Pre-fix (dialect not forwarded) BOTH would be syntax + // errors; this asserts they diverge, proving the arg reaches the parser. + const colonSql = "select payload:f::int as v from t" + const equivCall = (dialect?: string) => + Dispatcher.call("altimate_core.equivalence", { sql1: colonSql, sql2: colonSql, schema_context: variantSchema, dialect }) + const hasSyntaxError = (data: any) => + (data.validation_errors ?? []).some((e: string) => /Syntax error|Expected end of statement/i.test(String(e))) + + test("dialect hint is forwarded to the engine (changes parse outcome)", async () => { + const noDialect = await equivCall(undefined) + const snowflake = await equivCall("snowflake") + expect(noDialect.success).toBe(true) + expect(snowflake.success).toBe(true) + // Without a dialect the colon is a syntax error; snowflake accepts it. + expect(hasSyntaxError(noDialect.data)).toBe(true) + expect(hasSyntaxError(snowflake.data)).toBe(false) + }) + + test("decidable=false is surfaced when the engine cannot parse/plan", async () => { + const noDialect = await equivCall(undefined) + expect((noDialect.data as any).decidable).toBe(false) + }) + + test("decidable=true is surfaced for a cleanly decided pair", async () => { + const r = await Dispatcher.call("altimate_core.equivalence", { + sql1: "select count(*) from t", + sql2: "select count(1) from t", + schema_context: variantSchema, + }) + expect(r.success).toBe(true) + const d = r.data as any + expect(d.decidable).toBe(true) + expect(d.equivalent).toBe(true) + }) + + test("empty-string dialect is coerced to no-hint (does NOT throw)", async () => { + // ReviewConfig.dialect defaults to "" (auto-detect). The engine throws on an + // unknown dialect "", so the handler must coerce "" → undefined (`|| undefined`, + // not `??`). Regression guard: before the coercion this returned success=false + // and silently disabled equivalence checking on the default config. + const r = await Dispatcher.call("altimate_core.equivalence", { + sql1: "select count(*) from t", + sql2: "select count(1) from t", + schema_context: variantSchema, + dialect: "", + }) + expect(r.success).toBe(true) + const d = r.data as any + expect(d.decidable).toBe(true) + expect(d.equivalent).toBe(true) + }) + + test("sql.diff accepts and forwards the dialect hint without throwing", async () => { + // sql.diff's runtime shape (success/diff/equivalent) predates its declared + // SqlDiffResult type, so read through `any`. The assertion that matters: the + // dialect param is plumbed into the handler's checkEquivalence call (Edit in + // sql/register.ts) and the snowflake path is reachable end-to-end. + const call = (dialect?: string) => + Dispatcher.call("sql.diff", { + original: colonSql, + modified: colonSql, + schema_context: variantSchema, + dialect, + } as any) as Promise + const noDialect = await call(undefined) + const snowflake = await call("snowflake") + expect(noDialect.success).toBe(true) + expect(snowflake.success).toBe(true) + expect(typeof noDialect.diff).toBe("string") + expect(typeof snowflake.diff).toBe("string") + }) +}) diff --git a/packages/opencode/test/altimate/review-equivalence-e2e.test.ts b/packages/opencode/test/altimate/review-equivalence-e2e.test.ts new file mode 100644 index 0000000000..f0031d17cb --- /dev/null +++ b/packages/opencode/test/altimate/review-equivalence-e2e.test.ts @@ -0,0 +1,143 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test" +import { writeFileSync } from "node:fs" +import path from "node:path" +import { runReview, DEFAULT_RUBRIC, DEFAULT_REVIEW_CONFIG, type ChangedFile } from "../../src/altimate/review" +import { createDispatcherRunner } from "../../src/altimate/review/runner" +import { registerAll } from "../../src/altimate/native/altimate-core" +import { tmpdir } from "../fixture/fixture" + +// --------------------------------------------------------------------------- +// Full-stack E2E: dbt PR review pipeline → real Dispatcher → real altimate-core +// 0.5.1 engine. NO mocks. Exercises the complete chain that the dialect wiring +// (core 0.5.1 `dialect` arg) and `decidable` handling run through: +// +// runReview → semanticChangeLane → runner.equivalence(old, new, dialect) +// → Dispatcher → altimate_core.equivalence handler +// → core.checkEquivalence(sqlA, sqlB, schema, dialect) +// +// A real dbt manifest supplies the schema so the engine can resolve columns and +// actually DECIDE equivalence rather than abstain. +// --------------------------------------------------------------------------- + +describe("E2E: review pipeline + real engine equivalence (core 0.5.1)", () => { + beforeAll(async () => { + process.env.ALTIMATE_TELEMETRY_DISABLED = "true" + // The DispatcherRunner calls the REAL native handlers; register them in case + // another file reset the Dispatcher. + registerAll() + const { registerAllSql } = await import("../../src/altimate/native/sql/register") + registerAllSql() + }) + afterAll(() => { + delete process.env.ALTIMATE_TELEMETRY_DISABLED + }) + + // A real manifest with typed columns so resolveSchema() yields a usable schema. + async function reviewerWithManifest() { + const tmp = await tmpdir() + const manifestPath = path.join(tmp.path, "manifest.json") + writeFileSync( + manifestPath, + JSON.stringify({ + metadata: { adapter_type: "snowflake" }, + nodes: { + "model.demo.orders": { + resource_type: "model", + name: "orders", + original_file_path: "models/marts/orders.sql", + config: { materialized: "table" }, + depends_on: { nodes: [] }, + columns: { + id: { name: "id", data_type: "integer" }, + amount: { name: "amount", data_type: "integer" }, + status: { name: "status", data_type: "varchar" }, + }, + }, + }, + sources: {}, + }), + ) + return { tmp, runner: createDispatcherRunner({ manifestPath }) } + } + + async function review(oldSql: string, newSql: string, dialect = "snowflake") { + const { tmp, runner } = await reviewerWithManifest() + try { + const files: ChangedFile[] = [{ path: "models/marts/orders.sql", status: "modified", diff: "+changed" }] + const env = await runReview({ + changedFiles: files, + config: { ...DEFAULT_REVIEW_CONFIG, reviewers: ["semantic_change"], dialect, ai: false }, + rubric: DEFAULT_RUBRIC, + mode: "comment", + runner, + getContent: async (_f, side) => (side === "new" ? newSql : oldSql), + getCompiled: async (_f, side) => (side === "new" ? newSql : oldSql), + generatedAt: "2026-06-10T00:00:00Z", + }) + return env + } finally { + await tmp[Symbol.asyncDispose]?.() + } + } + + const eqFindings = (env: Awaited>) => + env.findings.filter((f) => f.evidence?.tool === "altimate_core.equivalence") + + test("real engine DECIDES a non-equivalent rewrite through the full pipeline", async () => { + // `amount > 5` vs `amount > 6` is a genuine row-changing predicate change. + const env = await review( + "select id from orders where amount > 5", + "select id from orders where amount > 6", + ) + const findings = eqFindings(env) + expect(findings.length).toBeGreaterThan(0) + const f = findings[0] + expect(f.category).toBe("semantic_change") + expect((f.evidence?.result as any)?.equivalent).toBe(false) + // The engine names the concrete predicate difference (not a vague abstention). + const diffs = (f.evidence?.result as any)?.differences ?? [] + expect(diffs.length).toBeGreaterThan(0) + }) + + test("real engine PROVES an equivalent refactor → lane stays silent (no false positive)", async () => { + // AND-conjunct reorder is provably equivalent; the reviewer must not nitpick it. + const env = await review( + "select id from orders where amount > 5 and status = 'x'", + "select id from orders where status = 'x' and amount > 5", + ) + expect(eqFindings(env)).toEqual([]) + expect(env.verdict).toBe("APPROVE") + }) + + test("identical compiled SQL → equivalence lane is skipped entirely", async () => { + const sql = "select id from orders where amount > 5" + const env = await review(sql, sql) + expect(eqFindings(env)).toEqual([]) + expect(env.verdict).toBe("APPROVE") + }) + + test("column projection change (drop a column) is decided NOT equivalent", async () => { + const env = await review( + "select id, amount from orders", + "select id from orders", + ) + const findings = eqFindings(env) + expect(findings.length).toBeGreaterThan(0) + expect((findings[0].evidence?.result as any)?.equivalent).toBe(false) + }) + + test("DEFAULT config dialect (empty string) still DECIDES — no engine throw", async () => { + // ReviewConfig.dialect defaults to "". The engine throws on an unknown dialect + // "", so without coercion the lane would abstain on EVERY change under the + // default config. This drives the full pipeline with dialect="" and asserts + // the non-equivalent change is still caught (decided), proving the coercion. + const env = await review( + "select id from orders where amount > 5", + "select id from orders where amount > 6", + "", // <- default ReviewConfig.dialect + ) + const findings = eqFindings(env) + expect(findings.length).toBeGreaterThan(0) + expect((findings[0].evidence?.result as any)?.equivalent).toBe(false) + }) +}) diff --git a/packages/opencode/test/altimate/review-runner.test.ts b/packages/opencode/test/altimate/review-runner.test.ts index 40e38c8ad8..538b2d7631 100644 --- a/packages/opencode/test/altimate/review-runner.test.ts +++ b/packages/opencode/test/altimate/review-runner.test.ts @@ -88,3 +88,77 @@ describe("review manifest loading", () => { expect(seenParams.dialect).toBe("duckdb") }) }) + +describe("runner honors engine `decidable` flag (core 0.5.1)", () => { + // Manifest with a typed column so resolveSchema() yields a non-null schema + // (the runner abstains when no schema is available, independent of decidable). + async function runnerWithManifest() { + const tmp = await tmpdir() + const manifestPath = path.join(tmp.path, "manifest.json") + writeFileSync( + manifestPath, + JSON.stringify({ + metadata: { adapter_type: "duckdb" }, + nodes: { + "model.demo.orders": { + resource_type: "model", + name: "orders", + original_file_path: "models/orders.sql", + config: { materialized: "table" }, + depends_on: { nodes: [] }, + columns: { id: { name: "id", data_type: "integer" } }, + }, + }, + sources: {}, + }), + ) + return { tmp, runner: createDispatcherRunner({ manifestPath }) } + } + + function mockEquivalence(data: Record) { + dispatcherSpy = spyOn(Dispatcher, "call").mockImplementation((async () => ({ + success: true, + data, + })) as any) + } + + test("decidable=false with NO validation errors → abstains (decided:false)", async () => { + const { tmp, runner } = await runnerWithManifest() + // The engine says equivalent=true but flags the comparison as not decidable. + // We must NOT clear the change as safe on an authoritative abstention. + mockEquivalence({ equivalent: true, confidence: 0.95, differences: [], validation_errors: [], decidable: false }) + const result = await runner.equivalence("select id from orders", "select id from orders", "duckdb") + expect(result).toEqual({ decided: false }) + await tmp[Symbol.asyncDispose]?.() + }) + + test("decidable=true → decided verdict is returned", async () => { + const { tmp, runner } = await runnerWithManifest() + mockEquivalence({ equivalent: true, confidence: 0.95, differences: [], validation_errors: [], decidable: true }) + const result = await runner.equivalence("select id from orders", "select id from orders", "duckdb") + expect(result).toEqual({ decided: true, equivalent: true, differences: [], confidence: "high" }) + await tmp[Symbol.asyncDispose]?.() + }) + + test("decidable absent (0.4.0 legacy shape) → still decided (backward compatible)", async () => { + const { tmp, runner } = await runnerWithManifest() + mockEquivalence({ equivalent: true, confidence: 0.95, differences: [], validation_errors: [] }) + const result = await runner.equivalence("select id from orders", "select id from orders", "duckdb") + expect(result).toEqual({ decided: true, equivalent: true, differences: [], confidence: "high" }) + await tmp[Symbol.asyncDispose]?.() + }) + + test("decidable=false overrides even a non-equivalent verdict (no false block)", async () => { + const { tmp, runner } = await runnerWithManifest() + mockEquivalence({ + equivalent: false, + confidence: 0.5, + differences: [{ description: "maybe changed", severity: "semantic" }], + validation_errors: [], + decidable: false, + }) + const result = await runner.equivalence("select id from orders", "select 1 as id from orders", "duckdb") + expect(result).toEqual({ decided: false }) + await tmp[Symbol.asyncDispose]?.() + }) +})