feat(validators): deterministic completion gates — zero-write, build-green, literal deliverables, config/dialect lints

Merged
AN
2mo ago
+11199-22

Issue for this PR

Closes #1174

Type of change

  • New feature
  • Bug fix

What does this PR do?

Adds five deterministic completion-gate validators to the existing ALTIMATE_VALIDATORS_ENABLED lane, and closes a structural blind spot in that lane. Every check is answer-free — it asserts structure, invariants, or the task's own literal contract, never a known-correct output — so the gates work on unseen tasks.

Closes the zero-write blind spot. Both pre-existing validators key on "did the session modify models", so a session that authored nothing passed every gate by default. dbt-nothing-built is an inverse gate: in a dbt project, with no session-authored files and no fresh successful run artifact, the session is not done. It is deliberately conservative — appliesTo returns false unless a task document literally names required deliverables, or ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS=1 is set — so genuinely read-only/analysis sessions are unaffected.

New validators

ValidatorCatches
dbt-nothing-builtDeclared done without producing any deliverable
dbt-build-greenEdited-but-never-built; artifact predating the session; fresh artifact where the model errored, is missing, or was edited after the build
dbt-deliverable-namesRequired model/relation names not produced; self-chosen substitutes. Diffs literal names against filesystem inventory u manifest names/aliases
dbt-incremental-configmerge/delete+insert without unique_key; missing is_incremental() guard where the task literally requires idempotency; non-deterministic calls inside the guard predicate
dbt-dialect-guardUnguarded warehouse-specific functions, only in projects that already use target.type guards (or opt-in via env)

Conservative by construction. No fuzzy matching anywhere — required names come from three literal tiers only. No discoverable source of required names means a silent skip, never a false failure. dbt_project.yml-inherited config is intentionally not resolved rather than guessed. Non-determinism outside an is_incremental() predicate is advisory detail, never blocking. Out-of-scope build failures are telemetry, not a block. Per the lane's existing contract, a validator that throws soft-passes, so a buggy check cannot brick a session.

Also adds docs/internal/deterministic-checks-engine-split.md, assessing two further candidate checks that need real SQL parsing rather than filesystem/regex analysis, and where each belongs relative to the engine's existing capabilities.

Reviewer-critical: these gates almost never get to run on a current build

None of the dispatch code is in this PR. git diff main...HEAD touches no file under packages/opencode/src/session/. The hook already exists on main; this PR only adds validators to the registry it calls. The consequence is that the value of this PR is contingent on how often that hook fires, which is a property of the harness, not of these five validators.

The gate condition, at packages/opencode/src/session/prompt.ts:1405-1412:

if (
  validatorsActive &&
  result !== "stop" &&
  result !== "compact" &&
  processor.message.finish === "stop" &&
  !processor.message.error &&
  validatorCount > 0
)

Two conditions skip dispatch entirely, and neither logs anything beyond the validator_hook_reached line:

  1. processor.message.finish === "stop" — the model must declare a clean stop. Any other finish reason (tool-calls, length, unknown, or an error) skips.
  2. result !== "compact" — processor.process() returns "compact" when the step ended in compaction (packages/opencode/src/session/processor.ts:661), and those steps skip too. result === "stop" (blocked, or an assistant-message error) skips as well.

Measured on a pre-#1171 build, 9 trials / 72 validator_hook_reached events:

finishresultcount
tool-callscontinue54
tool-callscompact17
stopcontinue1

The gate fired on 1.4% of step-ends. All seven validators registered every time, so registration is fine, and the one dispatch went hook → dispatch_enter → dispatch_result with no throw or hang: the mechanism is correct whenever it is reached. The accurate word is structurally starved, not broken — sessions on that build end by exhausting turns or by compacting, not by declaring completion.

#1171 is the dependency. It replaces trusting a bare provider finish-stop with explicit DONE-token termination (session/termination.ts). Until it lands, enabling this lane on a current build is close to a no-op. A reviewer evaluating this PR on merit should know that the pre-#1171 measurements say nothing about whether these gates work.

What the single dispatch showed, reported because it is the only enforcement event in the run and a reviewer should hear it here rather than find it later:

checks=4  dbt-build-green ok=True  dbt-incremental-config ok=True
          dbt-schema-verify ok=True  dbt-tests-pass ok=FALSE

Three of this PR's five validators did not declare themselves applicable at all. The two that ran both passed. The only validator that failed was dbt-tests-pass, which is pre-existing, not one of the five — and it converted nothing: the off, shadow and enforce arms all passed that task, so the retry it triggered was spent on work that would have succeeded anyway. This is N = 1 on a build where the gate is starved; it is not a verdict on the five, in either direction.

Open design question for the reviewer, deliberately not acted on here. The two endings where dispatch is silent — budget exhaustion and compaction — are arguably the two where a completion gate is most valuable, because they are the endings most likely to hand a user unfinished work. There is a defensible reading (the gates check a claim of completion; no claim, nothing to validate; a stalled run has already failed by other means) and a worrying one (this is a gate that is quiet precisely when it should be loud, which is the same shape as the dbt-build-green-passes-with-zero-models bug this PR exists to fix). Changing it means running validators on abnormal finish reasons and converting a silent stall into an explicit failure — a change to shared harness code outside this PR's scope. Flagging it for a decision rather than making it.

Recommendation: shadow mode first, not enable-by-default

docs/internal/validator-e2e-evidence.md reports an end-to-end run against real dbt projects rather than fixtures. The recommendation from it is ALTIMATE_VALIDATORS_SHADOW=1 only, and it rests on two independent findings.

1. Five reproducible false positives, since fixed — and the class of bug matters more than the count. Across 38 known-good states the gates fired five times, every one on ordinary dbt practice rather than on a defect: editing an ephemeral model, disabling a model on purpose, touching a file seconds after a green build, a nested {% if %} inside a target.type guard, and a dialect function name inside a string literal. In enforce mode each costs a session a synthetic retry turn, and in two of the five the fix hint asked for something impossible. All five now have regression tests asserting no firing on the known-good state, and the evidence run also found that dbt-build-green returned green having checked nothing when the session's last dbt command was a dbt test (which overwrites run_results.json with test nodes only). That is fixed too — coverage now also reads the model DDL under <target>/run/, which a test invocation does not touch.

Fixing the observed false positives does not establish that no others exist. Shadow mode is how you find out, on real traffic, without spending anyone's retry budget.

2. There is no conversion evidence in either direction. The planned A/B was 10 tasks x 2 arms x 2 rollouts. Three sessions completed: N = 1 paired task plus one unpaired run, terminated for machine capacity. The single pair needed no retry, so there was nothing to convert. Nothing here shows these gates turn a failure into a pass, and nothing here shows they do not — the honest word is untested, not disproven. What a properly powered run would need (three arms including shadow, a task set where each gate is reachable, a pre-grade workspace snapshot) is specified in the evidence document.

Cost. ~2-10 ms per dispatch on a small project, but ~1-3.5 s on a 2 000-model project even when the session touched no models, because each validator walks the tree independently. That cost is paid on every dispatch, and consolidating the walks is recorded as a follow-up rather than done here.

ALTIMATE_VALIDATORS_ENABLED=1 is not the same decision as "enable these five." That flag activates all seven registered validators, including the two pre-existing ones. Probed against a green, complete project, dbt-schema-verify and dbt-tests-pass both returned ok:false with zero actual mismatches and zero actual test failures — every one was a subprocess that did not return a parseable result, and both treat "could not verify" as "blocks". They also cost 11-14 s each. None of that is this PR's doing, but it is what switching the lane on today would actually do.

Suggested sequence: shadow these five and measure the real fire rate; then run the three-arm A/B on isolated infrastructure; then decide on enforcement from those numbers.

How did you verify your code works?

  • Validator suites: bun test test/altimate/validators/ -> 574 pass, 132 skip, 0 fail. That includes regression tests for each observed false positive, for the test-overwrite blind spot and its discriminating converse, and for the shared task-parsing and SQL-scrubbing utilities.
  • Full suite: bun test test/session/ test/altimate/ -> 5039 pass, 649 skip, 2 fail. Both failures are pre-existing and were confirmed by re-running them on a detached origin/main checkout: a 5 s timeout in test/session/prompt.test.ts and a PostgreSQL driver E2E that requires a local database.
  • bun run typecheck clean; marker guard (--markers --base main --strict) clean.
  • Negative control: a live session in a non-dbt TypeScript repo with the lane and the artifact opt-in both forced on. Zero validators executed.

Not verified, stated plainly:

  • No conversion measurement. See above — N = 1, inconclusive.
  • Fixing the five observed false positives does not bound the false-positive rate. The 38 known-good states are a sample, not a proof.
  • The lane's dispatch hook is still skipped when a step ends in compaction, so in compaction-heavy sessions these gates fire rarely. Pre-existing, out of scope here, noted in the engine-split document.
  • Per-dispatch cost on large projects has not been optimised; each validator still walks the project tree independently.

Deferred and declined review findings are recorded with rationale in .github/meta/harness-review-followups.md.

Repo note: script/upstream/analyze.ts fails out of the box in a fresh worktree with Cannot find package 'minimatch' (no longer a transitive dep since glob@13). Worked around transiently to run the marker check; worth fixing separately.

Screenshots / recordings

N/A - no user-visible surface; these run inside the completion-gate lane.

Checklist

  • Tests added for new behavior
  • Typecheck passes
  • Marker guard passes
  • No changes to default behavior (lane remains opt-in via env)

[!NOTE]
Medium Risk
Completion validators can block session termination and inject synthetic retries when ALTIMATE_VALIDATORS_ENABLED is on; documented false-positive classes and consolidation requirements remain before broad enforcement.

Overview
Adds five filesystem/regex completion validators to the Altimate lane and registers them before dbt-schema-verify / dbt-tests-pass: inverse nothing-built when tasks name deliverables; build-green over fresh run_results.json plus <target>/run/ DDL with finer verdicts (fresh-build, build-unproven, etc.) and rejection of non-executing artifacts like dbt compile; deliverable-names against task prose; incremental-config and dialect-guard lints on session-touched models.

The new gates lean on shared utilities (configured dbt paths, manifest-aware matching where added, sanitizeForPrompt on retry text) and only run when the existing session hook dispatches on a clean stop—still opt-in via env.

Also adds internal docs: superseded e2e evidence, engine-vs-lane split for SQL-parse checks, and a long deferred follow-ups meta doc that records open gaps (duplicated heuristics, bare-name identity) and argues against turning on full enforcement until shared primitives exist.

Reviewed by Cursor Bugbot for commit b97c58a3f7cb4cd8a99ea14418ecf2853a38f6e2. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Closes #1174 by adding five deterministic completion gates to the Altimate validator lane. Sessions could previously finish green after writing nothing, missing required names, or citing stale or non-executing build evidence; the gates now report these failures and can trigger a synthetic retry.

  • dbt-nothing-built blocks when a task document or ALTIMATE_VALIDATORS_REQUIRE_ARTIFACTS=1 demands deliverables and none were produced or built; modification targets outside scanned dbt source paths are now satisfied by fresh mtime.
  • dbt-build-green verifies edited models against fresh build evidence, rejects non-executing commands like dbt compile, and falls back to <target>/run/ DDL when run_results.json carries only test nodes.
  • dbt-deliverable-names diffs literal model, seed, and snapshot names from task prose against produced inventory; dbt-incremental-config flags keyless upserts, missing idempotency guards, and non-deterministic incremental predicates; dbt-dialect-guard flags unguarded warehouse-specific calls in projects that use target.type.
  • Shared utilities honor configured dbt paths, skip vendored dbt_packages rewrites, resolve Jinja env_var target paths, and sanitize repository-derived retry text and telemetry.
  • The lane stays off by default; ALTIMATE_VALIDATORS_ENABLED=1 runs all seven validators and shadow mode suppresses only the retry, while dispatch still skips compaction and other non-stop endings. Enforcement must not ship beyond a shadow soak until five duplicated heuristics are consolidated into shared primitives.
  • Fixture-based regression tests cover artifact provenance, configurable layouts, Jinja edge cases, and vacuous passes; docs record the engine-split assessment and deferred findings.

Summary by CodeRabbit

  • New Features

    • Added dbt completion checks for fresh, successful builds of edited models.
    • Added checks confirming required models and files were delivered.
    • Added validation for warehouse-specific SQL usage and incremental model configuration.
    • Added safeguards when no dbt artifacts are produced after changes.
    • Validators now run in a defined order and provide actionable failure guidance.
  • Bug Fixes

    • Improved SQL and Jinja parsing to reduce false positives.
    • Improved handling of versioned, exempt, and vendored dbt models.
  • Documentation

    • Added guidance and evidence for evaluating deterministic validator behavior.