From 57ea234a078108dc653b66b36feea92456dc6ef4 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 28 Aug 2026 18:17:58 -0700 Subject: [PATCH 01/21] feat(validators): nothing-built inverse completion gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both existing validators start from `modelsModifiedSince(sessionStartMs)`, so a session that wrote nothing has an empty work list and passes every gate trivially. Evaluation traces show empty-workspace-plus-confident-summary is a dominant lost-session end state, so the lane needs an inverse gate. `dbt-nothing-built` refuses to terminate when the workspace is a dbt project, the session authored no project files, and no fresh successful `run_results.json` exists. Read-only/analysis sessions stay unaffected: `appliesTo` requires positive evidence that artifacts were demanded — a task/instruction document that literally names required models or files, or the explicit `ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS=1` opt-in. Absent both, the validator never inspects the session. Shared helpers added to `validator-utils.ts`: - `findTaskInstructionFile` — closed candidate list, `README.md` excluded - `extractRequiredDeliverables` — three literal tiers (declaration marker, deliverables section, requirement lines); no fuzzy matching, returns null on an unknown contract - `resolveDbtTargetPath` / `readRunResults` / `isFailedRunStatus` - `collectProducedNodeNames` — union of fs inventory and manifest aliases - `stripSqlComments` 38 tests covering extraction tiers, task-file discovery, artifact parsing and every appliesTo/check branch. Co-Authored-By: Claude Fable 5 --- .../altimate/validators/dbt-nothing-built.ts | 189 ++++++++ .../opencode/src/altimate/validators/index.ts | 10 +- .../altimate/validators/validator-utils.ts | 429 ++++++++++++++++++ .../validators/dbt-nothing-built.test.ts | 383 ++++++++++++++++ 4 files changed, 1008 insertions(+), 3 deletions(-) create mode 100644 packages/opencode/src/altimate/validators/dbt-nothing-built.ts create mode 100644 packages/opencode/test/altimate/validators/dbt-nothing-built.test.ts diff --git a/packages/opencode/src/altimate/validators/dbt-nothing-built.ts b/packages/opencode/src/altimate/validators/dbt-nothing-built.ts new file mode 100644 index 0000000000..f97a54e3ec --- /dev/null +++ b/packages/opencode/src/altimate/validators/dbt-nothing-built.ts @@ -0,0 +1,189 @@ +// altimate_change start — nothing-built inverse completion gate +/** + * "Nothing was built" inverse gate. + * + * Every other validator in this lane starts from the set of files the session + * modified. That leaves a blind spot at the bottom of the lane: a session + * that wrote *nothing at all* touches no models, so every model-scoped gate + * has an empty work list and passes trivially. Evaluation traces show this is + * not a hypothetical — the dominant end-state of a lost session is an empty + * workspace plus a confident summary, which the lane waves through. + * + * This validator inverts the question: instead of "is what you wrote + * correct?", it asks "the task required artifacts — where are they?". + * + * False-positive safety is the whole design problem here, because plenty of + * legitimate sessions are read-only (analysis, code reading, cost review) and + * must be allowed to finish having written nothing. So the gate never fires + * on the mere absence of writes. It requires positive evidence that the task + * demanded artifacts: + * + * - a task/instruction document in the workspace that literally names + * required models or files (see `extractRequiredDeliverables`), or + * - explicit opt-in via `ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS=1`. + * + * With neither present, `appliesTo` returns false and the session is never + * even inspected. + */ + +import { promises as fs } from "fs" +import { join } from "path" +import type { Validator, ValidatorContext, ValidatorResult } from "../../session/validators/types" +import { + findDbtProjectRoot, + findTaskInstructionFile, + extractRequiredDeliverables, + readRunResults, + isFailedRunStatus, + type RequiredDeliverables, +} from "./validator-utils" + +/** Env flag that forces the gate on regardless of task-file discovery. */ +const OPT_IN_ENV = "ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS" + +/** + * Directories whose contents count as "the session produced something". Wider + * than `models/` on purpose: editing a seed, a snapshot, a macro or a schema + * file is real work, and this gate must only fire on a session that produced + * nothing whatsoever. + */ +const AUTHORED_DIRS = ["models", "seeds", "snapshots", "data", "analyses", "macros", "tests"] +/** Depth limit mirroring the other project scans in this lane. */ +const SCAN_MAX_DEPTH = 8 + +/** Evidence that this session was expected to produce artifacts. */ +interface ArtifactExpectation { + /** Why the gate considers itself applicable. */ + kind: "task-file" | "opt-in" + /** Path of the task document, when that is the source of the expectation. */ + taskFile?: string + /** Deliverables the document named, when it named any. */ + required?: RequiredDeliverables +} + +/** + * Decide whether this session was expected to produce artifacts. Returns null + * when there is no such evidence, which makes the validator skip entirely. + */ +async function artifactExpectation( + cwd: string, + dbtRoot: string, +): Promise { + const task = await findTaskInstructionFile(cwd, dbtRoot) + if (task) { + const required = extractRequiredDeliverables(task.content) + if (required) return { kind: "task-file", taskFile: task.path, required } + } + if (process.env[OPT_IN_ENV] === "1") return { kind: "opt-in" } + return null +} + +/** + * True as soon as any authored file under the project was written during this + * session. Short-circuits on the first hit so the common case is cheap. + */ +async function anyAuthoredFileSince(dbtRoot: string, sinceMs: number): Promise { + async function scan(dir: string, depth: number): Promise { + if (depth > SCAN_MAX_DEPTH) return false + let entries: import("fs").Dirent[] + try { + entries = await fs.readdir(dir, { withFileTypes: true }) + } catch { + return false + } + for (const entry of entries) { + if (entry.name.startsWith(".") || entry.name === "node_modules" || entry.name === "target") { + continue + } + const full = join(dir, entry.name) + let stat: import("fs").Stats + try { + stat = await fs.stat(full) + } catch { + continue + } + if (stat.isDirectory()) { + if (await scan(full, depth + 1)) return true + } else if (stat.isFile() && stat.mtimeMs >= sinceMs) { + return true + } + } + return false + } + for (const dir of AUTHORED_DIRS) { + if (await scan(join(dbtRoot, dir), 0)) return true + } + return false +} + +export const DbtNothingBuiltValidator: Validator = { + name: "dbt-nothing-built", + description: + "Inverse completion gate. When the workspace carries a task document that literally names required models or files (or the require-artifacts opt-in is set), refuses to terminate a session that authored no project files and produced no fresh successful build artifact.", + + async appliesTo(ctx: ValidatorContext): Promise { + const dbtRoot = await findDbtProjectRoot(ctx.workingDirectory) + if (!dbtRoot) return false + return (await artifactExpectation(ctx.workingDirectory, dbtRoot)) !== null + }, + + async check(ctx: ValidatorContext): Promise { + const startedAt = Date.now() + const dbtRoot = await findDbtProjectRoot(ctx.workingDirectory) + if (!dbtRoot) { + return { ok: true, details: { skipped: "no dbt project", session_id: ctx.sessionID } } + } + const expectation = await artifactExpectation(ctx.workingDirectory, dbtRoot) + if (!expectation) { + return { + ok: true, + details: { skipped: "no artifact expectation", session_id: ctx.sessionID }, + } + } + + const authored = await anyAuthoredFileSince(dbtRoot, ctx.sessionStartMs) + const runResults = await readRunResults(dbtRoot) + const freshRun = + runResults !== null && + runResults.mtimeMs >= ctx.sessionStartMs && + runResults.results.some((r) => !isFailedRunStatus(r.status)) + + const details = { + expectation: expectation.kind, + task_file: expectation.taskFile ?? null, + required_models: expectation.required?.models ?? [], + required_source: expectation.required?.source ?? null, + authored_files: authored, + fresh_run_results: freshRun, + run_results_path: runResults?.path ?? null, + dbt_root: dbtRoot, + session_id: ctx.sessionID, + elapsed_ms: Date.now() - startedAt, + } + + if (authored || freshRun) return { ok: true, details } + + const named = expectation.required?.models ?? [] + const namedText = named.length > 0 ? `: ${named.join(", ")}` : "" + const reason = + expectation.kind === "task-file" + ? `The task document at ${expectation.taskFile} names required deliverables${namedText}, but this session wrote no project files and produced no fresh successful build artifact. Nothing was built, so the task is not done.` + : `This session wrote no project files and produced no fresh successful build artifact, but the workspace is configured to require artifacts. Nothing was built, so the task is not done.` + + return { + ok: false, + reason, + fixHint: + [ + "Do the work before declaring done:", + named.length > 0 + ? ` • Create each required deliverable under \`models/\` using the literal name given: ${named.join(", ")}.` + : " • Create the model files the task asks for under `models/`.", + " • Build them (`dbt build` / `dbt run`) so a successful `run_results.json` exists.", + " • If you believe the work is already present, re-read the task document and name the file you produced for each deliverable.", + ].join("\n"), + details, + } + }, +} +// altimate_change end diff --git a/packages/opencode/src/altimate/validators/index.ts b/packages/opencode/src/altimate/validators/index.ts index 3481757b6e..8ea0a38179 100644 --- a/packages/opencode/src/altimate/validators/index.ts +++ b/packages/opencode/src/altimate/validators/index.ts @@ -1,5 +1,6 @@ // altimate_change start — explicit registration entry point for altimate validators import { ValidatorRegistry } from "../../session/validators/registry" +import { DbtNothingBuiltValidator } from "./dbt-nothing-built" import { DbtSchemaVerifyValidator } from "./dbt-schema-verify" import { DbtTestsPassValidator } from "./dbt-tests-pass" @@ -12,11 +13,14 @@ import { DbtTestsPassValidator } from "./dbt-tests-pass" * Idempotent: ValidatorRegistry.register is keyed by name so repeat calls * just overwrite. * - * Validators run in registration order; schema-verify is registered first - * because column-shape mismatches typically explain test failures, so we - * want that signal surfaced before generic test-failure noise. + * Validators run in registration order, cheapest and most fundamental first: + * "did you build anything at all" precedes "is what you built shaped right", + * which precedes "do its tests pass". Column-shape mismatches typically + * explain test failures, so that signal is surfaced before generic + * test-failure noise. */ export function registerAltimateValidators(): void { + ValidatorRegistry.register(DbtNothingBuiltValidator) ValidatorRegistry.register(DbtSchemaVerifyValidator) ValidatorRegistry.register(DbtTestsPassValidator) } diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index c228b9ffce..47b6ca0a20 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -326,3 +326,432 @@ function isValidEnvelope(obj: Record): boolean { ) } // altimate_change end + +// altimate_change start — task-contract, build-artifact and project-inventory helpers +/** + * Helpers shared by the completion-gate validators that reason about the + * task's own literal contract and about build artifacts, rather than about a + * single touched model. + * + * Design rule for everything in this block: be conservative. These helpers + * feed gates that can refuse to let a session finish, so each returns + * "unknown" (null / empty) rather than a guess when the workspace does not + * carry unambiguous evidence. + */ + +// --------------------------------------------------------------------------- +// Task / instruction file discovery +// --------------------------------------------------------------------------- + +/** A discovered task/instruction document plus where it came from. */ +export interface TaskInstructionFile { + /** Absolute path of the file that was read. */ + path: string + /** Raw file contents. */ + content: string +} + +/** + * Filenames accepted as "the task the session was given". Deliberately a + * closed list of names that only ever exist because somebody wrote down an + * assignment — `README.md` is excluded because it is present in almost every + * repository and describes the project, not the task. + * + * Order is significant: the first match wins, so the list runs from most to + * least explicit. + */ +export const TASK_FILE_CANDIDATES = [ + "TASK.md", + "TASK.txt", + "TASKS.md", + "INSTRUCTIONS.md", + "INSTRUCTIONS.txt", + "REQUIREMENTS.md", + "SPEC.md", + "task.md", + "task.txt", + "instructions.md", + "requirements.md", + "spec.md", +] as const + +/** Largest task file we will read. Guards against a stray multi-MB document. */ +const TASK_FILE_MAX_BYTES = 512 * 1024 + +/** + * Locate the task/instruction document for this workspace. + * + * Search order: + * 1. `ALTIMATE_VALIDATORS_TASK_FILE` (absolute, or relative to `cwd`) — the + * explicit opt-in for harnesses that put the task somewhere unusual. + * 2. `TASK_FILE_CANDIDATES` at `cwd`. + * 3. `TASK_FILE_CANDIDATES` inside `cwd/.altimate/`. + * 4. `TASK_FILE_CANDIDATES` at `dbtRoot`, when the dbt project is nested + * below `cwd`. + * + * Returns null when no such document exists. Callers must treat that as "the + * task contract is unknown" and skip, never as "nothing was required". + */ +export async function findTaskInstructionFile( + cwd: string, + dbtRoot?: string | null, +): Promise { + const explicit = process.env.ALTIMATE_VALIDATORS_TASK_FILE + const roots = [cwd, join(cwd, ".altimate")] + if (dbtRoot && dbtRoot !== cwd) roots.push(dbtRoot) + const paths: string[] = [] + if (explicit && explicit.trim().length > 0) { + paths.push(isAbsolutePath(explicit) ? explicit : join(cwd, explicit)) + } + for (const root of roots) { + for (const name of TASK_FILE_CANDIDATES) paths.push(join(root, name)) + } + for (const p of paths) { + try { + const stat = await fs.stat(p) + if (!stat.isFile() || stat.size > TASK_FILE_MAX_BYTES) continue + const content = await fs.readFile(p, "utf8") + if (content.trim().length === 0) continue + return { path: p, content } + } catch { + // not present / unreadable — keep looking + } + } + return null +} + +/** Absolute-path test that also accepts Windows drive-letter roots. */ +function isAbsolutePath(p: string): boolean { + return p.startsWith("/") || /^[A-Za-z]:[\\/]/.test(p) +} + +// --------------------------------------------------------------------------- +// Literal deliverable extraction +// --------------------------------------------------------------------------- + +/** Deliverables a task document names literally. */ +export interface RequiredDeliverables { + /** Bare relation/model identifiers, lowercased, de-duplicated. */ + models: string[] + /** Literal file paths, as written (e.g. `models/marts/orders.sql`). */ + files: string[] + /** Which extraction tier produced the names — recorded for telemetry. */ + source: "declaration" | "deliverables-section" | "requirement-lines" +} + +/** + * Words that are never a deliverable name even inside a code span on a + * requirement line. This list is what keeps the extractor literal: anything + * that survives it was written by the task author as an identifier. + */ +const DELIVERABLE_STOPWORDS = new Set([ + "model", "models", "table", "tables", "view", "views", "seed", "seeds", + "snapshot", "snapshots", "mart", "marts", "staging", "source", "sources", + "column", "columns", "row", "rows", "schema", "database", "warehouse", + "dbt", "sql", "yml", "yaml", "json", "csv", "select", "from", "where", + "group", "order", "join", "ref", "config", "target", "project", + "dbt_project", "profiles", "run", "build", "test", "tests", "compile", + "true", "false", "null", "int", "integer", "float", "string", "varchar", + "date", "datetime", "timestamp", "boolean", "the", "and", "not", "with", +]) + +/** A deliverable identifier: SQL-identifier shaped, at least three chars. */ +const IDENTIFIER_RE = /^[A-Za-z_][A-Za-z0-9_]{2,}$/ +/** A literal file path a task can require verbatim. */ +const FILE_PATH_RE = /^[A-Za-z0-9_./-]+\.(?:sql|ya?ml|csv)$/i +/** Inline code spans — the only place a name is accepted from. */ +const CODE_SPAN_RE = /`([^`\n]+)`/g +/** Verb that makes a line a requirement rather than background prose. */ +const REQUIREMENT_VERB_RE = + /\b(?:creat|build|produc|implement|deliver|materiali[sz]|generat|writ|deploy)\w*\b/i +/** Noun that makes the requirement about a data artifact. */ +const DELIVERABLE_NOUN_RE = + /\b(?:model|models|table|tables|view|views|seed|seeds|snapshot|snapshots|mart|marts|file|files)\b/i +/** Heading that opens an explicit deliverables list. */ +const DELIVERABLES_HEADING_RE = /^\s{0,3}#{1,6}\s*(?:required|deliverab|expected output)/i +/** Any other heading closes it. */ +const ANY_HEADING_RE = /^\s{0,3}#{1,6}\s/ +/** Machine-readable declaration block. */ +const DECLARATION_RE = + /(?:|\n|$)/i + +/** + * Extract the deliverable names a task document states **literally**. + * + * Three tiers, most explicit first; the first tier that yields anything wins, + * so a workspace that declares its contract machine-readably is never second- + * guessed by prose scanning: + * + * 1. `declaration` — an `altimate:required-models: a, b` marker, optionally + * inside an HTML comment. + * 2. `deliverables-section` — inline-code identifiers under a heading whose + * text starts with "Required" / "Deliverab…" / "Expected output". + * 3. `requirement-lines` — inline-code identifiers on a line carrying both + * a requirement verb and a data-artifact noun. + * + * No fuzzy matching and no inference: a name must sit inside a code span (or + * the declaration block), must be identifier- or path-shaped, and must not be + * a generic data-modelling word. Returns null when the document names + * nothing — callers must treat that as "unknown contract". + */ +export function extractRequiredDeliverables(text: string): RequiredDeliverables | null { + if (!text) return null + + const declaration = DECLARATION_RE.exec(text) + if (declaration && declaration[1]) { + const collected = collectDeliverableTokens(declaration[1].split(/[,\s]+/)) + if (collected.models.length > 0 || collected.files.length > 0) { + return { ...collected, source: "declaration" } + } + } + + const lines = text.split(/\r?\n/) + + // Tier 2 — an explicit deliverables section. + const sectionTokens: string[] = [] + let inSection = false + for (const line of lines) { + if (DELIVERABLES_HEADING_RE.test(line)) { + inSection = true + continue + } + if (inSection && ANY_HEADING_RE.test(line)) { + inSection = false + continue + } + if (inSection) sectionTokens.push(...inlineCodeSpans(line)) + } + const section = collectDeliverableTokens(sectionTokens) + if (section.models.length > 0 || section.files.length > 0) { + return { ...section, source: "deliverables-section" } + } + + // Tier 3 — requirement lines in prose. + const proseTokens: string[] = [] + for (const line of lines) { + if (!REQUIREMENT_VERB_RE.test(line)) continue + if (!DELIVERABLE_NOUN_RE.test(line)) continue + proseTokens.push(...inlineCodeSpans(line)) + } + const prose = collectDeliverableTokens(proseTokens) + if (prose.models.length > 0 || prose.files.length > 0) { + return { ...prose, source: "requirement-lines" } + } + return null +} + +/** Pull the contents of every inline code span on a line. */ +function inlineCodeSpans(line: string): string[] { + const out: string[] = [] + CODE_SPAN_RE.lastIndex = 0 + let m: RegExpExecArray | null + while ((m = CODE_SPAN_RE.exec(line)) !== null) { + if (m[1]) out.push(m[1]) + } + return out +} + +/** Classify raw tokens into model identifiers and literal file paths. */ +function collectDeliverableTokens(tokens: string[]): { models: string[]; files: string[] } { + const models: string[] = [] + const files: string[] = [] + for (const raw of tokens) { + const token = raw.trim().replace(/[.,;:]+$/, "") + if (!token) continue + if (token.includes("/") && FILE_PATH_RE.test(token)) { + if (!files.includes(token)) files.push(token) + // A required `models/marts/orders.sql` also requires the model `orders`. + const bare = modelNameFromPath(token).toLowerCase() + if (IDENTIFIER_RE.test(bare) && !DELIVERABLE_STOPWORDS.has(bare) && !models.includes(bare)) { + models.push(bare) + } + continue + } + // A bare `orders.sql` names a model without pinning its directory. + const withoutExt = token.toLowerCase().replace(/\.(?:sql|ya?ml|csv)$/i, "") + if (!IDENTIFIER_RE.test(withoutExt)) continue + if (DELIVERABLE_STOPWORDS.has(withoutExt)) continue + if (!models.includes(withoutExt)) models.push(withoutExt) + } + return { models, files } +} + +// --------------------------------------------------------------------------- +// dbt build artifacts +// --------------------------------------------------------------------------- + +/** + * Resolve the project's artifact directory. Honours `DBT_TARGET_PATH` and a + * `target-path:` key in `dbt_project.yml`; defaults to `target`. + */ +export async function resolveDbtTargetPath(dbtRoot: string): Promise { + const fromEnv = process.env.DBT_TARGET_PATH + if (fromEnv && fromEnv.trim().length > 0) { + return isAbsolutePath(fromEnv) ? fromEnv : join(dbtRoot, fromEnv) + } + try { + const yml = await fs.readFile(join(dbtRoot, "dbt_project.yml"), "utf8") + const m = /^\s*target-path\s*:\s*["']?([^"'#\n]+?)["']?\s*(?:#.*)?$/m.exec(yml) + if (m && m[1] && m[1].trim().length > 0) { + const value = m[1].trim() + return isAbsolutePath(value) ? value : join(dbtRoot, value) + } + } catch { + // no project file / unreadable — fall through to the default + } + return join(dbtRoot, "target") +} + +/** One node of a dbt `run_results.json`. */ +export interface RunResultNode { + /** e.g. `model.my_project.orders`. */ + uniqueId: string + /** Bare node name, lowercased (`orders`). */ + name: string + /** dbt status string, lowercased (`success`, `error`, `skipped`, …). */ + status: string + /** dbt's message for the node, when present. */ + message: string | null +} + +/** A parsed `run_results.json` plus its freshness. */ +export interface RunResultsArtifact { + path: string + /** mtime of the artifact file. */ + mtimeMs: number + results: RunResultNode[] +} + +/** dbt statuses that mean the node built cleanly. `warn` is not a failure. */ +const OK_RUN_STATUSES = new Set(["success", "pass", "warn"]) + +/** True when a run_results status means the node did NOT build cleanly. */ +export function isFailedRunStatus(status: string): boolean { + return !OK_RUN_STATUSES.has(status.toLowerCase()) +} + +/** + * Read and parse `/run_results.json`. Returns null when the artifact + * is absent or unparseable — callers decide what that means for their gate. + */ +export async function readRunResults(dbtRoot: string): Promise { + const targetPath = await resolveDbtTargetPath(dbtRoot) + const path = join(targetPath, "run_results.json") + try { + const stat = await fs.stat(path) + if (!stat.isFile()) return null + const raw = await fs.readFile(path, "utf8") + const parsed = JSON.parse(raw) as { results?: unknown } + const rows = Array.isArray(parsed.results) ? parsed.results : [] + const results: RunResultNode[] = [] + for (const row of rows) { + if (typeof row !== "object" || row === null) continue + const r = row as Record + const uniqueId = typeof r["unique_id"] === "string" ? r["unique_id"] : "" + if (!uniqueId) continue + const parts = uniqueId.split(".") + results.push({ + uniqueId, + name: (parts[parts.length - 1] ?? "").toLowerCase(), + status: typeof r["status"] === "string" ? r["status"].toLowerCase() : "", + message: typeof r["message"] === "string" ? r["message"] : null, + }) + } + return { path, mtimeMs: stat.mtimeMs, results } + } catch { + return null + } +} + +// --------------------------------------------------------------------------- +// Project inventory +// --------------------------------------------------------------------------- + +/** Directories under a dbt project that hold buildable node definitions. */ +const NODE_DIRS = ["models", "seeds", "snapshots", "data", "analyses"] +/** File extensions that define a node. */ +const NODE_EXTENSIONS = [".sql", ".csv", ".py"] +/** Depth limit mirroring `modelsModifiedSince`. */ +const INVENTORY_MAX_DEPTH = 8 + +/** + * Collect every node name the project defines on disk (models, seeds, + * snapshots, analyses) plus every node name and alias recorded in + * `manifest.json` when one exists. + * + * The union is deliberate: a gate built on this set fails only when a name is + * absent from BOTH sources, so an aliased or dynamically-named node cannot + * produce a false "you did not build it". + */ +export async function collectProducedNodeNames(dbtRoot: string): Promise> { + const names = new Set() + async function scan(dir: string, depth: number): Promise { + if (depth > INVENTORY_MAX_DEPTH) return + let entries: import("fs").Dirent[] + try { + entries = await fs.readdir(dir, { withFileTypes: true }) + } catch { + return + } + for (const entry of entries) { + if (entry.name.startsWith(".") || entry.name === "node_modules") continue + const full = join(dir, entry.name) + let isDir = entry.isDirectory() + let isFile = entry.isFile() + if (entry.isSymbolicLink()) { + try { + const target = await fs.stat(full) + isDir = target.isDirectory() + isFile = target.isFile() + } catch { + continue + } + } + if (isDir) { + await scan(full, depth + 1) + } else if (isFile) { + const lower = entry.name.toLowerCase() + const ext = NODE_EXTENSIONS.find((e) => lower.endsWith(e)) + if (ext) names.add(lower.slice(0, lower.length - ext.length)) + } + } + } + for (const nodeDir of NODE_DIRS) { + await scan(join(dbtRoot, nodeDir), 0) + } + // manifest.json contributes names and aliases for nodes whose relation name + // differs from the filename. + try { + const targetPath = await resolveDbtTargetPath(dbtRoot) + const raw = await fs.readFile(join(targetPath, "manifest.json"), "utf8") + const manifest = JSON.parse(raw) as { nodes?: Record } + for (const node of Object.values(manifest.nodes ?? {})) { + if (typeof node !== "object" || node === null) continue + const n = node as Record + for (const key of ["name", "alias", "identifier"]) { + const value = n[key] + if (typeof value === "string" && value.length > 0) names.add(value.toLowerCase()) + } + } + } catch { + // no manifest — the fs inventory stands alone + } + return names +} + +// --------------------------------------------------------------------------- +// SQL / Jinja text handling +// --------------------------------------------------------------------------- + +/** + * Blank out SQL and Jinja comments so a lint regex cannot match text the + * warehouse never sees. Comment bodies are replaced with spaces of equal + * length so downstream character offsets stay meaningful. + */ +export function stripSqlComments(sql: string): string { + return sql + .replace(/\/\*[\s\S]*?\*\//g, (m) => " ".repeat(m.length)) + .replace(/--[^\n]*/g, (m) => " ".repeat(m.length)) + .replace(/\{#[\s\S]*?#\}/g, (m) => " ".repeat(m.length)) +} +// altimate_change end diff --git a/packages/opencode/test/altimate/validators/dbt-nothing-built.test.ts b/packages/opencode/test/altimate/validators/dbt-nothing-built.test.ts new file mode 100644 index 0000000000..e0cb288b12 --- /dev/null +++ b/packages/opencode/test/altimate/validators/dbt-nothing-built.test.ts @@ -0,0 +1,383 @@ +// altimate_change start — tests for the nothing-built inverse completion gate +import { describe, expect, test, beforeEach, afterEach } from "bun:test" +import { promises as fs } from "fs" +import { tmpdir } from "os" +import { join } from "path" +import { DbtNothingBuiltValidator } from "../../../src/altimate/validators/dbt-nothing-built" +import { + extractRequiredDeliverables, + findTaskInstructionFile, + readRunResults, + resolveDbtTargetPath, + isFailedRunStatus, + collectProducedNodeNames, + stripSqlComments, +} from "../../../src/altimate/validators/validator-utils" +import type { ValidatorContext } from "../../../src/session/validators/types" + +let dir = "" + +async function makeProject(): Promise { + dir = await fs.mkdtemp(join(tmpdir(), "nothing-built-")) + await fs.writeFile( + join(dir, "dbt_project.yml"), + "name: t\nversion: '1.0'\nconfig-version: 2\nprofile: t\n", + ) + await fs.mkdir(join(dir, "models"), { recursive: true }) + return dir +} + +async function writeModel(name: string, sql = "select 1 as id"): Promise { + await fs.writeFile(join(dir, "models", `${name}.sql`), sql) +} + +async function writeRunResults(nodes: Array<{ id: string; status: string }>): Promise { + await fs.mkdir(join(dir, "target"), { recursive: true }) + await fs.writeFile( + join(dir, "target", "run_results.json"), + JSON.stringify({ + metadata: { dbt_schema_version: "v5" }, + results: nodes.map((n) => ({ unique_id: n.id, status: n.status, message: null })), + }), + ) +} + +/** Context whose session start is in the past — files on disk count as authored. */ +const ctxPast = (): ValidatorContext => ({ + sessionID: "s", + workingDirectory: dir, + sessionStartMs: 0, + step: 1, + retryCount: 0, +}) + +/** Context whose session start is in the future — nothing on disk counts as authored. */ +const ctxFuture = (): ValidatorContext => ({ + sessionID: "s", + workingDirectory: dir, + sessionStartMs: Date.now() + 60_000, + step: 1, + retryCount: 0, +}) + +afterEach(async () => { + delete process.env.ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS + delete process.env.ALTIMATE_VALIDATORS_TASK_FILE + delete process.env.DBT_TARGET_PATH + if (dir) await fs.rm(dir, { recursive: true, force: true }) + dir = "" +}) + +// --------------------------------------------------------------------------- +// extractRequiredDeliverables +// --------------------------------------------------------------------------- + +describe("extractRequiredDeliverables", () => { + test("returns null for empty input", () => { + expect(extractRequiredDeliverables("")).toBeNull() + }) + + test("returns null for prose that names nothing in code spans", () => { + expect( + extractRequiredDeliverables("Please build some models that summarise the orders data."), + ).toBeNull() + }) + + test("returns null when code spans hold only generic modelling words", () => { + expect(extractRequiredDeliverables("Create the `model` in the `models` folder.")).toBeNull() + }) + + test("declaration block wins and parses a comma list", () => { + const r = extractRequiredDeliverables("") + expect(r).not.toBeNull() + expect(r!.source).toBe("declaration") + expect(r!.models).toEqual(["orders_daily", "cust_dim"]) + }) + + test("declaration block is honoured without the HTML comment wrapper", () => { + const r = extractRequiredDeliverables("altimate:required_models: fct_sales") + expect(r!.models).toEqual(["fct_sales"]) + }) + + test("deliverables section collects code spans under the heading", () => { + const doc = [ + "# Task", + "Some background about `ignored_here`.", + "## Required deliverables", + "- `stg_orders`", + "- `fct_orders`", + "## Notes", + "- `not_a_deliverable`", + ].join("\n") + const r = extractRequiredDeliverables(doc) + expect(r!.source).toBe("deliverables-section") + expect(r!.models).toEqual(["stg_orders", "fct_orders"]) + }) + + test("requirement lines need both a verb and a data-artifact noun", () => { + const withBoth = extractRequiredDeliverables("Create a model named `dim_customer`.") + expect(withBoth!.source).toBe("requirement-lines") + expect(withBoth!.models).toEqual(["dim_customer"]) + // Verb but no artifact noun. + expect(extractRequiredDeliverables("Create a report called `dim_customer`.")).toBeNull() + // Artifact noun but no requirement verb. + expect(extractRequiredDeliverables("The model `dim_customer` is interesting.")).toBeNull() + }) + + test("literal file paths are captured and also imply the bare model name", () => { + const r = extractRequiredDeliverables("Create the model `models/marts/fct_orders.sql`.") + expect(r!.files).toEqual(["models/marts/fct_orders.sql"]) + expect(r!.models).toEqual(["fct_orders"]) + }) + + test("names are lowercased and de-duplicated", () => { + const r = extractRequiredDeliverables( + "## Deliverables\n- `Fct_Orders`\n- `fct_orders`\n- `FCT_ORDERS`\n", + ) + expect(r!.models).toEqual(["fct_orders"]) + }) + + test("rejects tokens that are not identifier shaped", () => { + const r = extractRequiredDeliverables( + "## Required\n- `select * from x`\n- `ok_name`\n- `a`\n", + ) + expect(r!.models).toEqual(["ok_name"]) + }) +}) + +// --------------------------------------------------------------------------- +// findTaskInstructionFile +// --------------------------------------------------------------------------- + +describe("findTaskInstructionFile", () => { + test("returns null when no task document exists", async () => { + await makeProject() + expect(await findTaskInstructionFile(dir, dir)).toBeNull() + }) + + test("does not treat README.md as a task document", async () => { + await makeProject() + await fs.writeFile(join(dir, "README.md"), "Create the model `fct_orders`.") + expect(await findTaskInstructionFile(dir, dir)).toBeNull() + }) + + test("finds TASK.md at the workspace root", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "hello") + const found = await findTaskInstructionFile(dir, dir) + expect(found!.path).toBe(join(dir, "TASK.md")) + expect(found!.content).toBe("hello") + }) + + test("finds a task document under .altimate/", async () => { + await makeProject() + await fs.mkdir(join(dir, ".altimate")) + await fs.writeFile(join(dir, ".altimate", "task.md"), "hello") + const found = await findTaskInstructionFile(dir, dir) + // Path case is not asserted: case-insensitive volumes resolve the `TASK.md` + // candidate onto the `task.md` file that exists. + expect(found!.path.toLowerCase()).toBe(join(dir, ".altimate", "task.md").toLowerCase()) + expect(found!.content).toBe("hello") + }) + + test("honours ALTIMATE_VALIDATORS_TASK_FILE ahead of the candidates", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "candidate") + await fs.writeFile(join(dir, "custom-brief.md"), "explicit") + process.env.ALTIMATE_VALIDATORS_TASK_FILE = "custom-brief.md" + expect((await findTaskInstructionFile(dir, dir))!.content).toBe("explicit") + }) + + test("skips an empty task document", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), " \n\n") + expect(await findTaskInstructionFile(dir, dir)).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// run_results / target-path / inventory helpers +// --------------------------------------------------------------------------- + +describe("run_results and inventory helpers", () => { + test("resolveDbtTargetPath defaults to target/", async () => { + await makeProject() + expect(await resolveDbtTargetPath(dir)).toBe(join(dir, "target")) + }) + + test("resolveDbtTargetPath honours target-path in dbt_project.yml", async () => { + await makeProject() + await fs.appendFile(join(dir, "dbt_project.yml"), 'target-path: "build_out"\n') + expect(await resolveDbtTargetPath(dir)).toBe(join(dir, "build_out")) + }) + + test("resolveDbtTargetPath honours DBT_TARGET_PATH", async () => { + await makeProject() + process.env.DBT_TARGET_PATH = "alt_target" + expect(await resolveDbtTargetPath(dir)).toBe(join(dir, "alt_target")) + }) + + test("readRunResults returns null when the artifact is missing", async () => { + await makeProject() + expect(await readRunResults(dir)).toBeNull() + }) + + test("readRunResults returns null on malformed JSON rather than throwing", async () => { + await makeProject() + await fs.mkdir(join(dir, "target")) + await fs.writeFile(join(dir, "target", "run_results.json"), "{not json") + expect(await readRunResults(dir)).toBeNull() + }) + + test("readRunResults parses node names and statuses", async () => { + await makeProject() + await writeRunResults([ + { id: "model.t.stg_orders", status: "success" }, + { id: "model.t.fct_orders", status: "ERROR" }, + ]) + const rr = await readRunResults(dir) + expect(rr!.results.map((r) => r.name)).toEqual(["stg_orders", "fct_orders"]) + expect(rr!.results.map((r) => r.status)).toEqual(["success", "error"]) + }) + + test("isFailedRunStatus treats warn as clean and skipped as failed", () => { + expect(isFailedRunStatus("success")).toBe(false) + expect(isFailedRunStatus("PASS")).toBe(false) + expect(isFailedRunStatus("warn")).toBe(false) + expect(isFailedRunStatus("skipped")).toBe(true) + expect(isFailedRunStatus("error")).toBe(true) + expect(isFailedRunStatus("")).toBe(true) + }) + + test("collectProducedNodeNames unions filesystem names and manifest aliases", async () => { + await makeProject() + await writeModel("stg_orders") + await fs.mkdir(join(dir, "seeds")) + await fs.writeFile(join(dir, "seeds", "country_codes.csv"), "a,b\n") + await fs.mkdir(join(dir, "target"), { recursive: true }) + await fs.writeFile( + join(dir, "target", "manifest.json"), + JSON.stringify({ nodes: { "model.t.x": { name: "x", alias: "renamed_relation" } } }), + ) + const names = await collectProducedNodeNames(dir) + expect(names.has("stg_orders")).toBe(true) + expect(names.has("country_codes")).toBe(true) + expect(names.has("renamed_relation")).toBe(true) + }) + + test("stripSqlComments blanks line, block and Jinja comments", () => { + const out = stripSqlComments("select 1 -- a / b\n/* x / y */ {# z / w #} select 2") + expect(out).not.toContain("a / b") + expect(out).not.toContain("x / y") + expect(out).not.toContain("z / w") + expect(out).toContain("select 2") + }) +}) + +// --------------------------------------------------------------------------- +// The validator +// --------------------------------------------------------------------------- + +describe("DbtNothingBuiltValidator — appliesTo is conservative", () => { + test("does not apply outside a dbt project", async () => { + dir = await fs.mkdtemp(join(tmpdir(), "nothing-built-nodbt-")) + await fs.writeFile(join(dir, "TASK.md"), "Create the model `fct_orders`.") + expect(await DbtNothingBuiltValidator.appliesTo(ctxFuture())).toBe(false) + }) + + test("does not apply to a read-only session with no task document", async () => { + await makeProject() + expect(await DbtNothingBuiltValidator.appliesTo(ctxFuture())).toBe(false) + }) + + test("does not apply when the task document names no deliverables", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "Investigate why the nightly run is slow.") + expect(await DbtNothingBuiltValidator.appliesTo(ctxFuture())).toBe(false) + }) + + test("applies when the task document names deliverables", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "Create the model `fct_orders`.") + expect(await DbtNothingBuiltValidator.appliesTo(ctxFuture())).toBe(true) + }) + + test("applies under the explicit opt-in even without a task document", async () => { + await makeProject() + process.env.ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS = "1" + expect(await DbtNothingBuiltValidator.appliesTo(ctxFuture())).toBe(true) + }) +}) + +describe("DbtNothingBuiltValidator — check", () => { + test("fails a session that authored nothing and built nothing", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "Create the model `fct_orders`.") + const r = await DbtNothingBuiltValidator.check(ctxFuture()) + expect(r.ok).toBe(false) + expect(r.reason).toContain("fct_orders") + expect(r.fixHint).toContain("fct_orders") + expect(r.details!["authored_files"]).toBe(false) + expect(r.details!["fresh_run_results"]).toBe(false) + }) + + test("passes when the session authored a model file", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "Create the model `fct_orders`.") + await writeModel("fct_orders") + const r = await DbtNothingBuiltValidator.check(ctxPast()) + expect(r.ok).toBe(true) + expect(r.details!["authored_files"]).toBe(true) + }) + + test("passes when the session authored a macro rather than a model", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "Create the model `fct_orders`.") + await fs.mkdir(join(dir, "macros")) + await fs.writeFile(join(dir, "macros", "helper.sql"), "{% macro h() %}{% endmacro %}") + const r = await DbtNothingBuiltValidator.check(ctxPast()) + expect(r.ok).toBe(true) + }) + + test("passes on a fresh successful run artifact even with no file writes", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "Create the model `fct_orders`.") + await writeRunResults([{ id: "model.t.fct_orders", status: "success" }]) + const r = await DbtNothingBuiltValidator.check(ctxPast()) + expect(r.ok).toBe(true) + expect(r.details!["fresh_run_results"]).toBe(true) + }) + + test("a stale run artifact does not rescue a session that wrote nothing", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "Create the model `fct_orders`.") + await writeRunResults([{ id: "model.t.fct_orders", status: "success" }]) + const r = await DbtNothingBuiltValidator.check(ctxFuture()) + expect(r.ok).toBe(false) + expect(r.details!["fresh_run_results"]).toBe(false) + }) + + test("an all-failed fresh run artifact does not count as a build", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "Create the model `fct_orders`.") + await writeRunResults([{ id: "model.t.fct_orders", status: "error" }]) + const r = await DbtNothingBuiltValidator.check(ctxFuture()) + expect(r.ok).toBe(false) + }) + + test("opt-in path reports the opt-in expectation and still fails an empty session", async () => { + await makeProject() + process.env.ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS = "1" + const r = await DbtNothingBuiltValidator.check(ctxFuture()) + expect(r.ok).toBe(false) + expect(r.details!["expectation"]).toBe("opt-in") + }) + + test("check soft-passes (no throw) when the project disappears mid-run", async () => { + await makeProject() + const gone = join(dir, "does-not-exist") + const r = await DbtNothingBuiltValidator.check({ ...ctxFuture(), workingDirectory: gone }) + expect(r.ok).toBe(true) + }) +}) +// altimate_change end From 5a7100938bc6c33f9bd044075364b7e71439d019 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 28 Aug 2026 18:20:22 -0700 Subject: [PATCH 02/21] feat(validators): build-green completion gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dbt-build-green` refuses to terminate a session unless a fresh successful build artifact covers the models it edited. Catches three end-states seen in evaluation traces as "declared done, nothing usable on disk": models edited with no artifact at all, an artifact that predates the session, and a fresh artifact in which the edited models errored, are missing, or predate the last edit. Filesystem-only — `/run_results.json` plus model mtimes. No subprocess, no warehouse, no knowledge of the expected output. False-positive guards: - edited nothing and no fresh artifact -> `nothing-to-gate` pass; that case belongs to `dbt-nothing-built`, which only fires when the task demanded artifacts - failures on nodes the session did not touch are recorded in telemetry but never block, so a pre-existing broken model elsewhere cannot trap the loop - when the fresh artifact holds no model nodes (a `dbt test` run overwrites `run_results.json` with test nodes only) build coverage is unknowable, so the coverage assertion is skipped rather than guessed - 1s tolerance on the edited-after-build comparison for mtime granularity 16 tests over every branch, including custom `target-path` and malformed JSON. Co-Authored-By: Claude Fable 5 --- .../altimate/validators/dbt-build-green.ts | 219 +++++++++++++++++ .../opencode/src/altimate/validators/index.ts | 2 + .../validators/dbt-build-green.test.ts | 220 ++++++++++++++++++ 3 files changed, 441 insertions(+) create mode 100644 packages/opencode/src/altimate/validators/dbt-build-green.ts create mode 100644 packages/opencode/test/altimate/validators/dbt-build-green.test.ts diff --git a/packages/opencode/src/altimate/validators/dbt-build-green.ts b/packages/opencode/src/altimate/validators/dbt-build-green.ts new file mode 100644 index 0000000000..84281da8da --- /dev/null +++ b/packages/opencode/src/altimate/validators/dbt-build-green.ts @@ -0,0 +1,219 @@ +// altimate_change start — build-green completion gate +/** + * Build-green completion gate. + * + * Refuses to let a session finish claiming success when the models it edited + * were never successfully built. Three distinct end-states are caught, all of + * them observed in evaluation traces as "declared done, nothing usable on + * disk": + * + * 1. Models were edited but no build artifact exists at all. + * 2. A `run_results.json` exists but predates the session — the agent is + * reading someone else's green build. + * 3. A fresh `run_results.json` exists but the edited models are absent + * from it, are older than the last edit, or built with a failing status. + * + * Everything is read off the filesystem: `/run_results.json` plus + * model mtimes. No subprocess, no warehouse connection, no knowledge of the + * expected answer. + * + * Conservative by construction: + * - When the session edited nothing and no fresh artifact exists, this gate + * stays out of the way (that is `dbt-nothing-built`'s question, and only + * when the task demanded artifacts). + * - Failures on nodes the session did not touch are reported in telemetry + * but never block, so a pre-existing broken model elsewhere in the + * project cannot trap the session in a retry loop. + * - When the fresh artifact contains no model nodes at all (the last + * command was `dbt test`, which overwrites `run_results.json` with test + * nodes only), build coverage cannot be established, so the coverage + * assertion is skipped rather than guessed. + */ + +import { promises as fs } from "fs" +import type { Validator, ValidatorContext, ValidatorResult } from "../../session/validators/types" +import { + findDbtProjectRoot, + modelsModifiedSince, + modelNameFromPath, + readRunResults, + isFailedRunStatus, + type RunResultsArtifact, +} from "./validator-utils" + +/** + * Slack allowed between a model's mtime and the build artifact's mtime before + * the model counts as "edited after the last build". Absorbs filesystem + * timestamp granularity and the ordering of writes inside a single dbt + * invocation; it is not a correctness knob. + */ +const BUILD_FRESHNESS_TOLERANCE_MS = 1_000 + +/** A touched model and what the build artifact says about it. */ +interface ModelBuildState { + name: string + path: string + mtimeMs: number + /** dbt status, or null when the model is absent from the artifact. */ + status: string | null + message: string | null +} + +/** Model-node names present in a run_results artifact. */ +function modelNodeNames(artifact: RunResultsArtifact): Set { + const out = new Set() + for (const r of artifact.results) { + if (r.uniqueId.startsWith("model.")) out.add(r.name) + } + return out +} + +export const DbtBuildGreenValidator: Validator = { + name: "dbt-build-green", + description: + "After the agent declares done, refuses to terminate unless a fresh successful dbt build artifact (`run_results.json` newer than the session start, no failing status) covers every model the session edited.", + + async appliesTo(ctx: ValidatorContext): Promise { + return (await findDbtProjectRoot(ctx.workingDirectory)) !== null + }, + + async check(ctx: ValidatorContext): Promise { + const startedAt = Date.now() + const dbtRoot = await findDbtProjectRoot(ctx.workingDirectory) + if (!dbtRoot) { + return { ok: true, details: { skipped: "no dbt project", session_id: ctx.sessionID } } + } + + const touchedPaths = await modelsModifiedSince(dbtRoot, ctx.sessionStartMs) + const artifact = await readRunResults(dbtRoot) + const artifactIsFresh = artifact !== null && artifact.mtimeMs >= ctx.sessionStartMs + + const baseDetails = { + models_touched: touchedPaths.length, + run_results_path: artifact?.path ?? null, + run_results_fresh: artifactIsFresh, + dbt_root: dbtRoot, + session_id: ctx.sessionID, + elapsed_ms: Date.now() - startedAt, + } + + // Nothing edited and no build of our own: there is no claim to check here. + if (touchedPaths.length === 0 && !artifactIsFresh) { + return { ok: true, details: { ...baseDetails, verdict: "nothing-to-gate" } } + } + + if (touchedPaths.length > 0 && !artifactIsFresh) { + const reason = + artifact === null + ? `You edited ${touchedPaths.length} model(s) but this project has no dbt build artifact — the models were never built, so nothing shows they compile or run.` + : `You edited ${touchedPaths.length} model(s) but the only build artifact (${artifact.path}) predates this session. Your edits have never been built.` + return { + ok: false, + reason, + fixHint: + "Run `dbt build` (or `dbt run` followed by `dbt test`) for the models you changed, confirm it finishes without errors, then declare done. If the build fails, fix the model SQL — do not delete the failing model or narrow the selector to hide it.", + details: { ...baseDetails, verdict: "no-fresh-build" }, + } + } + + // From here on there IS a fresh artifact. + const fresh = artifact as RunResultsArtifact + const modelNodes = modelNodeNames(fresh) + const statusByName = new Map() + for (const r of fresh.results) { + statusByName.set(r.name, { status: r.status, message: r.message }) + } + + const states: ModelBuildState[] = [] + for (const path of touchedPaths) { + const name = modelNameFromPath(path).toLowerCase() + let mtimeMs = 0 + try { + mtimeMs = (await fs.stat(path)).mtimeMs + } catch { + // The file vanished between the scan and now; treat as unknown mtime + // so it can never be reported as "edited after the build". + mtimeMs = 0 + } + const recorded = statusByName.get(name) + states.push({ + name, + path, + mtimeMs, + status: recorded?.status ?? null, + message: recorded?.message ?? null, + }) + } + + const inScope = new Set(states.map((s) => s.name)) + const failedInScope = states.filter((s) => s.status !== null && isFailedRunStatus(s.status)) + // With no edits of our own, the fresh artifact IS this session's build, so + // every failing node in it is in scope. + const failedWholeRun = + touchedPaths.length === 0 + ? fresh.results.filter((r) => isFailedRunStatus(r.status)) + : fresh.results.filter((r) => isFailedRunStatus(r.status) && inScope.has(r.name)) + const failedOutOfScope = fresh.results.filter( + (r) => isFailedRunStatus(r.status) && !inScope.has(r.name), + ).length + + // Coverage is only assertable when the artifact actually recorded models. + const coverageAssertable = modelNodes.size > 0 + const notBuilt = coverageAssertable ? states.filter((s) => s.status === null) : [] + const staleBuild = states.filter( + (s) => s.status !== null && s.mtimeMs > fresh.mtimeMs + BUILD_FRESHNESS_TOLERANCE_MS, + ) + + const details = { + ...baseDetails, + verdict: "fresh-build", + coverage_assertable: coverageAssertable, + model_nodes_in_artifact: modelNodes.size, + failed_in_scope: failedWholeRun.map((r) => r.name), + failed_out_of_scope: failedOutOfScope, + not_built: notBuilt.map((s) => s.name), + stale_build: staleBuild.map((s) => s.name), + } + + if (failedWholeRun.length === 0 && notBuilt.length === 0 && staleBuild.length === 0) { + return { ok: true, details } + } + + const reasonParts: string[] = [] + if (failedWholeRun.length > 0) { + reasonParts.push( + `${failedWholeRun.length} node(s) failed in the last build: ${failedWholeRun.map((r) => `${r.name} (${r.status})`).join(", ")}`, + ) + } + if (notBuilt.length > 0) { + reasonParts.push( + `${notBuilt.length} model(s) you edited were never built: ${notBuilt.map((s) => s.name).join(", ")}`, + ) + } + if (staleBuild.length > 0) { + reasonParts.push( + `${staleBuild.length} model(s) were edited after the last build: ${staleBuild.map((s) => s.name).join(", ")}`, + ) + } + + const hintLines: string[] = [] + for (const failure of failedWholeRun.slice(0, 10)) { + const msg = (failure.message ?? "").split("\n")[0]?.slice(0, 200) + hintLines.push(` • ${failure.name} — ${failure.status}${msg ? `: ${msg}` : ""}`) + } + if (failedWholeRun.length > 10) { + hintLines.push(` • …and ${failedWholeRun.length - 10} more`) + } + hintLines.push( + "Rebuild the models you changed with `dbt build` and make the run finish clean before declaring done. Fix the model SQL rather than removing the model, disabling the test, or narrowing the selector.", + ) + + return { + ok: false, + reason: `The build is not green: ${reasonParts.join("; ")}.`, + fixHint: hintLines.join("\n"), + details, + } + }, +} +// altimate_change end diff --git a/packages/opencode/src/altimate/validators/index.ts b/packages/opencode/src/altimate/validators/index.ts index 8ea0a38179..5674df8f98 100644 --- a/packages/opencode/src/altimate/validators/index.ts +++ b/packages/opencode/src/altimate/validators/index.ts @@ -1,5 +1,6 @@ // altimate_change start — explicit registration entry point for altimate validators import { ValidatorRegistry } from "../../session/validators/registry" +import { DbtBuildGreenValidator } from "./dbt-build-green" import { DbtNothingBuiltValidator } from "./dbt-nothing-built" import { DbtSchemaVerifyValidator } from "./dbt-schema-verify" import { DbtTestsPassValidator } from "./dbt-tests-pass" @@ -21,6 +22,7 @@ import { DbtTestsPassValidator } from "./dbt-tests-pass" */ export function registerAltimateValidators(): void { ValidatorRegistry.register(DbtNothingBuiltValidator) + ValidatorRegistry.register(DbtBuildGreenValidator) ValidatorRegistry.register(DbtSchemaVerifyValidator) ValidatorRegistry.register(DbtTestsPassValidator) } diff --git a/packages/opencode/test/altimate/validators/dbt-build-green.test.ts b/packages/opencode/test/altimate/validators/dbt-build-green.test.ts new file mode 100644 index 0000000000..2021640d07 --- /dev/null +++ b/packages/opencode/test/altimate/validators/dbt-build-green.test.ts @@ -0,0 +1,220 @@ +// altimate_change start — tests for the build-green completion gate +import { describe, expect, test, afterEach } from "bun:test" +import { promises as fs } from "fs" +import { tmpdir } from "os" +import { join } from "path" +import { DbtBuildGreenValidator } from "../../../src/altimate/validators/dbt-build-green" +import type { ValidatorContext } from "../../../src/session/validators/types" + +let dir = "" + +async function makeProject(): Promise { + dir = await fs.mkdtemp(join(tmpdir(), "build-green-")) + await fs.writeFile( + join(dir, "dbt_project.yml"), + "name: t\nversion: '1.0'\nconfig-version: 2\nprofile: t\n", + ) + await fs.mkdir(join(dir, "models"), { recursive: true }) + return dir +} + +async function writeModel(name: string, sql = "select 1 as id"): Promise { + await fs.writeFile(join(dir, "models", `${name}.sql`), sql) +} + +/** Write run_results.json, optionally back- or forward-dating its mtime. */ +async function writeRunResults( + nodes: Array<{ id: string; status: string; message?: string }>, + mtimeOffsetMs = 0, +): Promise { + await fs.mkdir(join(dir, "target"), { recursive: true }) + const path = join(dir, "target", "run_results.json") + await fs.writeFile( + path, + JSON.stringify({ + metadata: { dbt_schema_version: "v5" }, + results: nodes.map((n) => ({ + unique_id: n.id, + status: n.status, + message: n.message ?? null, + })), + }), + ) + if (mtimeOffsetMs !== 0) { + const t = (Date.now() + mtimeOffsetMs) / 1000 + await fs.utimes(path, t, t) + } +} + +const ctx = (overrides: Partial = {}): ValidatorContext => ({ + sessionID: "s", + workingDirectory: dir, + sessionStartMs: 0, + step: 1, + retryCount: 0, + ...overrides, +}) + +afterEach(async () => { + if (dir) await fs.rm(dir, { recursive: true, force: true }) + dir = "" +}) + +describe("DbtBuildGreenValidator — appliesTo", () => { + test("applies inside a dbt project", async () => { + await makeProject() + expect(await DbtBuildGreenValidator.appliesTo(ctx())).toBe(true) + }) + + test("does not apply outside a dbt project", async () => { + dir = await fs.mkdtemp(join(tmpdir(), "build-green-nodbt-")) + expect(await DbtBuildGreenValidator.appliesTo(ctx())).toBe(false) + }) +}) + +describe("DbtBuildGreenValidator — nothing to gate", () => { + test("passes a session that edited nothing and built nothing", async () => { + await makeProject() + await writeModel("stg_orders") + const r = await DbtBuildGreenValidator.check(ctx({ sessionStartMs: Date.now() + 60_000 })) + expect(r.ok).toBe(true) + expect(r.details!["verdict"]).toBe("nothing-to-gate") + }) +}) + +describe("DbtBuildGreenValidator — missing or stale artifact", () => { + test("fails when models were edited and no artifact exists", async () => { + await makeProject() + await writeModel("stg_orders") + const r = await DbtBuildGreenValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.reason).toContain("never built") + expect(r.details!["verdict"]).toBe("no-fresh-build") + }) + + test("fails when the only artifact predates the session", async () => { + await makeProject() + await writeModel("stg_orders") + await writeRunResults([{ id: "model.t.stg_orders", status: "success" }], -600_000) + const r = await DbtBuildGreenValidator.check(ctx({ sessionStartMs: Date.now() - 60_000 })) + expect(r.ok).toBe(false) + expect(r.reason).toContain("predates this session") + expect(r.details!["run_results_fresh"]).toBe(false) + }) +}) + +describe("DbtBuildGreenValidator — fresh artifact", () => { + test("passes when every edited model built successfully", async () => { + await makeProject() + await writeModel("stg_orders") + await writeRunResults([{ id: "model.t.stg_orders", status: "success" }]) + const r = await DbtBuildGreenValidator.check(ctx()) + expect(r.ok).toBe(true) + expect(r.details!["verdict"]).toBe("fresh-build") + }) + + test("treats warn as a clean build", async () => { + await makeProject() + await writeModel("stg_orders") + await writeRunResults([{ id: "model.t.stg_orders", status: "warn" }]) + expect((await DbtBuildGreenValidator.check(ctx())).ok).toBe(true) + }) + + test("fails on an errored edited model and surfaces dbt's message", async () => { + await makeProject() + await writeModel("stg_orders") + await writeRunResults([ + { id: "model.t.stg_orders", status: "error", message: "Compilation Error in model" }, + ]) + const r = await DbtBuildGreenValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.reason).toContain("stg_orders") + expect(r.fixHint).toContain("Compilation Error in model") + }) + + test("fails on a skipped edited model", async () => { + await makeProject() + await writeModel("stg_orders") + await writeRunResults([{ id: "model.t.stg_orders", status: "skipped" }]) + expect((await DbtBuildGreenValidator.check(ctx())).ok).toBe(false) + }) + + test("fails when an edited model is absent from the artifact", async () => { + await makeProject() + await writeModel("stg_orders") + await writeModel("fct_orders") + await writeRunResults([{ id: "model.t.stg_orders", status: "success" }]) + const r = await DbtBuildGreenValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.details!["not_built"]).toEqual(["fct_orders"]) + }) + + test("does not assert coverage when the artifact holds no model nodes", async () => { + // `dbt test` overwrites run_results.json with test nodes only; a missing + // model entry then proves nothing about whether the model was built. + await makeProject() + await writeModel("stg_orders") + await writeRunResults([{ id: "test.t.not_null_stg_orders_id.abc", status: "pass" }]) + const r = await DbtBuildGreenValidator.check(ctx()) + expect(r.ok).toBe(true) + expect(r.details!["coverage_assertable"]).toBe(false) + }) + + test("fails when a model was edited after the last build", async () => { + await makeProject() + await writeRunResults([{ id: "model.t.stg_orders", status: "success" }], -30_000) + await writeModel("stg_orders") + const r = await DbtBuildGreenValidator.check(ctx({ sessionStartMs: Date.now() - 120_000 })) + expect(r.ok).toBe(false) + expect(r.details!["stale_build"]).toEqual(["stg_orders"]) + }) + + test("failures on untouched nodes are reported but do not block", async () => { + await makeProject() + await writeModel("stg_orders") + await writeRunResults([ + { id: "model.t.stg_orders", status: "success" }, + { id: "model.t.some_other_model", status: "error" }, + ]) + const r = await DbtBuildGreenValidator.check(ctx()) + expect(r.ok).toBe(true) + expect(r.details!["failed_out_of_scope"]).toBe(1) + }) + + test("with no edits of our own, every failure in the fresh artifact is in scope", async () => { + await makeProject() + await writeModel("stg_orders") + // Backdate the model so the session touched nothing, but the build is ours. + const old = (Date.now() - 600_000) / 1000 + await fs.utimes(join(dir, "models", "stg_orders.sql"), old, old) + await writeRunResults([{ id: "model.t.stg_orders", status: "error" }]) + const r = await DbtBuildGreenValidator.check(ctx({ sessionStartMs: Date.now() - 60_000 })) + expect(r.ok).toBe(false) + expect(r.details!["models_touched"]).toBe(0) + expect(r.details!["failed_in_scope"]).toEqual(["stg_orders"]) + }) + + test("a malformed artifact is treated as no artifact, not as a crash", async () => { + await makeProject() + await writeModel("stg_orders") + await fs.mkdir(join(dir, "target"), { recursive: true }) + await fs.writeFile(join(dir, "target", "run_results.json"), "{{{") + const r = await DbtBuildGreenValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.details!["verdict"]).toBe("no-fresh-build") + }) + + test("honours a custom target-path", async () => { + await makeProject() + await fs.appendFile(join(dir, "dbt_project.yml"), 'target-path: "build_out"\n') + await writeModel("stg_orders") + await fs.mkdir(join(dir, "build_out"), { recursive: true }) + await fs.writeFile( + join(dir, "build_out", "run_results.json"), + JSON.stringify({ results: [{ unique_id: "model.t.stg_orders", status: "success" }] }), + ) + const r = await DbtBuildGreenValidator.check(ctx()) + expect(r.ok).toBe(true) + }) +}) +// altimate_change end From 8b15d9a1a200ffe6421b90d15728f00044e0c8f2 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 28 Aug 2026 18:22:05 -0700 Subject: [PATCH 03/21] feat(validators): literal-deliverable (spec-name) completion gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fully deterministic loss mode in evaluation traces: the work is functionally reasonable but ships under self-chosen names — a prefix added, a plural dropped, a `_v2` suffix — and the agent then self-verifies against its own renamed output and reports success. The literal contract is never re-read. `dbt-deliverable-names` re-reads it: the deliverable names the task document states literally, diffed against the model, seed and snapshot names the project actually defines. Missing name -> refuse to terminate. Conservative by construction: - required names come only from `extractRequiredDeliverables` (declaration marker, deliverables section, or requirement line; inline code span only; identifier- or path-shaped; stopword-filtered). No fuzzy matching. - no discoverable required-names source -> `appliesTo` false, silent skip, never a false failure - produced names are the union of the filesystem inventory and every `manifest.json` name/alias, so an aliased relation cannot read as missing - comparison is exact (case-insensitive only); a near-miss name is reported as a possible substitute in the hint, never accepted as the deliverable - required column names are deliberately out of scope: asserting a column exists means resolving `select *`, CTEs and upstream schemas, which is SQL analysis rather than a filesystem inventory 15 tests: nesting, aliases, seeds, literal path requirements, case folding, substitute reporting and the silent-skip paths. Co-Authored-By: Claude Fable 5 --- .../validators/dbt-deliverable-names.ts | 172 ++++++++++++++++++ .../opencode/src/altimate/validators/index.ts | 2 + .../validators/dbt-deliverable-names.test.ts | 171 +++++++++++++++++ 3 files changed, 345 insertions(+) create mode 100644 packages/opencode/src/altimate/validators/dbt-deliverable-names.ts create mode 100644 packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts diff --git a/packages/opencode/src/altimate/validators/dbt-deliverable-names.ts b/packages/opencode/src/altimate/validators/dbt-deliverable-names.ts new file mode 100644 index 0000000000..669413df04 --- /dev/null +++ b/packages/opencode/src/altimate/validators/dbt-deliverable-names.ts @@ -0,0 +1,172 @@ +// altimate_change start — literal deliverable / spec-name completion gate +/** + * Literal-deliverable (spec-name) gate. + * + * A recurring, fully deterministic loss mode in evaluation traces: the work is + * functionally reasonable but shipped under self-chosen names — a prefix + * added, a plural dropped, a "v2" suffix, or an entirely different noun — and + * the agent then self-verifies against its own renamed output and reports + * success. The literal contract in the task document is never re-read. + * + * This gate re-reads it. It compares the deliverable names the task states + * **literally** against the names the project actually defines, and refuses + * to terminate when a required name is absent. + * + * Conservatism is the whole design: + * - Required names come only from `extractRequiredDeliverables`, which + * accepts a name solely from an explicit declaration marker, a + * deliverables section, or a requirement line — and only when it sits in + * an inline code span and is identifier- or path-shaped. There is no + * fuzzy matching and no inference. + * - When no required-names source is discoverable, `appliesTo` returns + * false and the session is never inspected. Silence, never a guess. + * - Produced names are the union of the filesystem inventory and every + * `manifest.json` name/alias, so an aliased relation cannot read as + * missing. + * - Comparison is exact (case-insensitive only). A near-miss name is + * reported as a possible substitute in the hint, never accepted as the + * deliverable. + * + * Deliberately out of scope: required *column* names. Asserting a column + * exists means resolving `select *`, CTEs and upstream schemas — real SQL + * analysis, not a filesystem inventory — so it is not attempted here rather + * than attempted badly. + */ + +import { promises as fs } from "fs" +import { join } from "path" +import type { Validator, ValidatorContext, ValidatorResult } from "../../session/validators/types" +import { + findDbtProjectRoot, + findTaskInstructionFile, + extractRequiredDeliverables, + collectProducedNodeNames, + modelsModifiedSince, + modelNameFromPath, + type RequiredDeliverables, +} from "./validator-utils" + +/** The task contract for this workspace, when one is discoverable. */ +interface Contract { + taskFile: string + required: RequiredDeliverables +} + +/** Read the workspace's literal deliverable contract, or null if there is none. */ +async function readContract(cwd: string, dbtRoot: string): Promise { + const task = await findTaskInstructionFile(cwd, dbtRoot) + if (!task) return null + const required = extractRequiredDeliverables(task.content) + if (!required) return null + return { taskFile: task.path, required } +} + +/** True when `relative` exists under either the dbt project or the workspace. */ +async function fileExists(dbtRoot: string, cwd: string, relative: string): Promise { + for (const root of new Set([dbtRoot, cwd])) { + try { + const stat = await fs.stat(join(root, relative)) + if (stat.isFile()) return true + } catch { + // keep looking + } + } + return false +} + +export const DbtDeliverableNamesValidator: Validator = { + name: "dbt-deliverable-names", + description: + "After the agent declares done, compares the deliverable names the task document states literally against the model, seed and snapshot names the project actually defines, and refuses to terminate when a required name is absent — catching renames and self-chosen substitutes.", + + async appliesTo(ctx: ValidatorContext): Promise { + const dbtRoot = await findDbtProjectRoot(ctx.workingDirectory) + if (!dbtRoot) return false + return (await readContract(ctx.workingDirectory, dbtRoot)) !== null + }, + + async check(ctx: ValidatorContext): Promise { + const startedAt = Date.now() + const dbtRoot = await findDbtProjectRoot(ctx.workingDirectory) + if (!dbtRoot) { + return { ok: true, details: { skipped: "no dbt project", session_id: ctx.sessionID } } + } + const contract = await readContract(ctx.workingDirectory, dbtRoot) + if (!contract) { + return { ok: true, details: { skipped: "no literal contract", session_id: ctx.sessionID } } + } + + const produced = await collectProducedNodeNames(dbtRoot) + const missingModels = contract.required.models.filter((name) => !produced.has(name)) + const missingFiles: string[] = [] + for (const relative of contract.required.files) { + if (!(await fileExists(dbtRoot, ctx.workingDirectory, relative))) missingFiles.push(relative) + } + + const details = { + task_file: contract.taskFile, + required_source: contract.required.source, + required_models: contract.required.models, + required_files: contract.required.files, + produced_count: produced.size, + missing_models: missingModels, + missing_files: missingFiles, + dbt_root: dbtRoot, + session_id: ctx.sessionID, + elapsed_ms: Date.now() - startedAt, + } + + if (missingModels.length === 0 && missingFiles.length === 0) { + return { ok: true, details } + } + + // Names this session authored that the task did not ask for. These are the + // likely substitutes behind a missing required name; reported as context, + // never asserted as equivalent. + const requiredSet = new Set(contract.required.models) + const authored = await modelsModifiedSince(dbtRoot, ctx.sessionStartMs) + const unrequested = Array.from( + new Set( + authored + .map((p) => modelNameFromPath(p).toLowerCase()) + .filter((name) => name.length > 0 && !requiredSet.has(name)), + ), + ) + + const reasonParts: string[] = [] + if (missingModels.length > 0) { + reasonParts.push( + `the task names ${missingModels.length} deliverable(s) this project does not define: ${missingModels.join(", ")}`, + ) + } + if (missingFiles.length > 0) { + reasonParts.push(`required file(s) missing: ${missingFiles.join(", ")}`) + } + + const hintLines: string[] = [ + `The task document (${contract.taskFile}) states these names literally. A model that does the right thing under a different name does not satisfy the task, and self-verification against the renamed output will not detect it.`, + ] + if (missingModels.length > 0) { + hintLines.push(` • Create or rename to exactly: ${missingModels.join(", ")}`) + } + if (missingFiles.length > 0) { + hintLines.push(` • Create at exactly these paths: ${missingFiles.join(", ")}`) + } + if (unrequested.length > 0) { + hintLines.push( + ` • Models you created this session that the task did not name: ${unrequested.join(", ")}. If one of them is a renamed version of a required deliverable, rename the file (and any \`ref()\` to it) back to the required name.`, + ) + } + hintLines.push( + " • If a required deliverable is produced under an alias, set `alias` in its config so the required name is the relation name.", + ) + + return { + ok: false, + reason: `Deliverable-name mismatch: ${reasonParts.join("; ")}.`, + fixHint: hintLines.join("\n"), + details: { ...details, unrequested_models: unrequested }, + } + }, +} +// altimate_change end diff --git a/packages/opencode/src/altimate/validators/index.ts b/packages/opencode/src/altimate/validators/index.ts index 5674df8f98..19ecfdfb7f 100644 --- a/packages/opencode/src/altimate/validators/index.ts +++ b/packages/opencode/src/altimate/validators/index.ts @@ -1,6 +1,7 @@ // altimate_change start — explicit registration entry point for altimate validators import { ValidatorRegistry } from "../../session/validators/registry" import { DbtBuildGreenValidator } from "./dbt-build-green" +import { DbtDeliverableNamesValidator } from "./dbt-deliverable-names" import { DbtNothingBuiltValidator } from "./dbt-nothing-built" import { DbtSchemaVerifyValidator } from "./dbt-schema-verify" import { DbtTestsPassValidator } from "./dbt-tests-pass" @@ -23,6 +24,7 @@ import { DbtTestsPassValidator } from "./dbt-tests-pass" export function registerAltimateValidators(): void { ValidatorRegistry.register(DbtNothingBuiltValidator) ValidatorRegistry.register(DbtBuildGreenValidator) + ValidatorRegistry.register(DbtDeliverableNamesValidator) ValidatorRegistry.register(DbtSchemaVerifyValidator) ValidatorRegistry.register(DbtTestsPassValidator) } diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts new file mode 100644 index 0000000000..07dd49118a --- /dev/null +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -0,0 +1,171 @@ +// altimate_change start — tests for the literal deliverable / spec-name gate +import { describe, expect, test, afterEach } from "bun:test" +import { promises as fs } from "fs" +import { tmpdir } from "os" +import { join } from "path" +import { DbtDeliverableNamesValidator } from "../../../src/altimate/validators/dbt-deliverable-names" +import type { ValidatorContext } from "../../../src/session/validators/types" + +let dir = "" + +async function makeProject(): Promise { + dir = await fs.mkdtemp(join(tmpdir(), "deliverable-names-")) + await fs.writeFile( + join(dir, "dbt_project.yml"), + "name: t\nversion: '1.0'\nconfig-version: 2\nprofile: t\n", + ) + await fs.mkdir(join(dir, "models"), { recursive: true }) + return dir +} + +async function writeTask(text: string): Promise { + await fs.writeFile(join(dir, "TASK.md"), text) +} + +async function writeModel(relative: string, sql = "select 1 as id"): Promise { + const path = join(dir, "models", relative) + await fs.mkdir(join(path, ".."), { recursive: true }) + await fs.writeFile(path, sql) +} + +const ctx = (overrides: Partial = {}): ValidatorContext => ({ + sessionID: "s", + workingDirectory: dir, + sessionStartMs: 0, + step: 1, + retryCount: 0, + ...overrides, +}) + +afterEach(async () => { + if (dir) await fs.rm(dir, { recursive: true, force: true }) + dir = "" +}) + +describe("DbtDeliverableNamesValidator — appliesTo is silent without a contract", () => { + test("does not apply outside a dbt project", async () => { + dir = await fs.mkdtemp(join(tmpdir(), "deliverable-names-nodbt-")) + await fs.writeFile(join(dir, "TASK.md"), "Create the model `fct_orders`.") + expect(await DbtDeliverableNamesValidator.appliesTo(ctx())).toBe(false) + }) + + test("does not apply with no task document", async () => { + await makeProject() + await writeModel("stg_orders.sql") + expect(await DbtDeliverableNamesValidator.appliesTo(ctx())).toBe(false) + }) + + test("does not apply when the task document names nothing literally", async () => { + await makeProject() + await writeTask("Build a daily orders summary that the finance team can use.") + expect(await DbtDeliverableNamesValidator.appliesTo(ctx())).toBe(false) + }) + + test("applies once the task names a deliverable", async () => { + await makeProject() + await writeTask("Create the model `fct_orders`.") + expect(await DbtDeliverableNamesValidator.appliesTo(ctx())).toBe(true) + }) +}) + +describe("DbtDeliverableNamesValidator — check", () => { + test("passes when every required name exists", async () => { + await makeProject() + await writeTask("## Required deliverables\n- `stg_orders`\n- `fct_orders`\n") + await writeModel("stg_orders.sql") + await writeModel("marts/fct_orders.sql") + const r = await DbtDeliverableNamesValidator.check(ctx()) + expect(r.ok).toBe(true) + expect(r.details!["missing_models"]).toEqual([]) + }) + + test("required names match regardless of directory nesting", async () => { + await makeProject() + await writeTask("Create the model `fct_orders`.") + await writeModel("marts/finance/deep/fct_orders.sql") + expect((await DbtDeliverableNamesValidator.check(ctx())).ok).toBe(true) + }) + + test("fails on a renamed deliverable and names the likely substitute", async () => { + await makeProject() + await writeTask("Create the model `fct_orders`.") + await writeModel("fct_orders_v2.sql") + const r = await DbtDeliverableNamesValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.details!["missing_models"]).toEqual(["fct_orders"]) + expect(r.fixHint).toContain("fct_orders_v2") + }) + + test("does not list an unrequested model the session did not author", async () => { + await makeProject() + await writeTask("Create the model `fct_orders`.") + await writeModel("pre_existing.sql") + const old = (Date.now() - 600_000) / 1000 + await fs.utimes(join(dir, "models", "pre_existing.sql"), old, old) + const r = await DbtDeliverableNamesValidator.check(ctx({ sessionStartMs: Date.now() - 60_000 })) + expect(r.ok).toBe(false) + expect(r.details!["unrequested_models"]).toEqual([]) + }) + + test("an alias recorded in manifest.json satisfies the required name", async () => { + await makeProject() + await writeTask("Create the model `fct_orders`.") + await writeModel("orders_fact.sql") + await fs.mkdir(join(dir, "target"), { recursive: true }) + await fs.writeFile( + join(dir, "target", "manifest.json"), + JSON.stringify({ nodes: { "model.t.orders_fact": { name: "orders_fact", alias: "fct_orders" } } }), + ) + expect((await DbtDeliverableNamesValidator.check(ctx())).ok).toBe(true) + }) + + test("a seed satisfies a required name", async () => { + await makeProject() + await writeTask("## Deliverables\n- `country_codes`\n") + await fs.mkdir(join(dir, "seeds")) + await fs.writeFile(join(dir, "seeds", "country_codes.csv"), "a,b\n") + expect((await DbtDeliverableNamesValidator.check(ctx())).ok).toBe(true) + }) + + test("required literal file paths are checked as paths", async () => { + await makeProject() + await writeTask("Create the model `models/marts/fct_orders.sql`.") + // Right model name, wrong path. + await writeModel("fct_orders.sql") + const r = await DbtDeliverableNamesValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.details!["missing_files"]).toEqual(["models/marts/fct_orders.sql"]) + expect(r.details!["missing_models"]).toEqual([]) + }) + + test("passes when the required literal path exists", async () => { + await makeProject() + await writeTask("Create the model `models/marts/fct_orders.sql`.") + await writeModel("marts/fct_orders.sql") + expect((await DbtDeliverableNamesValidator.check(ctx())).ok).toBe(true) + }) + + test("matching is case-insensitive", async () => { + await makeProject() + await writeTask("## Required\n- `FCT_Orders`\n") + await writeModel("fct_orders.sql") + expect((await DbtDeliverableNamesValidator.check(ctx())).ok).toBe(true) + }) + + test("declaration marker drives the contract when present", async () => { + await makeProject() + await writeTask("\nDo whatever you like.") + const r = await DbtDeliverableNamesValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.details!["required_source"]).toBe("declaration") + }) + + test("check soft-passes when the workspace disappears", async () => { + await makeProject() + const r = await DbtDeliverableNamesValidator.check( + ctx({ workingDirectory: join(dir, "gone") }), + ) + expect(r.ok).toBe(true) + }) +}) +// altimate_change end From 830e4447eadad11ed40c8f6588e81cca1628af88 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 28 Aug 2026 18:24:14 -0700 Subject: [PATCH 04/21] feat(validators): incremental-config consistency lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dbt-incremental-config` flags configurations that contradict themselves, not absences — dbt legitimately supports append-only and keyless incremental models, so a missing `unique_key` is only a defect when the declared strategy needs one. Three inconsistencies, all grep-level over the edited model source with comments stripped: - `incremental_strategy='merge'` / `'delete+insert'` with no `unique_key`: dbt has nothing to match rows on, so the model silently appends duplicates - no `is_incremental()` guard in an incremental model when the workspace task document literally asks for idempotent re-runs (and only then) - a non-deterministic call (`current_timestamp`, `random()`, …) inside the `is_incremental()` predicate, which makes the selected row set differ run to run. The same functions elsewhere in the model — an audit column, say — are recorded as advisories in telemetry and never block. Config inherited from `dbt_project.yml` is deliberately not resolved: doing it properly means materialising dbt's config inheritance, and guessing it trades a real check for false failures. 16 tests including the intentional-append, guarded-model and advisory-not-blocking paths. Co-Authored-By: Claude Fable 5 --- .../validators/dbt-incremental-config.ts | 224 +++++++++++++++++ .../opencode/src/altimate/validators/index.ts | 2 + .../validators/dbt-incremental-config.test.ts | 230 ++++++++++++++++++ 3 files changed, 456 insertions(+) create mode 100644 packages/opencode/src/altimate/validators/dbt-incremental-config.ts create mode 100644 packages/opencode/test/altimate/validators/dbt-incremental-config.test.ts diff --git a/packages/opencode/src/altimate/validators/dbt-incremental-config.ts b/packages/opencode/src/altimate/validators/dbt-incremental-config.ts new file mode 100644 index 0000000000..01608c47c0 --- /dev/null +++ b/packages/opencode/src/altimate/validators/dbt-incremental-config.ts @@ -0,0 +1,224 @@ +// altimate_change start — incremental-config consistency lint +/** + * Incremental-config lint. + * + * dbt's incremental materialisation is legitimately flexible: append-only + * models are normal, keyless models are normal, and a non-deterministic + * expression in a projected column is normal. So this lint does not flag + * *absences* — it flags **inconsistencies**, configurations that contradict + * themselves and therefore cannot be what the author meant: + * + * 1. Upsert semantics declared without a key. `incremental_strategy='merge'` + * or `'delete+insert'` need a `unique_key` to match rows on. Without one + * dbt cannot upsert, and the model silently degrades to appending + * duplicates on every run. + * 2. No `is_incremental()` guard in a model whose task demands idempotent + * re-runs. Only raised when the workspace task document literally says + * idempotent/idempotency — otherwise dbt's full-refresh-every-run + * behaviour is a valid choice and the lint stays quiet. + * 3. A non-deterministic function inside the `is_incremental()` predicate. + * A high-water mark computed from `current_timestamp` / `random()` + * selects a different row set on every run, so the model is not + * reproducible by construction. The same functions elsewhere in the + * model (an `updated_at` audit column, say) are recorded for telemetry + * but never block. + * + * Grep-level over the model source with comments stripped. Config declared in + * `dbt_project.yml` rather than in the model is intentionally not resolved: + * that would require materialising dbt's config inheritance, and guessing it + * would trade a real inconsistency check for false failures. + */ + +import { promises as fs } from "fs" +import type { Validator, ValidatorContext, ValidatorResult } from "../../session/validators/types" +import { + findDbtProjectRoot, + findTaskInstructionFile, + modelsModifiedSince, + modelNameFromPath, + stripSqlComments, +} from "./validator-utils" + +/** `{{ config(...) }}` call, capturing its argument text. */ +const CONFIG_CALL_RE = /\{\{-?\s*config\s*\(([\s\S]*?)\)\s*-?\}\}/gi +/** In-model incremental materialisation. */ +const INCREMENTAL_RE = /materiali[sz]ed\s*=\s*['"]incremental['"]/i +/** Declared incremental strategy. */ +const STRATEGY_RE = /incremental_strategy\s*=\s*['"]([a-z0-9_+]+)['"]/i +/** Any `unique_key=` in the config args. */ +const UNIQUE_KEY_RE = /unique_key\s*=/i +/** The `is_incremental()` guard call. */ +const IS_INCREMENTAL_RE = /is_incremental\s*\(\s*\)/i +/** Body of the first `{% if is_incremental() %} … {% endif %}` block. */ +const IS_INCREMENTAL_BLOCK_RE = + /\{%-?\s*if\s+is_incremental\s*\(\s*\)\s*-?%\}([\s\S]*?)\{%-?\s*endif\s*-?%\}/gi +/** Functions whose value changes between otherwise identical runs. */ +const NONDETERMINISTIC_RE = + /\b(current_timestamp|current_date|localtimestamp|getdate|sysdate|now|random|rand|uuid_string|gen_random_uuid|newid)\b/gi +/** The task literally asks for repeatable re-runs. */ +const IDEMPOTENCY_RE = /\bidempoten(?:t|cy|tly)\b/i + +/** Strategies whose semantics require a key to match rows on. */ +const KEYED_STRATEGIES = new Set(["merge", "delete+insert"]) + +/** One inconsistency found in one model. */ +interface Finding { + model: string + kind: "upsert-without-unique-key" | "missing-is-incremental-guard" | "nondeterministic-predicate" + detail: string +} + +/** Concatenate the argument text of every `{{ config() }}` call in a model. */ +function configArgs(sql: string): string { + const parts: string[] = [] + CONFIG_CALL_RE.lastIndex = 0 + let m: RegExpExecArray | null + while ((m = CONFIG_CALL_RE.exec(sql)) !== null) { + if (m[1]) parts.push(m[1]) + } + return parts.join("\n") +} + +/** Concatenate the bodies of every `is_incremental()` guard block. */ +function incrementalPredicates(sql: string): string { + const parts: string[] = [] + IS_INCREMENTAL_BLOCK_RE.lastIndex = 0 + let m: RegExpExecArray | null + while ((m = IS_INCREMENTAL_BLOCK_RE.exec(sql)) !== null) { + if (m[1]) parts.push(m[1]) + } + return parts.join("\n") +} + +/** Distinct non-deterministic function names appearing in a fragment. */ +function nondeterministicCalls(fragment: string): string[] { + const out = new Set() + NONDETERMINISTIC_RE.lastIndex = 0 + let m: RegExpExecArray | null + while ((m = NONDETERMINISTIC_RE.exec(fragment)) !== null) { + if (m[1]) out.add(m[1].toLowerCase()) + } + return Array.from(out) +} + +export const DbtIncrementalConfigValidator: Validator = { + name: "dbt-incremental-config", + description: + "After the agent declares done, lints the incremental models the session edited for self-contradictory configuration: upsert semantics declared without a unique_key, a missing is_incremental() guard where the task demands idempotent re-runs, and non-deterministic functions inside the incremental predicate.", + + async appliesTo(ctx: ValidatorContext): Promise { + return (await findDbtProjectRoot(ctx.workingDirectory)) !== null + }, + + async check(ctx: ValidatorContext): Promise { + const startedAt = Date.now() + const dbtRoot = await findDbtProjectRoot(ctx.workingDirectory) + if (!dbtRoot) { + return { ok: true, details: { skipped: "no dbt project", session_id: ctx.sessionID } } + } + const touched = await modelsModifiedSince(dbtRoot, ctx.sessionStartMs) + if (touched.length === 0) { + return { ok: true, details: { models_touched: 0, session_id: ctx.sessionID } } + } + + const task = await findTaskInstructionFile(ctx.workingDirectory, dbtRoot) + const idempotencyDemanded = task !== null && IDEMPOTENCY_RE.test(task.content) + + const findings: Finding[] = [] + const advisories: Array<{ model: string; functions: string[] }> = [] + let incrementalModels = 0 + + for (const path of touched) { + let raw: string + try { + raw = await fs.readFile(path, "utf8") + } catch { + continue + } + const sql = stripSqlComments(raw) + const args = configArgs(sql) + if (!INCREMENTAL_RE.test(args)) continue + incrementalModels++ + const model = modelNameFromPath(path) + + const strategyMatch = STRATEGY_RE.exec(args) + const strategy = strategyMatch?.[1]?.toLowerCase() ?? null + if (strategy && KEYED_STRATEGIES.has(strategy) && !UNIQUE_KEY_RE.test(args)) { + findings.push({ + model, + kind: "upsert-without-unique-key", + detail: `\`incremental_strategy='${strategy}'\` declares upsert semantics but no \`unique_key\` is configured, so dbt has nothing to match rows on and every run appends instead of updating.`, + }) + } + + const hasGuard = IS_INCREMENTAL_RE.test(sql) + if (idempotencyDemanded && !hasGuard) { + findings.push({ + model, + kind: "missing-is-incremental-guard", + detail: + "The task asks for idempotent re-runs, but this incremental model has no `is_incremental()` guard, so a re-run reprocesses the full source into an existing table.", + }) + } + + const predicate = incrementalPredicates(sql) + const predicateCalls = nondeterministicCalls(predicate) + if (predicateCalls.length > 0) { + findings.push({ + model, + kind: "nondeterministic-predicate", + detail: `The \`is_incremental()\` predicate uses ${predicateCalls.join(", ")}, so each run selects a different row set and the model cannot reproduce its own output.`, + }) + } + + const elsewhere = nondeterministicCalls(sql).filter((f) => !predicateCalls.includes(f)) + if (elsewhere.length > 0) advisories.push({ model, functions: elsewhere }) + } + + const details = { + models_touched: touched.length, + incremental_models: incrementalModels, + idempotency_demanded: idempotencyDemanded, + findings: findings.map((f) => ({ model: f.model, kind: f.kind })), + advisories, + dbt_root: dbtRoot, + session_id: ctx.sessionID, + elapsed_ms: Date.now() - startedAt, + } + + if (findings.length === 0) return { ok: true, details } + + const byModel = new Map() + for (const f of findings) { + const list = byModel.get(f.model) ?? [] + list.push(f) + byModel.set(f.model, list) + } + const hintLines: string[] = [] + for (const [model, list] of byModel) { + hintLines.push(`Model \`${model}\`:`) + for (const f of list) hintLines.push(` • ${f.detail}`) + } + hintLines.push("") + hintLines.push( + "These are configuration inconsistencies, not style rules — dbt supports append-only and keyless incremental models, so fix the contradiction rather than adding boilerplate:", + ) + hintLines.push( + " • Upsert without a key: add `unique_key=` naming the grain, or state the intent by setting `incremental_strategy='append'`.", + ) + hintLines.push( + " • Missing guard: wrap the incremental filter in `{% if is_incremental() %} … {% endif %}` so a re-run only picks up new rows.", + ) + hintLines.push( + " • Non-deterministic predicate: compare against a value read from the existing table (`select max(col) from {{ this }}`) instead of a clock or random call.", + ) + + return { + ok: false, + reason: `${findings.length} incremental-configuration inconsistency(ies) in ${byModel.size} model(s) you edited: ${Array.from(byModel.keys()).join(", ")}.`, + fixHint: hintLines.join("\n"), + details, + } + }, +} +// altimate_change end diff --git a/packages/opencode/src/altimate/validators/index.ts b/packages/opencode/src/altimate/validators/index.ts index 19ecfdfb7f..2988671ed1 100644 --- a/packages/opencode/src/altimate/validators/index.ts +++ b/packages/opencode/src/altimate/validators/index.ts @@ -2,6 +2,7 @@ import { ValidatorRegistry } from "../../session/validators/registry" import { DbtBuildGreenValidator } from "./dbt-build-green" import { DbtDeliverableNamesValidator } from "./dbt-deliverable-names" +import { DbtIncrementalConfigValidator } from "./dbt-incremental-config" import { DbtNothingBuiltValidator } from "./dbt-nothing-built" import { DbtSchemaVerifyValidator } from "./dbt-schema-verify" import { DbtTestsPassValidator } from "./dbt-tests-pass" @@ -25,6 +26,7 @@ export function registerAltimateValidators(): void { ValidatorRegistry.register(DbtNothingBuiltValidator) ValidatorRegistry.register(DbtBuildGreenValidator) ValidatorRegistry.register(DbtDeliverableNamesValidator) + ValidatorRegistry.register(DbtIncrementalConfigValidator) ValidatorRegistry.register(DbtSchemaVerifyValidator) ValidatorRegistry.register(DbtTestsPassValidator) } diff --git a/packages/opencode/test/altimate/validators/dbt-incremental-config.test.ts b/packages/opencode/test/altimate/validators/dbt-incremental-config.test.ts new file mode 100644 index 0000000000..a9f03d689b --- /dev/null +++ b/packages/opencode/test/altimate/validators/dbt-incremental-config.test.ts @@ -0,0 +1,230 @@ +// altimate_change start — tests for the incremental-config consistency lint +import { describe, expect, test, afterEach } from "bun:test" +import { promises as fs } from "fs" +import { tmpdir } from "os" +import { join } from "path" +import { DbtIncrementalConfigValidator } from "../../../src/altimate/validators/dbt-incremental-config" +import type { ValidatorContext } from "../../../src/session/validators/types" + +let dir = "" + +async function makeProject(): Promise { + dir = await fs.mkdtemp(join(tmpdir(), "incremental-config-")) + await fs.writeFile( + join(dir, "dbt_project.yml"), + "name: t\nversion: '1.0'\nconfig-version: 2\nprofile: t\n", + ) + await fs.mkdir(join(dir, "models"), { recursive: true }) + return dir +} + +async function writeModel(name: string, sql: string): Promise { + await fs.writeFile(join(dir, "models", `${name}.sql`), sql) +} + +const ctx = (overrides: Partial = {}): ValidatorContext => ({ + sessionID: "s", + workingDirectory: dir, + sessionStartMs: 0, + step: 1, + retryCount: 0, + ...overrides, +}) + +afterEach(async () => { + if (dir) await fs.rm(dir, { recursive: true, force: true }) + dir = "" +}) + +describe("DbtIncrementalConfigValidator — scope", () => { + test("applies inside a dbt project only", async () => { + await makeProject() + expect(await DbtIncrementalConfigValidator.appliesTo(ctx())).toBe(true) + const outside = await fs.mkdtemp(join(tmpdir(), "incremental-config-nodbt-")) + expect( + await DbtIncrementalConfigValidator.appliesTo(ctx({ workingDirectory: outside })), + ).toBe(false) + await fs.rm(outside, { recursive: true, force: true }) + }) + + test("ignores non-incremental models entirely", async () => { + await makeProject() + await writeModel( + "stg_orders", + "{{ config(materialized='table') }}\nselect current_timestamp as loaded_at, 1 as id", + ) + const r = await DbtIncrementalConfigValidator.check(ctx()) + expect(r.ok).toBe(true) + expect(r.details!["incremental_models"]).toBe(0) + }) + + test("passes when the session touched nothing", async () => { + await makeProject() + await writeModel("stg_orders", "{{ config(materialized='incremental') }} select 1 as id") + const r = await DbtIncrementalConfigValidator.check(ctx({ sessionStartMs: Date.now() + 60_000 })) + expect(r.ok).toBe(true) + expect(r.details!["models_touched"]).toBe(0) + }) +}) + +describe("DbtIncrementalConfigValidator — upsert without a key", () => { + test("flags merge strategy with no unique_key", async () => { + await makeProject() + await writeModel( + "fct_orders", + "{{ config(materialized='incremental', incremental_strategy='merge') }}\nselect 1 as id", + ) + const r = await DbtIncrementalConfigValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.fixHint).toContain("unique_key") + expect((r.details!["findings"] as Array<{ kind: string }>)[0]!.kind).toBe( + "upsert-without-unique-key", + ) + }) + + test("flags delete+insert with no unique_key", async () => { + await makeProject() + await writeModel( + "fct_orders", + "{{ config(materialized='incremental', incremental_strategy='delete+insert') }}\nselect 1 as id", + ) + expect((await DbtIncrementalConfigValidator.check(ctx())).ok).toBe(false) + }) + + test("accepts merge with a unique_key", async () => { + await makeProject() + await writeModel( + "fct_orders", + "{{ config(materialized='incremental', incremental_strategy='merge', unique_key='order_id') }}\nselect 1 as order_id", + ) + expect((await DbtIncrementalConfigValidator.check(ctx())).ok).toBe(true) + }) + + test("accepts an explicit keyless append strategy", async () => { + await makeProject() + await writeModel( + "events", + "{{ config(materialized='incremental', incremental_strategy='append') }}\nselect 1 as id", + ) + expect((await DbtIncrementalConfigValidator.check(ctx())).ok).toBe(true) + }) + + test("accepts an incremental model that declares no strategy at all", async () => { + await makeProject() + await writeModel("events", "{{ config(materialized='incremental') }}\nselect 1 as id") + expect((await DbtIncrementalConfigValidator.check(ctx())).ok).toBe(true) + }) + + test("ignores a strategy that only appears in a comment", async () => { + await makeProject() + await writeModel( + "events", + "-- incremental_strategy='merge' was considered\n{{ config(materialized='incremental') }}\nselect 1 as id", + ) + expect((await DbtIncrementalConfigValidator.check(ctx())).ok).toBe(true) + }) +}) + +describe("DbtIncrementalConfigValidator — is_incremental guard", () => { + test("stays quiet about a missing guard when the task says nothing about idempotency", async () => { + await makeProject() + await writeModel("events", "{{ config(materialized='incremental') }}\nselect 1 as id") + expect((await DbtIncrementalConfigValidator.check(ctx())).ok).toBe(true) + }) + + test("flags a missing guard when the task demands idempotent re-runs", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "The model must be idempotent across re-runs.") + await writeModel("events", "{{ config(materialized='incremental') }}\nselect 1 as id") + const r = await DbtIncrementalConfigValidator.check(ctx()) + expect(r.ok).toBe(false) + expect((r.details!["findings"] as Array<{ kind: string }>)[0]!.kind).toBe( + "missing-is-incremental-guard", + ) + }) + + test("accepts a guarded model when idempotency is demanded", async () => { + await makeProject() + await fs.writeFile(join(dir, "TASK.md"), "The model must be idempotent across re-runs.") + await writeModel( + "events", + [ + "{{ config(materialized='incremental') }}", + "select * from {{ ref('src') }}", + "{% if is_incremental() %}", + " where loaded_at > (select max(loaded_at) from {{ this }})", + "{% endif %}", + ].join("\n"), + ) + expect((await DbtIncrementalConfigValidator.check(ctx())).ok).toBe(true) + }) +}) + +describe("DbtIncrementalConfigValidator — non-determinism", () => { + test("flags a clock call inside the incremental predicate", async () => { + await makeProject() + await writeModel( + "events", + [ + "{{ config(materialized='incremental') }}", + "select * from {{ ref('src') }}", + "{% if is_incremental() %}", + " where loaded_at > current_timestamp - interval '1 day'", + "{% endif %}", + ].join("\n"), + ) + const r = await DbtIncrementalConfigValidator.check(ctx()) + expect(r.ok).toBe(false) + expect((r.details!["findings"] as Array<{ kind: string }>)[0]!.kind).toBe( + "nondeterministic-predicate", + ) + expect(r.fixHint).toContain("max(col)") + }) + + test("records but does not block on a clock call in a projected column", async () => { + await makeProject() + await writeModel( + "events", + [ + "{{ config(materialized='incremental') }}", + "select id, current_timestamp as dbt_loaded_at from {{ ref('src') }}", + "{% if is_incremental() %}", + " where loaded_at > (select max(loaded_at) from {{ this }})", + "{% endif %}", + ].join("\n"), + ) + const r = await DbtIncrementalConfigValidator.check(ctx()) + expect(r.ok).toBe(true) + expect(r.details!["advisories"]).toEqual([ + { model: "events", functions: ["current_timestamp"] }, + ]) + }) +}) + +describe("DbtIncrementalConfigValidator — robustness", () => { + test("aggregates findings across several models", async () => { + await makeProject() + await writeModel( + "a", + "{{ config(materialized='incremental', incremental_strategy='merge') }} select 1 as id", + ) + await writeModel( + "b", + "{{ config(materialized='incremental', incremental_strategy='delete+insert') }} select 1 as id", + ) + const r = await DbtIncrementalConfigValidator.check(ctx()) + expect(r.ok).toBe(false) + expect((r.details!["findings"] as unknown[]).length).toBe(2) + expect(r.reason).toContain("a") + expect(r.reason).toContain("b") + }) + + test("tolerates an unreadable model file without throwing", async () => { + await makeProject() + await writeModel("ok_model", "{{ config(materialized='table') }} select 1 as id") + await fs.mkdir(join(dir, "models", "weird.sql")) + const r = await DbtIncrementalConfigValidator.check(ctx()) + expect(r.ok).toBe(true) + }) +}) +// altimate_change end From 13b21d9f727e7193abbb51060c1c1eafea02a738 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 28 Aug 2026 18:26:15 -0700 Subject: [PATCH 05/21] feat(validators): dialect-guard lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dbt-dialect-guard` flags warehouse-specific SQL used in the models a session edited without the project's prescribed `target.type` Jinja guard. The failure it catches: reaching for a function known from one warehouse, which compiles on the development target and breaks everywhere else. Only speaks when the project actually prescribes the convention — `target.type` must already appear under `models/` or `macros/`, or `ALTIMATE_VALIDATORS_DIALECT_GUARD=1` must be set. A single-warehouse project never sees this validator, because there warehouse-specific SQL is just correct SQL. Grep-level: comments stripped, `target.type`-guarded Jinja blocks blanked, then a curated call-shaped function list (Snowflake / BigQuery / DuckDB / Redshift) matched over what remains. Curated for precision rather than coverage; a project macro sharing a name with a listed builtin is the known residual false positive, which is why the message is advisory and names the guard to add. 14 tests including guarded usage, portable SQL, comment-only mentions and same-named column references. Co-Authored-By: Claude Fable 5 --- .../altimate/validators/dbt-dialect-guard.ts | 223 ++++++++++++++++++ .../opencode/src/altimate/validators/index.ts | 2 + .../validators/dbt-dialect-guard.test.ts | 172 ++++++++++++++ 3 files changed, 397 insertions(+) create mode 100644 packages/opencode/src/altimate/validators/dbt-dialect-guard.ts create mode 100644 packages/opencode/test/altimate/validators/dbt-dialect-guard.test.ts diff --git a/packages/opencode/src/altimate/validators/dbt-dialect-guard.ts b/packages/opencode/src/altimate/validators/dbt-dialect-guard.ts new file mode 100644 index 0000000000..eb9de8def1 --- /dev/null +++ b/packages/opencode/src/altimate/validators/dbt-dialect-guard.ts @@ -0,0 +1,223 @@ +// altimate_change start — dialect-guard lint +/** + * Dialect-guard lint. + * + * A project that has to run against more than one warehouse establishes a + * convention for it: warehouse-specific SQL sits behind a `target.type` Jinja + * guard (or behind a dispatched macro, which resolves to no raw + * warehouse-specific function text at all). Sessions routinely reach for a + * function they know from one warehouse and drop it in unguarded; the model + * compiles and runs on the development target and breaks everywhere else. + * + * This lint flags warehouse-specific function usage in the models the session + * edited that is not inside a `target.type` guard. + * + * It only speaks when the project actually prescribes the convention. Evidence + * is that `target.type` already appears somewhere under `models/` or + * `macros/`; alternatively `ALTIMATE_VALIDATORS_DIALECT_GUARD=1` forces it on. + * A single-warehouse project never sees this validator, because there + * warehouse-specific SQL is simply correct SQL. + * + * Grep-level by design. The function list is curated for precision rather + * than coverage: only functions whose availability genuinely differs across + * the warehouses this product targets, matched with a call-shaped pattern so + * a same-named column cannot trigger them. A project macro that happens to + * share a name with a listed function is the known residual false positive, + * which is why the message is advisory and names the guard to add. + */ + +import { promises as fs } from "fs" +import { join } from "path" +import type { Validator, ValidatorContext, ValidatorResult } from "../../session/validators/types" +import { + findDbtProjectRoot, + modelsModifiedSince, + modelNameFromPath, + stripSqlComments, +} from "./validator-utils" + +/** Env flag that forces the lint on for a project with no guards yet. */ +const OPT_IN_ENV = "ALTIMATE_VALIDATORS_DIALECT_GUARD" + +/** One warehouse-specific construct and where it is available. */ +interface DialectFunction { + /** Display name used in the message. */ + name: string + /** Warehouses that provide it. */ + dialects: string + /** Call-shaped matcher. */ + pattern: RegExp +} + +/** + * Curated list. Each entry is a construct that is unavailable — or means + * something different — on at least one warehouse this product targets, so + * using it unguarded is a portability defect rather than a style choice. + */ +const DIALECT_FUNCTIONS: DialectFunction[] = [ + { name: "iff()", dialects: "Snowflake", pattern: /\biff\s*\(/gi }, + { name: "zeroifnull()", dialects: "Snowflake", pattern: /\bzeroifnull\s*\(/gi }, + { name: "div0()", dialects: "Snowflake", pattern: /\bdiv0\s*\(/gi }, + { name: "nvl2()", dialects: "Snowflake / Redshift", pattern: /\bnvl2\s*\(/gi }, + { name: "try_to_number()", dialects: "Snowflake", pattern: /\btry_to_(?:number|date|timestamp)\s*\(/gi }, + { name: "object_construct()", dialects: "Snowflake", pattern: /\bobject_construct\s*\(/gi }, + { name: "parse_json()", dialects: "Snowflake", pattern: /\bparse_json\s*\(/gi }, + { name: "to_varchar()", dialects: "Snowflake", pattern: /\bto_varchar\s*\(/gi }, + { name: "listagg()", dialects: "Snowflake / Redshift / Oracle", pattern: /\blistagg\s*\(/gi }, + { name: "safe_cast()", dialects: "BigQuery", pattern: /\bsafe_cast\s*\(/gi }, + { name: "safe_divide()", dialects: "BigQuery", pattern: /\bsafe_divide\s*\(/gi }, + { name: "generate_date_array()", dialects: "BigQuery", pattern: /\bgenerate_date_array\s*\(/gi }, + { name: "approx_quantiles()", dialects: "BigQuery", pattern: /\bapprox_quantiles\s*\(/gi }, + { name: "_TABLE_SUFFIX", dialects: "BigQuery", pattern: /\b_table_suffix\b/gi }, + { name: "read_csv_auto()", dialects: "DuckDB", pattern: /\bread_csv_auto\s*\(/gi }, + { name: "read_parquet()", dialects: "DuckDB", pattern: /\bread_parquet\s*\(/gi }, + { name: "list_transform()", dialects: "DuckDB", pattern: /\blist_(?:transform|aggregate|value)\s*\(/gi }, + { name: "epoch_ms()", dialects: "DuckDB", pattern: /\bepoch_ms\s*\(/gi }, + { name: "getdate()", dialects: "Redshift / SQL Server", pattern: /\bgetdate\s*\(/gi }, +] + +/** A Jinja `if` whose condition mentions `target.type`, through its `endif`. */ +const TARGET_TYPE_GUARD_RE = /\{%-?\s*if\b[^%]*target\.type[\s\S]*?\{%-?\s*endif\s*-?%\}/gi +/** Bare mention of the guard variable, used as the project-convention probe. */ +const TARGET_TYPE_RE = /target\.type/i + +/** Depth limit mirroring the other project scans in this lane. */ +const SCAN_MAX_DEPTH = 8 + +/** One unguarded construct in one model. */ +interface Finding { + model: string + function: string + dialects: string +} + +/** + * True when the project already guards on `target.type` anywhere under + * `models/` or `macros/` — the evidence that this project prescribes the + * convention this lint enforces. + */ +async function projectPrescribesGuards(dbtRoot: string): Promise { + async function scan(dir: string, depth: number): Promise { + if (depth > SCAN_MAX_DEPTH) return false + let entries: import("fs").Dirent[] + try { + entries = await fs.readdir(dir, { withFileTypes: true }) + } catch { + return false + } + for (const entry of entries) { + if (entry.name.startsWith(".") || entry.name === "node_modules" || entry.name === "target") { + continue + } + const full = join(dir, entry.name) + let stat: import("fs").Stats + try { + stat = await fs.stat(full) + } catch { + continue + } + if (stat.isDirectory()) { + if (await scan(full, depth + 1)) return true + } else if (stat.isFile() && entry.name.toLowerCase().endsWith(".sql")) { + try { + if (TARGET_TYPE_RE.test(await fs.readFile(full, "utf8"))) return true + } catch { + // unreadable — keep scanning + } + } + } + return false + } + for (const dir of ["models", "macros"]) { + if (await scan(join(dbtRoot, dir), 0)) return true + } + return false +} + +/** Blank out every `target.type`-guarded Jinja block. */ +function stripGuardedBlocks(sql: string): string { + return sql.replace(TARGET_TYPE_GUARD_RE, (m) => " ".repeat(m.length)) +} + +export const DbtDialectGuardValidator: Validator = { + name: "dbt-dialect-guard", + description: + "After the agent declares done, flags warehouse-specific SQL functions used in the models the session edited without the project's prescribed `target.type` Jinja guard. Only active in projects that already establish the guard convention.", + + async appliesTo(ctx: ValidatorContext): Promise { + const dbtRoot = await findDbtProjectRoot(ctx.workingDirectory) + if (!dbtRoot) return false + if (process.env[OPT_IN_ENV] === "1") return true + return await projectPrescribesGuards(dbtRoot) + }, + + async check(ctx: ValidatorContext): Promise { + const startedAt = Date.now() + const dbtRoot = await findDbtProjectRoot(ctx.workingDirectory) + if (!dbtRoot) { + return { ok: true, details: { skipped: "no dbt project", session_id: ctx.sessionID } } + } + const touched = await modelsModifiedSince(dbtRoot, ctx.sessionStartMs) + if (touched.length === 0) { + return { ok: true, details: { models_touched: 0, session_id: ctx.sessionID } } + } + + const findings: Finding[] = [] + for (const path of touched) { + let raw: string + try { + raw = await fs.readFile(path, "utf8") + } catch { + continue + } + const sql = stripGuardedBlocks(stripSqlComments(raw)) + const model = modelNameFromPath(path) + for (const fn of DIALECT_FUNCTIONS) { + fn.pattern.lastIndex = 0 + if (fn.pattern.test(sql)) { + findings.push({ model, function: fn.name, dialects: fn.dialects }) + } + } + } + + const details = { + models_touched: touched.length, + findings, + dbt_root: dbtRoot, + session_id: ctx.sessionID, + elapsed_ms: Date.now() - startedAt, + } + + if (findings.length === 0) return { ok: true, details } + + const byModel = new Map() + for (const f of findings) { + const list = byModel.get(f.model) ?? [] + list.push(f) + byModel.set(f.model, list) + } + const hintLines: string[] = [] + for (const [model, list] of byModel) { + hintLines.push(`Model \`${model}\`:`) + for (const f of list) hintLines.push(` • ${f.function} — ${f.dialects} only`) + } + hintLines.push("") + hintLines.push( + "This project guards warehouse-specific SQL on `target.type`. Either put the call behind that guard with a portable branch for the other targets:", + ) + hintLines.push( + " {% if target.type == 'snowflake' %} … {% else %} … {% endif %}", + ) + hintLines.push( + "or replace it with the portable equivalent (`case when` for conditionals, `coalesce` for null handling, `cast` for conversions), or move the branch into a dispatched macro. If the name is a project macro rather than the warehouse builtin, no change is needed.", + ) + + return { + ok: false, + reason: `${findings.length} unguarded warehouse-specific construct(s) in ${byModel.size} model(s) you edited: ${Array.from(byModel.keys()).join(", ")}.`, + fixHint: hintLines.join("\n"), + details, + } + }, +} +// altimate_change end diff --git a/packages/opencode/src/altimate/validators/index.ts b/packages/opencode/src/altimate/validators/index.ts index 2988671ed1..1cf19e0304 100644 --- a/packages/opencode/src/altimate/validators/index.ts +++ b/packages/opencode/src/altimate/validators/index.ts @@ -2,6 +2,7 @@ import { ValidatorRegistry } from "../../session/validators/registry" import { DbtBuildGreenValidator } from "./dbt-build-green" import { DbtDeliverableNamesValidator } from "./dbt-deliverable-names" +import { DbtDialectGuardValidator } from "./dbt-dialect-guard" import { DbtIncrementalConfigValidator } from "./dbt-incremental-config" import { DbtNothingBuiltValidator } from "./dbt-nothing-built" import { DbtSchemaVerifyValidator } from "./dbt-schema-verify" @@ -27,6 +28,7 @@ export function registerAltimateValidators(): void { ValidatorRegistry.register(DbtBuildGreenValidator) ValidatorRegistry.register(DbtDeliverableNamesValidator) ValidatorRegistry.register(DbtIncrementalConfigValidator) + ValidatorRegistry.register(DbtDialectGuardValidator) ValidatorRegistry.register(DbtSchemaVerifyValidator) ValidatorRegistry.register(DbtTestsPassValidator) } diff --git a/packages/opencode/test/altimate/validators/dbt-dialect-guard.test.ts b/packages/opencode/test/altimate/validators/dbt-dialect-guard.test.ts new file mode 100644 index 0000000000..1babd4f6d2 --- /dev/null +++ b/packages/opencode/test/altimate/validators/dbt-dialect-guard.test.ts @@ -0,0 +1,172 @@ +// altimate_change start — tests for the dialect-guard lint +import { describe, expect, test, afterEach } from "bun:test" +import { promises as fs } from "fs" +import { tmpdir } from "os" +import { join } from "path" +import { DbtDialectGuardValidator } from "../../../src/altimate/validators/dbt-dialect-guard" +import type { ValidatorContext } from "../../../src/session/validators/types" + +let dir = "" + +async function makeProject(): Promise { + dir = await fs.mkdtemp(join(tmpdir(), "dialect-guard-")) + await fs.writeFile( + join(dir, "dbt_project.yml"), + "name: t\nversion: '1.0'\nconfig-version: 2\nprofile: t\n", + ) + await fs.mkdir(join(dir, "models"), { recursive: true }) + return dir +} + +/** Establish the project convention: an existing guard under macros/. */ +async function addProjectGuardConvention(): Promise { + await fs.mkdir(join(dir, "macros"), { recursive: true }) + await fs.writeFile( + join(dir, "macros", "portable.sql"), + "{% macro ts() %}{% if target.type == 'duckdb' %}now(){% else %}current_timestamp{% endif %}{% endmacro %}", + ) +} + +async function writeModel(name: string, sql: string): Promise { + await fs.writeFile(join(dir, "models", `${name}.sql`), sql) +} + +const ctx = (overrides: Partial = {}): ValidatorContext => ({ + sessionID: "s", + workingDirectory: dir, + sessionStartMs: 0, + step: 1, + retryCount: 0, + ...overrides, +}) + +afterEach(async () => { + delete process.env.ALTIMATE_VALIDATORS_DIALECT_GUARD + if (dir) await fs.rm(dir, { recursive: true, force: true }) + dir = "" +}) + +describe("DbtDialectGuardValidator — appliesTo needs the project convention", () => { + test("does not apply outside a dbt project", async () => { + dir = await fs.mkdtemp(join(tmpdir(), "dialect-guard-nodbt-")) + expect(await DbtDialectGuardValidator.appliesTo(ctx())).toBe(false) + }) + + test("does not apply to a single-warehouse project with no guards", async () => { + await makeProject() + await writeModel("stg_orders", "select iff(x > 0, 1, 0) as flag from t") + expect(await DbtDialectGuardValidator.appliesTo(ctx())).toBe(false) + }) + + test("applies once the project guards on target.type in a macro", async () => { + await makeProject() + await addProjectGuardConvention() + expect(await DbtDialectGuardValidator.appliesTo(ctx())).toBe(true) + }) + + test("applies once the project guards on target.type in a model", async () => { + await makeProject() + await writeModel( + "portable", + "select {% if target.type == 'duckdb' %}1{% else %}2{% endif %} as x", + ) + expect(await DbtDialectGuardValidator.appliesTo(ctx())).toBe(true) + }) + + test("applies under the explicit opt-in", async () => { + await makeProject() + process.env.ALTIMATE_VALIDATORS_DIALECT_GUARD = "1" + expect(await DbtDialectGuardValidator.appliesTo(ctx())).toBe(true) + }) +}) + +describe("DbtDialectGuardValidator — check", () => { + test("flags an unguarded Snowflake function", async () => { + await makeProject() + await addProjectGuardConvention() + await writeModel("fct_orders", "select iff(amount > 0, 1, 0) as is_positive from {{ ref('src') }}") + const r = await DbtDialectGuardValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.reason).toContain("fct_orders") + expect(r.fixHint).toContain("iff()") + expect(r.fixHint).toContain("target.type") + }) + + test("flags an unguarded BigQuery function", async () => { + await makeProject() + await addProjectGuardConvention() + await writeModel("fct_orders", "select safe_divide(a, b) as ratio from {{ ref('src') }}") + expect((await DbtDialectGuardValidator.check(ctx())).ok).toBe(false) + }) + + test("flags an unguarded DuckDB function", async () => { + await makeProject() + await addProjectGuardConvention() + await writeModel("raw_load", "select * from read_csv_auto('x.csv')") + expect((await DbtDialectGuardValidator.check(ctx())).ok).toBe(false) + }) + + test("accepts the same function inside a target.type guard", async () => { + await makeProject() + await addProjectGuardConvention() + await writeModel( + "fct_orders", + [ + "select", + "{% if target.type == 'snowflake' %}", + " iff(amount > 0, 1, 0) as is_positive", + "{% else %}", + " case when amount > 0 then 1 else 0 end as is_positive", + "{% endif %}", + "from {{ ref('src') }}", + ].join("\n"), + ) + const r = await DbtDialectGuardValidator.check(ctx()) + expect(r.ok).toBe(true) + expect(r.details!["findings"]).toEqual([]) + }) + + test("accepts portable SQL", async () => { + await makeProject() + await addProjectGuardConvention() + await writeModel( + "fct_orders", + "select coalesce(a, 0) as a, case when b > 0 then 1 else 0 end as f from {{ ref('src') }}", + ) + expect((await DbtDialectGuardValidator.check(ctx())).ok).toBe(true) + }) + + test("ignores a dialect function that only appears in a comment", async () => { + await makeProject() + await addProjectGuardConvention() + await writeModel("fct_orders", "-- was iff(a, 1, 0)\nselect 1 as id") + expect((await DbtDialectGuardValidator.check(ctx())).ok).toBe(true) + }) + + test("does not fire on a same-named column reference", async () => { + await makeProject() + await addProjectGuardConvention() + await writeModel("fct_orders", "select iff as legacy_flag, getdate as d from {{ ref('src') }}") + expect((await DbtDialectGuardValidator.check(ctx())).ok).toBe(true) + }) + + test("passes when the session touched nothing", async () => { + await makeProject() + await addProjectGuardConvention() + await writeModel("fct_orders", "select iff(a, 1, 0) as f from t") + const r = await DbtDialectGuardValidator.check(ctx({ sessionStartMs: Date.now() + 60_000 })) + expect(r.ok).toBe(true) + expect(r.details!["models_touched"]).toBe(0) + }) + + test("aggregates findings across models", async () => { + await makeProject() + await addProjectGuardConvention() + await writeModel("a", "select zeroifnull(x) as x from t") + await writeModel("b", "select safe_cast(x as int64) as x from t") + const r = await DbtDialectGuardValidator.check(ctx()) + expect(r.ok).toBe(false) + expect((r.details!["findings"] as unknown[]).length).toBe(2) + }) +}) +// altimate_change end From a659dad1b3120facd233ffb8892ad4949ffcb66d Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 28 Aug 2026 18:30:23 -0700 Subject: [PATCH 06/21] docs: engine-split assessment for the two parse-level completion checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Answers whether the SQL engine already covers the two deterministic checks that cannot live in the fs+regex validator tier. - Unguarded division: already shipped and already wired. Lint rule `L032` (`division_by_column_no_guard`) is a real sqlparser expression-tree walk; guarded denominators are excluded structurally because `NULLIF`/`CASE` parse as non-identifier nodes. Reachable today via `Dispatcher.call("altimate_core.lint", …)` with an empty schema. No engine work; the only consumer cost is feeding it compiled model SQL, which sequences it behind the build-green gate. - Filter consistency: the engine has a close cousin at the wrong granularity. `review::grain::extract_source_filters` compares WHERE-clause filters ACROSS models (already consumed by `siblingConsistencyLane`) but never looks inside projection expressions, so sibling aggregates carrying asymmetric CASE predicates in one SELECT are invisible. Needs one new analysis pass — best expressed as a lint rule alongside `L032`, since that path is already plumbed end to end — plus a napi export and a dispatcher entry. Rule-sized, not architecture-sized; the cost concentrates in structural predicate normalisation. Includes the capability-to-consumer path (crate, npm package, version pin, lazy dispatcher registration) so the engine ticket can be scoped. Co-Authored-By: Claude Fable 5 --- .../deterministic-checks-engine-split.md | 193 ++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 docs/internal/deterministic-checks-engine-split.md diff --git a/docs/internal/deterministic-checks-engine-split.md b/docs/internal/deterministic-checks-engine-split.md new file mode 100644 index 0000000000..147ff14786 --- /dev/null +++ b/docs/internal/deterministic-checks-engine-split.md @@ -0,0 +1,193 @@ +# Deterministic completion checks: what belongs in the engine + +**Status:** assessment, input to a build decision +**Date:** 2026-08-28 +**Scope:** the two deterministic checks the plan assigns to the engine rather than to the +validator lane — unguarded division (parse-level) and filter consistency (semantic / +lineage-level). The cheap fs+regex tier lives in +`packages/opencode/src/altimate/validators/` and is out of scope here. + +Engine repo inspected read-only at `/Users/anandgupta/codebase/altimate-core-internal` +(Rust workspace, version `0.7.0`). + +--- + +## Bottom line + +| Check | Engine capability today | Work required | +|---|---|---| +| Unguarded division | **Already implemented and already wired.** Lint rule `L032` (`division_by_column_no_guard`) is a real sqlparser expression-tree walk, reachable from altimate-code right now via `Dispatcher.call("altimate_core.lint", …)`. | **None engine-side.** Consumer-side only: a validator that feeds it compiled model SQL. | +| Filter consistency | **Partial — the wrong granularity.** The engine ships `extract_source_filters`, a *cross-model* sibling-filter primitive already consumed by the review orchestrator. It reads WHERE clauses only, and cannot see predicates attached to sibling aggregates inside one SELECT. | **One new engine analysis** (~a rule-sized addition, not an architecture change) plus a napi export, a dispatcher entry and a consumer tool. | + +Neither check can move into the cheap validator tier: both need a parsed expression tree. +Regex cannot tell `a / b` from `a / nullif(b, 0)` once either side is a function call, a +CASE, or a nested expression, and it certainly cannot group aggregates by the column they +read. + +--- + +## How engine capability reaches altimate-code + +Relevant because it sets the cost of "add something engine-side". + +- Engine crates: `altimate-core` (analysis), `altimate-core-bindings-common` (shared binding + layer), `altimate-core-node` (napi-rs), plus `polyglot-sql` for multi-dialect parse and + transpile. +- Published as the npm package `@altimateai/altimate-core` (per-platform native addon). + altimate-code pins it exactly: `packages/opencode/package.json` → `"@altimateai/altimate-core": "0.7.0"`. +- Consumer binding: `packages/opencode/src/altimate/native/altimate-core.ts` registers ~34 + `altimate_core.*` handlers on the dispatcher. Registration is lazy — the napi binary loads + on the first `Dispatcher.call()` (`packages/opencode/src/altimate/native/index.ts`), so a + validator importing `Dispatcher` costs nothing until it actually calls. +- Adding a capability therefore means: engine PR → workspace version bump → publish → + bump the consumer pin → one `register(...)` block in `altimate-core.ts` → optionally a + `Tool.define` file under `src/altimate/tools/`. Four repos-worth of steps, one release + boundary. + +--- + +## Check A — unguarded division + +**What the check is.** Flag a division whose denominator is not wrapped in a `NULLIF` or a +`CASE` guard. The failure it catches is a division-by-zero or silent-NULL result in a ratio +column; it recurs in evaluation traces as a wrong-value defect that builds green and passes +schema checks. + +**Verdict: the engine already does this. Nothing to build engine-side.** + +- Rule: `crates/altimate-core/src/linter/rules/division_by_column_no_guard.rs`, lint code + `L032`, name `division_by_column_no_guard`. +- Implementation is a genuine AST walk, not a text heuristic: it parses with sqlparser and + walks `Expr::BinaryOp { op: Divide }` nodes through CTE bodies, set-op branches, function + arguments and nested expressions (`find_division_by_column_in_set_expr`, and the shared + walkers in `crates/altimate-core/src/linter/rules/mod.rs`). +- The guard semantics fall out of the tree shape: it fires only when the denominator is a + bare `Expr::Identifier` / `Expr::CompoundIdentifier`. A denominator wrapped in + `NULLIF(...)` parses as `Expr::Function` and one wrapped in `CASE` as `Expr::Case`, so + guarded divisions are excluded structurally rather than by pattern-matching the guard. +- `fn check(&self, sql: &str, _schema: &SchemaDefinition)` ignores the schema, so the rule + needs no table/column resolution to be accurate. + +**How altimate-code would call it.** + +```ts +import { Dispatcher } from "../native" + +const result = await Dispatcher.call("altimate_core.lint", { sql, schema_path: "" }) +const findings = (result.data?.findings ?? []).filter((f: any) => f.rule === "L032") +``` + +`altimate_core.lint` is registered at `packages/opencode/src/altimate/native/altimate-core.ts` +(handler 2), calling `core.lint(sql, schema)` in `crates/altimate-core-node/src/safety.rs`. +`schemaOrEmpty()` in the same file means an empty schema is a supported argument. The +composite `altimate_core.check` handler folds the same lint output into `data.lint.findings` +and is already exposed to the agent through +`packages/opencode/src/altimate/tools/altimate-core-check.ts`, which shows the finding shape +(`f.rule`) end to end. + +**The one real consumer-side problem: Jinja.** The engine parses SQL, and a dbt model source +is not SQL — `{{ ref() }}`, `{% if %}` and `{{ config() }}` will not parse. A validator must +feed it the **compiled** SQL from `/compiled//models/**.sql`, which exists +only after a successful compile or build. That makes the division lint naturally sequenced +*after* the build-green gate: no fresh compiled artifact, nothing to lint, skip. The +existing `resolveDbtTargetPath()` in +`packages/opencode/src/altimate/validators/validator-utils.ts` already resolves the artifact +directory including a custom `target-path`. + +**Recommendation.** Build it as a validator in the existing lane, consuming +`altimate_core.lint` and filtering to `L032`, scoped to session-touched models and their +compiled counterparts. No engine work, no version bump. The cost is the compiled-SQL +plumbing, not the analysis. + +--- + +## Check B — filter consistency + +**What the check is.** Detect an exclusion predicate applied on one aggregate path but +omitted on a sibling aggregate over the same source — e.g. two `SUM(CASE WHEN … END)` +measures in one SELECT where one carries an extra exclusion the other lacks. This is the +most-evidenced wrong-logic family: the model builds, the shape is right, the numbers are +quietly inconsistent between columns. + +**Verdict: the engine has the building blocks and a close cousin, but not this check.** + +What exists: + +- `crates/altimate-core/src/review/grain.rs` → `extract_source_filters(sql) -> BTreeMap>`. + Returns, per upstream table, the filter columns applied to it. Exported over napi in + `crates/altimate-core-node/src/review.rs` and registered as the dispatcher key + `altimate_core.source_filters` (`altimate-core.ts`, "Per-upstream WHERE-filter columns, + for cross-model sibling filter-consistency"). +- It is already consumed: `siblingConsistencyLane` in + `packages/opencode/src/altimate/review/orchestrate.ts` compares those filter sets **across + different models reading the same upstream** in a diff, and flags a model missing a filter + its siblings apply. The doc comment on the Rust function cites the real incident that + motivated it (one of three sibling loaders missing a NULL filter). +- `crates/altimate-core/src/filter_analysis/` analyses WHERE-clause quality within a single + query (contradictory, redundant, missing-partition predicates) via logical plans. +- `crates/altimate-core/src/lineage/complete.rs` carries, per output column, a + `lens_code: Vec` — the SQL text of each transformation step — alongside + `transform_type` / `lineage_type` labels. Exposed via `column_lineage` in + `crates/altimate-core-node/src/lineage.rs` (dispatcher key `altimate_core.column_lineage`). + +Why none of it is the check: + +- `extract_source_filters` walks `sel.selection` — the WHERE clause — and attributes filter + *columns* to upstream tables. It never looks inside projection expressions, so a + `CASE WHEN status = 'x' AND NOT is_test THEN amount END` inside a `SUM()` is invisible to + it. Its comparison unit is a model, not a column. +- `filter_analysis` reasons about one predicate set at a time; there is no cross-aggregate + comparison anywhere in it. +- The lineage `lens_code` does carry the aggregate's expression *text* per target column, + which is tantalising but not sufficient: it is unparsed text with a coarse transform + label, so any comparison built on it would be string diffing — exactly the fuzzy matching + a deterministic gate must not do. Two logically identical predicates written differently + would read as inconsistent, and a genuinely asymmetric one written similarly could slip + through. + +**What would need to be added engine-side.** One new analysis pass, following the shape of +the existing rules: + +1. Walk the SELECT projection, collecting aggregate calls (`SUM`, `COUNT`, `AVG`, …) whose + argument is a `CASE` expression. The helpers already exist next door in + `crates/altimate-core/src/linter/rules/mod.rs` and `review/grain.rs` (aggregate + detection, bare/qualified column extraction). +2. For each, extract the CASE `WHEN` condition set as parsed predicates, plus the base + column being aggregated. +3. Group the aggregates by base column (and by upstream relation, reusing the attribution + `walk_query_filters` already performs). +4. Diff the predicate sets within a group and emit a finding for an asymmetric exclusion, + normalising predicates structurally rather than textually so that reordered or + differently-spelled equivalents do not produce noise. + +Then: a napi export in `crates/altimate-core-node/src/review.rs` (or a new lint code in +`safety.rs::lint` if it is expressed as a rule — a rule is the cheaper path, since `lint` +is already plumbed all the way through to the agent and to `altimate_core.check`), a +dispatcher entry in `altimate-core.ts`, and a validator consuming it. + +**Why this cannot live in the validator lane.** Step 1 needs the projection's expression +tree; step 2 needs parsed boolean predicates; step 3 needs column attribution through CTEs +and aliases; step 4 needs structural predicate comparison. Every one of those is SQL +analysis. A regex approximation would fire on formatting differences and miss the real +asymmetries, which is the worst outcome for a gate that blocks completion — a lane that +cries wolf gets its retry budget burned and then disabled. + +**Sizing.** Rule-sized, not architecture-sized: it reuses existing sqlparser walkers, has a +direct precedent in `extract_source_filters`, and needs no change to lineage or to the +binding architecture. The dominant cost is predicate normalisation (step 4), which is where +the false-positive risk concentrates and where the test corpus has to be built. + +--- + +## Decision inputs + +1. **Unguarded division is free.** It is a consumer-side validator over + `altimate_core.lint` + compiled SQL. Do it in the lane; no engine ticket, no version bump. +2. **Filter consistency is the only item here that needs an engine ticket**, and it is worth + scoping as a lint rule (code alongside `L032`) rather than a bespoke review API, because + the lint path is already wired end to end — engine rule → `lint` → dispatcher → + `altimate_core.check` → agent and validators. That collapses the wiring work to the rule + itself. +3. **Sequencing.** Both checks depend on compiled SQL, so both sit behind the build-green + gate in the completion lane. Neither is worth building before that gate demonstrably + fires on real sessions. From b6ecfaae14a9585566824cbb6425a77db4da3c16 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 28 Aug 2026 18:32:03 -0700 Subject: [PATCH 07/21] fix(validators): scope build-green failure counts correctly `failed_out_of_scope` counted every failing node when the session edited nothing, because the in-scope set was empty and out-of-scope was computed independently of the "no edits means the whole run is ours" branch. Telemetry therefore double-counted the same failures as both in and out of scope. Compute both from one partition of the failing nodes. Co-Authored-By: Claude Fable 5 --- .../altimate/validators/dbt-build-green.ts | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/opencode/src/altimate/validators/dbt-build-green.ts b/packages/opencode/src/altimate/validators/dbt-build-green.ts index 84281da8da..5d1a2278cf 100644 --- a/packages/opencode/src/altimate/validators/dbt-build-green.ts +++ b/packages/opencode/src/altimate/validators/dbt-build-green.ts @@ -145,17 +145,14 @@ export const DbtBuildGreenValidator: Validator = { }) } - const inScope = new Set(states.map((s) => s.name)) - const failedInScope = states.filter((s) => s.status !== null && isFailedRunStatus(s.status)) // With no edits of our own, the fresh artifact IS this session's build, so - // every failing node in it is in scope. - const failedWholeRun = - touchedPaths.length === 0 - ? fresh.results.filter((r) => isFailedRunStatus(r.status)) - : fresh.results.filter((r) => isFailedRunStatus(r.status) && inScope.has(r.name)) - const failedOutOfScope = fresh.results.filter( - (r) => isFailedRunStatus(r.status) && !inScope.has(r.name), - ).length + // every failing node in it is in scope. Otherwise scope is what we edited, + // and failures elsewhere are recorded but never block. + const inScope = new Set(states.map((s) => s.name)) + const allFailed = fresh.results.filter((r) => isFailedRunStatus(r.status)) + const failedInScope = + touchedPaths.length === 0 ? allFailed : allFailed.filter((r) => inScope.has(r.name)) + const failedOutOfScope = allFailed.length - failedInScope.length // Coverage is only assertable when the artifact actually recorded models. const coverageAssertable = modelNodes.size > 0 @@ -169,20 +166,20 @@ export const DbtBuildGreenValidator: Validator = { verdict: "fresh-build", coverage_assertable: coverageAssertable, model_nodes_in_artifact: modelNodes.size, - failed_in_scope: failedWholeRun.map((r) => r.name), + failed_in_scope: failedInScope.map((r) => r.name), failed_out_of_scope: failedOutOfScope, not_built: notBuilt.map((s) => s.name), stale_build: staleBuild.map((s) => s.name), } - if (failedWholeRun.length === 0 && notBuilt.length === 0 && staleBuild.length === 0) { + if (failedInScope.length === 0 && notBuilt.length === 0 && staleBuild.length === 0) { return { ok: true, details } } const reasonParts: string[] = [] - if (failedWholeRun.length > 0) { + if (failedInScope.length > 0) { reasonParts.push( - `${failedWholeRun.length} node(s) failed in the last build: ${failedWholeRun.map((r) => `${r.name} (${r.status})`).join(", ")}`, + `${failedInScope.length} node(s) failed in the last build: ${failedInScope.map((r) => `${r.name} (${r.status})`).join(", ")}`, ) } if (notBuilt.length > 0) { @@ -197,12 +194,12 @@ export const DbtBuildGreenValidator: Validator = { } const hintLines: string[] = [] - for (const failure of failedWholeRun.slice(0, 10)) { + for (const failure of failedInScope.slice(0, 10)) { const msg = (failure.message ?? "").split("\n")[0]?.slice(0, 200) hintLines.push(` • ${failure.name} — ${failure.status}${msg ? `: ${msg}` : ""}`) } - if (failedWholeRun.length > 10) { - hintLines.push(` • …and ${failedWholeRun.length - 10} more`) + if (failedInScope.length > 10) { + hintLines.push(` • …and ${failedInScope.length - 10} more`) } hintLines.push( "Rebuild the models you changed with `dbt build` and make the run finish clean before declaring done. Fix the model SQL rather than removing the model, disabling the test, or narrowing the selector.", From 39781d8bb7fdc3ddaf364cbe86325fdc5cf8936e Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 28 Aug 2026 18:34:54 -0700 Subject: [PATCH 08/21] test(validators): pin the lane's registration list A validator that is written but never registered is invisible, and nothing else in the suite would notice. Pins the registered names and their order, asserts idempotence and the framework contract (appliesTo/check/description), and checks that no validator fires against a directory that is not a dbt project. Co-Authored-By: Claude Fable 5 --- .../altimate/validators/registration.test.ts | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 packages/opencode/test/altimate/validators/registration.test.ts diff --git a/packages/opencode/test/altimate/validators/registration.test.ts b/packages/opencode/test/altimate/validators/registration.test.ts new file mode 100644 index 0000000000..c089aa85b5 --- /dev/null +++ b/packages/opencode/test/altimate/validators/registration.test.ts @@ -0,0 +1,65 @@ +// altimate_change start — registration contract for the altimate validator lane +import { describe, expect, test, afterAll } from "bun:test" +import { ValidatorRegistry } from "../../../src/session/validators/registry" +import { registerAltimateValidators } from "../../../src/altimate/validators" + +/** + * A validator that is written but never registered is invisible, and nothing + * else in the test suite would notice. This file pins the registration list. + */ + +const EXPECTED = [ + "dbt-nothing-built", + "dbt-build-green", + "dbt-deliverable-names", + "dbt-incremental-config", + "dbt-dialect-guard", + "dbt-schema-verify", + "dbt-tests-pass", +] + +const snapshot = ValidatorRegistry.list().slice() + +afterAll(() => { + ValidatorRegistry.clear() + for (const v of snapshot) ValidatorRegistry.register(v) +}) + +describe("registerAltimateValidators", () => { + test("registers every validator in the lane, in dependency order", () => { + ValidatorRegistry.clear() + registerAltimateValidators() + expect(ValidatorRegistry.list().map((v) => v.name)).toEqual(EXPECTED) + }) + + test("is idempotent", () => { + ValidatorRegistry.clear() + registerAltimateValidators() + registerAltimateValidators() + expect(ValidatorRegistry.list().length).toBe(EXPECTED.length) + }) + + test("every validator satisfies the framework contract", () => { + ValidatorRegistry.clear() + registerAltimateValidators() + for (const v of ValidatorRegistry.list()) { + expect(typeof v.appliesTo).toBe("function") + expect(typeof v.check).toBe("function") + expect(v.description.length).toBeGreaterThan(20) + } + }) + + test("no validator applies to a directory that is not a dbt project", async () => { + ValidatorRegistry.clear() + registerAltimateValidators() + const results = await ValidatorRegistry.runAll({ + sessionID: "s", + workingDirectory: "/nonexistent-path-for-validator-contract-test", + sessionStartMs: 0, + step: 1, + retryCount: 0, + }) + expect(results.filter((r) => !r.result.ok)).toEqual([]) + }) +}) +// altimate_change end From 14747ac6c98121d17e3b4a5b43fe9d6b91f4dbbb Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 28 Aug 2026 19:47:56 -0700 Subject: [PATCH 09/21] docs: placement assessment for the five shipped completion-gate validators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Appends a section to the engine-split doc answering whether dbt-nothing-built, dbt-build-green, dbt-deliverable-names, dbt-incremental-config, and dbt-dialect-guard belong in altimate-core-internal. Key finding: dbt-incremental-config's upsert/guard checks duplicate, with a weaker regex matcher, the engine's already-shipped and already-consumed dbt_config_lint (DBT001/DBT002) — should be rewired onto the dispatcher call rather than reimplemented. The other three validators are pure filesystem/artifact/task-doc checks with no SQL surface and no engine reuse value. dbt-dialect-guard's guard-detection has no engine home (needs un-rendered Jinja branches the compiled-SQL contract can't see); only its curated function list is worth reconciling with engine's L033 rule. Co-Authored-By: Claude Opus 5 --- .../deterministic-checks-engine-split.md | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/docs/internal/deterministic-checks-engine-split.md b/docs/internal/deterministic-checks-engine-split.md index 147ff14786..cdfc093c61 100644 --- a/docs/internal/deterministic-checks-engine-split.md +++ b/docs/internal/deterministic-checks-engine-split.md @@ -191,3 +191,55 @@ the false-positive risk concentrates and where the test corpus has to be built. 3. **Sequencing.** Both checks depend on compiled SQL, so both sit behind the build-green gate in the completion lane. Neither is worth building before that gate demonstrably fires on real sessions. + +--- + +## Placement of the shipped completion-gate validators + +**Status:** assessment, addendum to the above +**Date:** 2026-08-28 +**Scope:** the five completion-gate validators actually shipped in +`packages/opencode/src/altimate/validators/`: `dbt-nothing-built`, `dbt-build-green`, +`dbt-deliverable-names`, `dbt-incremental-config`, `dbt-dialect-guard`. Question: does any +of this — or its logic — belong in `altimate-core-internal` instead of `altimate-code`, so +other engine consumers (the dbt PR reviewer, CI tooling, other products) can reuse it? + +### Per-validator table + +| Validator | What it analyzes | Reuse value for other engine consumers | Engine-feasible (does the engine see the inputs)? | Recommendation | +|---|---|---|---|---| +| `dbt-nothing-built` | Filesystem: any authored file under `models/seeds/snapshots/data/analyses/macros/tests` since session start, `run_results.json` freshness. Plus a markdown/regex scan of a task-instruction file for literal required deliverables. | None. The check *is* "did this session's filesystem change since a wall-clock timestamp" — meaningless outside a live agent session. | No. The engine's API is SQL-in/findings-out; it has no concept of session start time, a live project directory to `stat`, or a task-instruction document. | **Stays in altimate-code.** Not a SQL question at any point. | +| `dbt-build-green` | Filesystem: `/run_results.json` parsed, cross-referenced against edited-model mtimes for coverage/staleness/failure status. | None. Same category as above — needs a live build artifact plus session-scoped mtime comparisons. | No. Same as above; also no dbt subprocess invocation happens in-engine. | **Stays in altimate-code.** | +| `dbt-deliverable-names` | Filesystem inventory (`models/seeds/snapshots/data/analyses`) + `manifest.json` node names/aliases, diffed against literal deliverable names extracted from a task document via markdown/regex parsing. | None directly — this is a naming-contract check against a task doc, not a SQL property. | No. Requires a live project directory and a task-instruction file; the engine never sees either. | **Stays in altimate-code.** | +| `dbt-incremental-config` | Two of its three findings are dbt **config** semantics — `incremental_strategy`/`unique_key` presence, `is_incremental()` guard presence — read via regex over `{{ config(...) }}` args and grep over the model body. The third is a regex scan for non-deterministic functions (`current_timestamp`, `random()`, …) inside the `is_incremental()` predicate block. | **High, and already realized elsewhere.** `crates/altimate-core/src/review/dbt_config.rs::dbt_config_lint` implements the *same two* checks — `DBT001` (`incremental_no_guard`) and `DBT002` (`incremental_no_unique_key`) — as a real minijinja-based structural config parser, not regex. It is already wired dispatcher-side as `altimate_core.dbt_config_lint` and is **already consumed** by the dbt-PR-reviewer (`packages/opencode/src/altimate/review/runner.ts:338`). | Yes for findings 1 & 2 — the engine already does this, on raw (uncompiled) model SQL, no compiled-artifact dependency. Finding 3 (non-determinism *specifically inside* the `is_incremental()` predicate) has no engine equivalent yet; the closest existing rule, `L049 clock_in_filter`, catches clock functions in any WHERE/JOIN/HAVING predicate but doesn't scope to the incremental guard block specifically. | **Rewire, don't reimplement.** Findings 1 & 2 should call `altimate_core.dbt_config_lint` and filter to `DBT001`/`DBT002` instead of carrying a parallel regex implementation — the validator's own comment tier ("grep-level … config declared in `dbt_project.yml` is intentionally not resolved") describes exactly the ceiling the engine's minijinja parser already clears. The idempotency-gating policy (only escalate the missing-guard finding to a blocker when the task doc says "idempotent") is genuinely consumer-side — it depends on a task-instruction file the engine doesn't see — and stays in the validator as a thin policy layer over the engine's finding. **Worth an engine follow-up ticket:** add a `DBT007`-style rule to the same `dbt_config.rs` file for "non-deterministic function inside an `is_incremental()` predicate" — the parsing/masking infrastructure (`mask_comments_and_strings`, predicate-block extraction) already exists in that file, so it's a small addition, not new architecture. | +| `dbt-dialect-guard` | Raw (uncompiled) model text: (a) does the project already establish a `target.type`-guard convention (fs scan for the string across `models/`+`macros/`); (b) a curated 19-entry list of warehouse-specific function names, regex-matched; (c) whether each match sits inside a `{% if …target.type… %}…{% endif %}` block, detected by blanking out guarded regions before matching. | Partial, and one-directional. The engine's `L033 non_portable_function` rule is a genuine sqlparser AST walk with a much larger curated function set (the excerpt alone showed 50+ names vs. the validator's 19) — the validator is duplicating a *subset* of engine data with a worse matcher. But L033 cannot do what this validator does: it requires parseable SQL, and compiling a dbt model resolves away whichever `{% if target.type %}` branch wasn't taken — the information "was this guarded" is gone by the time SQL reaches the engine's parser. | No, for the guard-detection half — that is a category error: the engine's SQL-in/findings-out contract has nothing to say about un-rendered Jinja branches, because by the time it sees valid SQL the branch has already been chosen. Yes, in principle, for the *function-name curation* half, which is pure data the engine already owns more completely. | **Hybrid, but the "hybrid" is data-sharing, not logic-splitting.** The guard-detection orchestration must stay in altimate-code (it needs live, un-rendered Jinja source — a live-project concern by definition). The curated dialect-function list should stop being hand-maintained twice: either export `non_portable_function_set()` (or a subset annotated with source dialect) via a small napi accessor so the validator sources its match list from the engine, or accept the current duplication and add a periodic reconciliation check. This is a maintenance/consistency follow-up, not an engine ticket for new analysis logic. | + +### Overall recommendation — does this need to go into the engine? + +**No, not as a wholesale move.** Three of the five validators (`dbt-nothing-built`, +`dbt-build-green`, `dbt-deliverable-names`) never touch SQL semantics at all — they reason +about filesystem state, build artifacts (`run_results.json`, `manifest.json`), and a task +document's prose. None of that is representable in the engine's SQL-in/findings-out +dispatcher contract, and none of it has any value to another engine consumer (the dbt PR +reviewer doesn't care whether *this* session's clock wrote a file). Pushing them +engine-side would be the same category error the prior assessment already named for +division-guard and filter-consistency, just in the other direction: these validators need a +live project directory and session-scoped timestamps, and the engine's API surface is +SQL text in, findings out. There's nothing to move. + +The one real finding here is `dbt-incremental-config`, and it isn't "should this move to +the engine" — **it already lives there.** `dbt_config_lint` (`DBT001`/`DBT002`) is shipped, +wired through the dispatcher, and already consumed by the dbt-PR-reviewer's `runner.ts`. +The validator built a second, weaker (regex-only) implementation of the same two checks +instead of calling the existing one. The fix is a rewire of the validator's detection +layer onto `altimate_core.dbt_config_lint`, keeping only the task-doc-driven idempotency +policy consumer-side (exactly the hybrid pattern the division-guard case established: +engine finds it, consumer decides when it's blocking). One small engine follow-up is worth +filing — a `DBT007` rule for non-determinism scoped to the `is_incremental()` predicate, +reusing infrastructure already in `dbt_config.rs` — but it's rule-sized, not a new +capability. + +`dbt-dialect-guard` is the only genuinely mixed case, and even there the "engine work" is +data reconciliation (share `L033`'s function-name set) rather than new analysis: the guard +-detection logic itself has no engine home because it depends on un-rendered Jinja +branches, which the engine's compiled-SQL contract structurally cannot see. From 6626c46b8d9c1aeef1dc806307e266fedb186bf9 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sat, 29 Aug 2026 11:05:02 -0700 Subject: [PATCH 10/21] docs: end-to-end evidence for the five completion-gate validators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measures the five validators on real dbt projects instead of unit-test fixtures: the ten `harness-loop` dbt workspaces (copied, never mutated) and a fresh clone of the public `dbt-labs/jaffle-shop-classic` on DuckDB. Findings: - **False positives: 5 across 38 known-good states.** Zero across 27 naturalistic end-states; five reproduce deterministically on states that are ordinary dbt practice — `dbt-build-green` blocks on an edited ephemeral model, on a deliberately disabled model, and on any file touch more than 1 s after a green build; `dbt-dialect-guard` reports a guarded call as unguarded when the `target.type` block contains a nested `{% if %}`, and matches a dialect function name inside a string literal. `dbt-nothing-built`, `dbt-deliverable-names` and `dbt-incremental-config` produced no false positives. - **True positives: 8 of 11 known-bad states fired.** The three silent ones trace to `REQUIREMENT_VERB_RE` not covering `add`, which is what decides whether the contract-driven validators activate at all. - **Negative control passes.** A live session in a non-dbt TypeScript repo with the lane and `ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS` both forced on registered all seven validators, dispatched once, and executed zero of them. - **Cost:** ~2-10 ms per dispatch on a small project; ~1-3.5 s on a 2005-model project, paid even when nothing was touched, because each validator walks the tree independently. - **A/B conversion: not measured.** N = 1 complete pair (3 sessions of a planned 40); the batch was stopped for machine capacity, not because a result was in. The doc states this explicitly and specifies what a proper VM-based three-arm run would need. Verdict recorded in the doc: shadow only, not enable-by-default and not revert, pending the five false-positive fixes and a powered A/B. Documentation only; no source changes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VqnuBDGkh1ZT65Ti7e6DHZ --- docs/internal/validator-e2e-evidence.md | 561 ++++++++++++++++++++++++ 1 file changed, 561 insertions(+) create mode 100644 docs/internal/validator-e2e-evidence.md diff --git a/docs/internal/validator-e2e-evidence.md b/docs/internal/validator-e2e-evidence.md new file mode 100644 index 0000000000..ba96ae78bd --- /dev/null +++ b/docs/internal/validator-e2e-evidence.md @@ -0,0 +1,561 @@ +# Completion-gate validators: end-to-end evidence + +**Status:** measurement report, input to an enable/shadow/revert decision +**Date:** 2026-08-29 +**Scope:** the five validators added in PR #1175 — +`dbt-nothing-built`, `dbt-build-green`, `dbt-deliverable-names`, +`dbt-incremental-config`, `dbt-dialect-guard` — measured on real dbt projects +rather than on unit-test fixtures. + +> **Sample-size disclosure, up front — read this before the A/B section.** +> The planned A/B was 10 tasks × 2 arms × 2 rollouts = 40 live sessions. +> **Three complete sessions were achieved: N = 1 paired task (arm A + arm B) +> plus one unpaired arm-A run.** The batch was terminated for machine capacity: +> these sessions ran on a single laptop that was concurrently hosting another +> workstream's agent fleet, the machine had already crashed twice under load, +> and local session execution was stopped outright at a load average above 20. +> **No conversion claim can be made from N = 1.** The A/B numbers below are +> reported for completeness and transparency, not as evidence of effect in +> either direction. +> +> The false-positive, true-positive, negative-control and cost sections do +> **not** carry this limitation. Those are deterministic probes — the validator +> code paths run directly against real project states with no model in the loop +> — and their N (38 known-good states, 11 known-bad states) is what the safety +> conclusions rest on. The negative control is a completed live session. +> +> What a properly powered A/B would need is specified in +> [What a real A/B would require](#what-a-real-ab-would-require). + +--- + +## Bottom line + +| Question | Answer | +|---|---| +| Do the five fire on healthy, complete dbt projects? | **Not in ordinary end-states** — 0 firings across 27 naturalistic known-good states. | +| Are there false positives at all? | **Yes, five reproducible ones**, all in `dbt-build-green` (3) and `dbt-dialect-guard` (2), each triggered by an ordinary dbt practice rather than by a defect. | +| Do they fire on genuinely-unfinished work? | **Yes** — every constructed defect state was caught, with two recall gaps in the task-document parser. | +| Do they execute in a non-dbt repo? | **No.** Zero validators executed; confirmed in a live session with the lane and the artifact opt-in both forced on. | +| Runtime cost? | ~2–10 ms per dispatch on a small project; ~1–3.5 s on a 2 000-model project, because each validator re-walks the tree independently. | +| Does enforcement convert failures into passes? | **Unknown — not measured.** N = 1 paired task; the run needed no retry, so there was nothing to convert. | +| Verdict | **Shadow only.** Two reasons, independently sufficient: five reproducible false positives, and no conversion evidence at all. Details in [Verdict](#verdict). | + +--- + +## Methodology + +### Code under test + +`feat/deterministic-validators` at `14747ac6c9`, worktree `/tmp/validators-build`. +Validators at `packages/opencode/src/altimate/validators/`; dispatch hook at +`packages/opencode/src/session/prompt.ts:1360`. + +The lane registers **seven** validators, not five: the two pre-existing members +(`dbt-schema-verify`, `dbt-tests-pass`) are registered alongside this PR's five. +To attribute results to this PR rather than to the lane as a whole, an +**experiment instrument** was added to `validators/index.ts` for the duration of +the measurement: an `ALTIMATE_VALIDATORS_ONLY` env allowlist filtering which +validators get registered (unset ⇒ register everything, i.e. shipped behaviour +unchanged). **That instrument is reverted in the commit that carries this +document** — it exists only so the numbers below describe the five. + +### Model + +altimate-code exposes a ChatGPT-subscription (Codex) provider +(`packages/opencode/src/plugin/codex.ts`, provider id `openai`, OAuth), but +**this environment has no credential for it** — `auth list` shows Anthropic +(oauth), Azure, Google, Vertex, Vertex (Anthropic), and the plugin's OAuth flow +needs an interactive browser login. Per the standing rule, no Vertex/GCP +Anthropic path was used. + +The three completed sessions therefore ran against a **self-hosted internal +staging endpoint**, registered as a custom OpenAI-compatible provider: + +``` +provider custom (npm: @ai-sdk/openai-compatible), internal staging endpoint +context 65536, output limit 8192 +sampling temperature 0.2, seed 1001 (pinned per rollout) +agent builder, --max-turns 40, --yolo, --format json +``` + +The model is a mid-capability coding model, not a frontier one. That matters +for reading the A/B: a stronger model would need the gates less often, a weaker +one more, so the fire rates here do not transfer directly to production traffic. +It does **not** affect the false-positive, true-positive, cost or +negative-control results, none of which involve a model. + +### Projects + +1. **An internal dbt task corpus** (10 tasks, all 10 used for the offline + probes, 2 reached in the live A/B) — real dbt projects (dbt-core + 1.8.7 + dbt-duckdb 1.8.3), each with a task prompt and a `verify.sh` grader + that rebuilds from scratch and diffs against a golden expectation file. + Every workspace was **copied** to a scratch directory before use; the + pristine corpus was never written to, and `verify.sh` was always invoked + with an explicit workspace argument. +2. **`dbt-labs/jaffle-shop-classic`** (public, Apache-2.0), cloned fresh and + pointed at a local DuckDB profile. Used for the false-positive work because + it is a complete, correct, third-party project with no injected defects. + +### Instruments + +* `validator-probe.ts` (repo root, **not committed**) — imports the registry + and runs `appliesTo()` + `check()` against an arbitrary directory with an + explicit `sessionStartMs`, emitting JSON. This is how the false-positive + sweep is executed: it exercises the exact validator code paths the session + hook calls, without a model in the loop, so results are deterministic and + repeatable. +* Live sessions via `bun run packages/opencode/src/index.ts run …` with + `ALTIMATE_VALIDATORS_DEBUG=1`, which mirrors every `validator_hook_reached` + and `dispatch_result` event to stderr. +* `altimate-dbt` was rebuilt from `packages/dbt-tools` and put first on `PATH`; + the globally-installed copy is stale and does not implement `schema-verify`. + +--- + +## Measurement 1 — false-positive rate (safety) + +A false positive here means: **a validator returns `ok:false` on a state a +competent engineer would call finished.** In enforce mode that state costs the +session a synthetic retry turn for nothing. + +### Set A — naturalistic known-good states (n = 27) + +For each of the seven `real-*` tasks whose pristine workspace builds green +(01, 02, 04, 05, 08, 09, 10), three end-states were probed: + +* **kg1a** — every project file written during the session, then `dbt build` + green ("the agent authored this and built it"). +* **kg1b** — project pre-existing, session ran only the build. +* **kg4** — read-only session: nothing written, nothing built. + +Plus six jaffle_shop states: whole project authored + built green; build-only; +read-only; a new correct model added and built green; the same with an explicit +`TASK.md` naming the deliverable (so the contract-driven validators activate); +and a correctly-configured incremental model built green. + +**Result: 0 firings from any of the five, across all 27 states.** + +The three `real-*` tasks whose pristine workspace does *not* build green +(03, 06, 07) were excluded from the known-good set — they are genuinely +unfinished, and firings there are true positives (recorded below). + +### Set B — constructed known-good states, each an ordinary dbt practice (n = 11) + +| # | State | Fired? | +|---|---|---| +| B1 | dialect-specific function inside a `{% if target.type … %}` guard | — | +| B2 | same, but the guard block contains a **nested `{% if %}`** | **FP — `dbt-dialect-guard`** | +| B3 | dialect function name appears only in a `--` SQL comment | — | +| B4 | dialect function name appears only inside a **string literal** | **FP — `dbt-dialect-guard`** | +| B5 | incremental config declared in `dbt_project.yml`, not in `config()` | — | +| B6 | last dbt command of the session was `dbt test` | — | +| B7 | green build, then the model file is **touched 3 s later** (reformat/comment) | **FP — `dbt-build-green`** | +| B8 | session edits an **ephemeral** model, builds green | **FP — `dbt-build-green`** | +| B9 | session **disables** a model on purpose (`enabled=false`), builds green | **FP — `dbt-build-green`** | +| B10 | session builds only the changed model with `--select` | — | +| B11 | session's build was `dbt run` (no tests) | — | + +### False-positive table + +| Validator | FPs | Known-good states where it could fire | Mechanism | +|---|---|---|---| +| `dbt-nothing-built` | **0** | 4 | — | +| `dbt-build-green` | **3** | 38 | see FP-1..3 | +| `dbt-deliverable-names` | **0** | 4 | — | +| `dbt-incremental-config` | **0** | 38 | — | +| `dbt-dialect-guard` | **2** | 7 | see FP-4..5 | + +Total: **5 false positives across 38 known-good states.** All five are +deterministic and reproduce on every run. + +#### FP-1 — `dbt-build-green`: any post-build write to a model file blocks + +State B7. The session writes a model, runs `dbt build` green, then appends a +trailing newline (a formatter, a comment, a tidy-up) three seconds later. + +``` +The build is not green: 1 model(s) were edited after the last build: extra_green2. + "stale_build": ["extra_green2"], "failed_in_scope": [], "not_built": [] +``` + +`BUILD_FRESHNESS_TOLERANCE_MS` is 1 000 ms. Any edit after that window — even a +whitespace-only one that cannot change compiled SQL — flips the model into +`stale_build` and blocks the session. "Build, then tidy, then summarise" is a +common agent trajectory, so this is not a corner case. + +#### FP-2 — `dbt-build-green`: editing an **ephemeral** model always blocks + +State B8. dbt does not emit a `run_results` node for an ephemeral model, so the +coverage assertion ("is every model I edited present in the artifact?") can +never be satisfied for one. + +``` +The build is not green: 1 model(s) you edited were never built: eph_helper. + "not_built": ["eph_helper"], "model_nodes_in_artifact": 6 +``` + +The build was green; the ephemeral model was compiled into its consumer, which +built and passed. There is no defect to fix, and no action the agent can take +that clears the gate short of changing the materialization. + +#### FP-3 — `dbt-build-green`: deliberately disabling a model always blocks + +State B9. `{{ config(enabled=false) }}` removes the node from the manifest, so +the same coverage assertion fires. Retiring a model is a legitimate, common +change. + +``` +The build is not green: 1 model(s) you edited were never built: retired_model. +``` + +FP-2 and FP-3 share one root cause: `not_built` treats "absent from +`run_results`" as evidence of an unbuilt model, but dbt legitimately omits +ephemeral and disabled nodes. + +#### FP-4 — `dbt-dialect-guard`: a nested `{% if %}` breaks guard detection + +State B2. `listagg()` sits inside a `{% if target.type == 'snowflake' %}` block +that also contains an inner `{% if var(...) %}…{% endif %}`: + +``` +1 unguarded warehouse-specific construct(s) in 1 model(s) you edited: nested_guard. + findings: [{"model":"nested_guard","function":"listagg()","dialects":"Snowflake / Redshift / Oracle"}] +``` + +`TARGET_TYPE_GUARD_RE` is non-greedy from `{% if … target.type … %}` to the +**first** `{% endif %}`. The inner `{% endif %}` closes the blanked region +early, so the still-guarded remainder is scanned and reported. Nested Jinja +inside a dialect guard is normal dbt. + +#### FP-5 — `dbt-dialect-guard`: string literals are not masked + +State B4. `'listagg('` as a string literal is flagged. + +Isolating the two halves of the composite probe shows the boundary precisely: +a `--` comment containing `listagg( … )` is **not** flagged (so +`stripSqlComments` works), while `select 'listagg(' as never_executed` **is**. +Lower frequency than FP-1..4, but the same class of bug: text matching without +lexical masking. + +### True-positive discrimination (known-bad states, n = 11) + +**8 of 11 known-bad states fired; 3 were silent.** This is the other half of the +safety question — a gate that never fires is also useless. + +| Known-bad state | Fired | +|---|---| +| `real-03` pristine: build genuinely red (`team_game_counts` errors) | `dbt-build-green` ✓ | +| `real-06`, `real-07` pristine: models edited, project does not parse, no artifact | `dbt-build-green` ✓ | +| `real-07`: required `stg_nba_games` does not exist | `dbt-nothing-built` ✓, `dbt-deliverable-names` ✓ | +| jaffle + `listagg()` unguarded in a project that establishes the guard convention | `dbt-dialect-guard` ✓ | +| jaffle + `incremental_strategy='delete+insert'` with no `unique_key` | `dbt-incremental-config` ✓ | + +The three silent known-bad states were `real-03` read-only, `real-06` build-only +and `real-06` read-only. All three are explained by the recall gaps below rather +than by a logic error. + +**Two recall gaps worth noting** (misses, not false positives): + +* `real-06`'s prompt reads *"Add the missing `models/staging/stg_nba_teams.sql`"*. + `REQUIREMENT_VERB_RE` covers `creat|build|produc|implement|deliver|materiali[sz]|generat|writ|deploy` + — **not `add`**. `real-07` says *"Implement the missing …"* and is picked up. + One word decides whether the contract-driven validators activate at all: of + ten real task documents, exactly one (`real-07`) yielded a literal contract. +* `dbt-dialect-guard` never activated on any of the ten `real-*` tasks or on + stock jaffle_shop: none of those projects establishes a `target.type` + convention, which is the validator's activation precondition. Its real-world + coverage is therefore narrow by design. + +### Adjacent finding: the two pre-existing lane members (out of PR scope) + +Not part of PR #1175, but it matters to anyone deciding whether to switch +`ALTIMATE_VALIDATORS_ENABLED=1` on, because that flag turns on all **seven** +registered validators, not five. + +Probing the full lane against a green, complete `real-01` workspace produced: + +``` +dbt-schema-verify ok:false errored=3/5 models elapsed_ms=10864 +dbt-tests-pass ok:false errored=4/5 models elapsed_ms=13919 +``` + +Zero actual mismatches and zero actual test failures — every one of those is a +subprocess that did not return a parseable result, and both validators treat +"could not verify" as "blocks". Two contributing causes were observed: the +globally-installed `altimate-dbt` predates `schema-verify` entirely (so the +subprocess errors out), and even with a freshly built binary on `PATH` the +validators fan out at `concurrency_limit: 4` against a single DuckDB file, which +is single-writer. They also cost **11–14 s each**, three orders of magnitude +more than the five under test. + +Separately, `schema-verify` treats columns present in the model but absent from +`schema.yml` as `columns_extra` — and partially-documented models are the norm +in real dbt projects, so that is a large latent false-positive surface on its +own. None of this is PR #1175's doing, but it means "enable the lane" and +"enable these five" are very different decisions. + +--- + +## Measurement 2 — A/B conversion (does it help?) + +**Read the sample-size disclosure at the top of this document first. This +section does not support a conclusion about conversion.** + +### Design (as intended) + +Same model, same prompts, same pinned seed (1001) and temperature (0.2), same +`--max-turns 40`, same task workspaces. Arm A: no validator env set. Arm B: +`ALTIMATE_VALIDATORS_ENABLED=1` with the five under test registered. Grading by +each task's own `verify.sh`, which wipes `target/`, rebuilds from scratch and +diffs against a golden expectation file — so the grader is independent of +anything the validators looked at. + +### Achieved N + +| | planned | achieved | +|---|---|---| +| tasks | 10 | 2 | +| arms per task | 2 | 2 for `real-01`, 1 for `real-07` | +| rollouts per cell | 2 | 1 | +| total sessions | 40 | **3** | +| complete A/B pairs | 20 | **1** | + +Terminated for machine capacity, not because the result was in. Two further +arm-B sessions (`real-07`, `real-10`) were in flight and were killed; their +partial data is discarded, not reported. + +### Runs completed + +| Task | Arm | verify.sh | wall clock | assistant steps | tool calls | validator dispatches | validator retries | validators that fired | +|---|---|---|---|---|---|---|---|---| +| `real-01-home-team-join` | A (off) | **pass** | 478 s | 14 | 24 | 0 | 0 | — | +| `real-01-home-team-join` | B (on) | **pass** | 392 s | 13 | 16 | 1 | 0 | — | +| `real-07-add-game-staging` | A (off) | **pass** | 867 s | 29 | 37 | 0 | 0 | — | + +### What can and cannot be read from this + +**Cannot be read:** any pass-rate difference, any conversion effect, any wall-clock +or turn-count overhead. One pair is one pair; the 478 s → 392 s difference between +the two `real-01` arms is ordinary run-to-run variance in a nondeterministic +agent loop, not a measured effect of the gate, and it would be dishonest to +present it as one. + +**Can be read**, because they are structural observations rather than statistics: + +1. **The hook fires where it is supposed to and nowhere else.** In arm A the + session logged `validator_hook_reached` on every step with + `validatorsEnabled: false` and **zero** `dispatch_enter` events — the flag-off + path costs nothing, as the code comment claims. In arm B the dispatch ran + exactly once, on the single step where the model declared a clean stop. + +2. **Only two of the five ever became applicable on this task.** + `dbt-nothing-built`, `dbt-deliverable-names` and `dbt-dialect-guard` all + returned `appliesTo: false`, consistent with the offline sweep: the first two + need a task document with a literal deliverable contract (1 of 10 real task + prompts produced one), the third needs a project that already uses + `target.type` guards (0 of 11 projects tested). + +3. **`dbt-build-green`, the lane's central gate, degraded to a no-op on this + real session.** Its own telemetry from the live dispatch: + + ```json + {"name":"dbt-build-green","ok":true,"details":{ + "models_touched":1,"run_results_fresh":true,"verdict":"fresh-build", + "coverage_assertable":false,"model_nodes_in_artifact":0, + "failed_in_scope":[],"not_built":[],"stale_build":[]}} + ``` + + `model_nodes_in_artifact: 0` means the agent's final dbt command left a + `run_results.json` containing test nodes only, so the coverage assertion was + skipped by design (`coverage_assertable: false`). The gate returned `ok:true` + **without checking anything** — it would have passed identically had the + model never been built. The same blind spot reproduces deterministically in + the offline probe (state B6). This is the documented conservative fallback, + but it means the gate's real-world discriminating power depends on which dbt + command the agent happens to run last, which the gate does not control. + +### What a real A/B would require + +On a GCP VM, isolated from developer machines: + +* **Scale:** 10 tasks × 2 arms × 3 rollouts = 60 sessions. At the observed + 392–867 s per session, that is ~9 h serial, or ~2 h at concurrency 5 on a + machine that can take it (each session is one `bun` process plus a DuckDB + build; ~4–6 GB RSS observed per session, so size for ≥8 vCPU / 32 GB). +* **A third arm.** Arm A (off) and arm B (enforce) are not enough, because a + fired validator changes the trajectory and destroys its own counterfactual. + Add **arm S (`ALTIMATE_VALIDATORS_SHADOW=1`)**: validators run and log but do + not enforce, so shadow tells you the true fire rate on unperturbed sessions, + and the arm-A/arm-S verdict difference should be zero (a sanity check on the + harness). +* **Tasks that can actually fire the gates.** Of the 10 `real-*` prompts, only + one yields a deliverable contract and none establishes a `target.type` + convention, so three of the five validators are structurally unreachable on + this task set. A conversion study needs a task set where each validator has a + reachable failure mode — otherwise arm B is arm A with extra logging. +* **A pre-grade workspace snapshot.** `verify.sh` deletes `target/` and + rebuilds, which destroys the very artifact state the validators inspect. + Snapshot the workspace before grading so post-hoc probes are valid. +* **Report per-dispatch telemetry, not just pass rates** — `dispatch_result` + already carries everything needed (`coverage_assertable`, `not_built`, + `stale_build`, per-validator `elapsed_ms`). + +--- + +## Negative control — non-dbt project + +A live session in a small TypeScript repo (`/tmp/valexp/negctl/repo`: two source +files, a `bun test` suite, a README, no dbt anywhere), with the **full lane** +enabled and the artifact gate additionally forced on: + +``` +ALTIMATE_VALIDATORS_ENABLED=1 +ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS=1 +(ALTIMATE_VALIDATORS_ONLY unset — all seven validators registered) +``` + +Task: *"Add a `stddev` function to src/stats.ts and a unit test for it. Run +`bun test`."* The session completed the work (`stddev` present in both files) +and stopped cleanly. + +Validator events, verbatim from the session's stderr: + +``` +validator_hook_reached step=1..5 finish=tool-calls validatorCount=7 +validator_hook_reached step=6 finish=stop validatorCount=7 +dispatch_enter step=6 +dispatch_result step=6 checks_count=0 results=[] +``` + +**Zero validators executed.** All seven were registered and the dispatch ran, +but every `appliesTo()` returned false because `findDbtProjectRoot()` finds no +`dbt_project.yml`. No synthetic message was injected (`grep -c +"altimate-validator:" trace.jsonl` → 0). The gate is inert outside dbt, as +designed, even with the most aggressive opt-in set. + +--- + +## Cost overhead + +Per-validator `check()` wall time, measured across 50 probe invocations on the +small projects (5–6 models): + +| Validator | mean | max | +|---|---|---| +| `dbt-build-green` | 1.4 ms | 4 ms | +| `dbt-incremental-config` | 1.7 ms | 5 ms | +| `dbt-dialect-guard` | 1.2 ms | 2 ms | +| `dbt-nothing-built` | 1.3 ms | 2 ms | +| `dbt-deliverable-names` | 0.8 ms | 2 ms | + +Whole-dispatch wall time on those projects: **mean 10 ms, max 39 ms.** +Negligible. + +**Scaling is the caveat.** On a synthetic 2 005-model project: + +| Scenario | `dbt-build-green` | `dbt-incremental-config` | dispatch total | +|---|---|---|---| +| all 2 005 models modified this session | 300 ms | 738 ms | 2 853 ms | +| zero models modified this session | 649 ms | (skipped) | 3 454 ms | + +Cost is dominated by the directory walk, not by the analysis, and **it is paid +even when nothing was touched**. Each validator runs its own independent +`modelsModifiedSince()` / project scan with no shared work or caching, so the +tree is walked up to five times per dispatch, and the dispatch fires on every +clean stop. On a large monorepo that is seconds of wall time per turn boundary. + +The dominant cost of enforce mode is not CPU — it is the injected retry turn, +which is a full model turn plus whatever tool calls the model makes in response. + +--- + +## Verdict + +**Shadow only. Do not enable by default yet.** Two independent reasons. + +**1. Five reproducible false positives, and they are not exotic.** Editing an +ephemeral model, disabling a model, touching a file after the build, nesting a +Jinja `if` inside a dialect guard — these are ordinary dbt work, not defects. In +enforce mode each one costs a session a synthetic retry turn, and the fix hint +tells the agent to do something that is either impossible (`eph_helper` can never +appear in `run_results`) or wrong (rebuild after a whitespace change). A gate +that burns its retry budget on non-problems is exactly the failure mode the +lane's own design doc warns about. All four of the highest-frequency ones are +small, contained fixes: + +* `dbt-build-green`: exclude nodes that dbt legitimately omits from + `run_results` — resolve `manifest.json` for `enabled: false` and for + `materialized: ephemeral` before asserting coverage. +* `dbt-build-green`: compare **content**, not mtime, for the stale check, or + raise the tolerance far above 1 s and skip files whose comment-stripped SQL is + unchanged since the build. +* `dbt-dialect-guard`: match `{% if %}`/`{% endif %}` by nesting depth instead + of a non-greedy regex; mask string literals as well as comments. + +**2. There is no conversion evidence at all.** N = 1 paired task, and that pair +needed no retry. Nothing here shows the gate turns a failure into a pass, and +nothing here shows it does not. The claim in PR #1175 that these gates improve +outcomes is, on this evidence, **untested** — which is a different and more +honest statement than "disproven". + +**What the evidence does support.** The safety envelope is genuinely +conservative where it was designed to be: 0 firings across 27 naturalistic +known-good end-states, 0 false positives from three of the five validators, a +clean negative control in a non-dbt repo with the most aggressive opt-in forced +on, and negligible CPU cost on normal projects. The true-positive side works +too: every constructed defect was caught. The problem is not that the lane is +reckless; it is that its blast radius includes a handful of legitimate dbt +practices, and its benefit is unmeasured. + +**Recommended sequence.** + +1. Ship the five behind `ALTIMATE_VALIDATORS_SHADOW=1` only. The lane already + emits per-validator `validator_check` telemetry with `enforced: false`, which + is enough to measure the real fire rate on real traffic. +2. Fix FP-1 through FP-5 (above) and add the five constructed known-good states + as regression tests — they are a few lines each and all five reproduce + deterministically. +3. Widen `REQUIREMENT_VERB_RE` to include `add` (and probably `convert`, + `rename`, `fix`, `repair`); today one verb decides whether two of the five + validators activate at all. +4. Look hard at whether `dbt-build-green`'s coverage assertion should survive + `dbt test` overwriting `run_results.json`. Reading `manifest.json` (which is + not overwritten by `dbt test`) alongside the run artifact would close the + blind spot that made the gate a no-op on the one real session measured here. +5. Only then run the VM-based three-arm A/B described above, and decide on + enable-by-default from those numbers. + +**Not recommended:** reverting. The checks are cheap, the design is sound, the +true-positive behaviour is real, and the defects found are localized bugs rather +than a flaw in the approach. + +**Also not recommended:** flipping `ALTIMATE_VALIDATORS_ENABLED=1` as a lane-wide +default, on the strength of these five. That flag also enables +`dbt-schema-verify` and `dbt-tests-pass`, which in this environment blocked a +green, complete project on subprocess errors alone and cost 11–14 s each. Those +two need their own evidence before the lane ships enabled. + +--- + +## Reproducing this + +Probe harness (not committed; recreate at the repo root): + +```ts +// validator-probe.ts — bun run validator-probe.ts