AltimateAI
diff --git a/‎CHANGELOG.md‎
Lines changed: 23 additions & 0 deletions b/‎CHANGELOG.md‎
Lines changed: 23 additions & 0 deletions
diff --git a/‎docs/docs/usage/dbt-pr-review.md‎
Lines changed: 21 additions & 1 deletion b/‎docs/docs/usage/dbt-pr-review.md‎
Lines changed: 21 additions & 1 deletion
diff --git a/‎packages/opencode/src/altimate/review/orchestrate.ts‎
Lines changed: 13 additions & 7 deletions b/‎packages/opencode/src/altimate/review/orchestrate.ts‎
Lines changed: 13 additions & 7 deletions
diff --git a/‎packages/opencode/src/altimate/review/run.ts‎
Lines changed: 61 additions & 19 deletions b/‎packages/opencode/src/altimate/review/run.ts‎
Lines changed: 61 additions & 19 deletions
@@ -5,6 +5,29 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.9.3] - 2026-07-24
+
+Focused polish on the `altimate review` dbt PR reviewer — plus TUI startup UX
+and audit-trail hardening on the signed verdict envelope.
+
+### Added
+
+- **Grain-key `not_null` completeness detector.** For every column named in a `unique_combination_of_columns` test (or the `dbt_utils` equivalent), the reviewer now checks that a `not_null` test or constraint exists on the same column — without it, a NULL grain value silently passes the uniqueness guardrail. Supports contracted models (`constraints:` with `contract.enforced: true`), column-level `data_tests:` / `tests:`, and model-level primary-key constraints. Case-folds column names for Snowflake compatibility. Delivered +11 real findings on the internal 5-PR corpus. (#1029)
+- **`--explain-tier` flag.** Pass `--explain-tier` to surface the classifier's tier-reason list on the verdict envelope so you can see why a diff was rated `trivial`, `lite`, or `full`. Full-tier verdicts also now include the reason line in the PR comment by default (no flag required), so a `REQUEST_CHANGES` on a schema.yml diff is never unexplained. (#1027)
+- **`--force-tier` flag.** *[EXPERIMENTAL / bench debug]* Bypass the classifier with `--force-tier=trivial|lite|full`. The signed envelope carries `tierForced: true` and the classifier's original decision, so an auditor always sees the bypass — the flag can't be used silently. (#1027)
+- **`riskTierPathTokens` config in `.altimate/review.yml`.** Configure named token categories (including `preset:finops` for the built-in cost/billing keyword list) to promote matching files to full review tier. Previously the FinOps preset was hardcoded and always on; it is now opt-in via this config. A typo in a category value surfaces in both stderr and the verdict envelope `tierReasons[]` (and the PR comment) so it can't kill your opt-in silently. (#1028)
+- **`engine.cliVersion` in the signed verdict envelope.** The altimate-code release that generated each verdict is now stamped into `engine.cliVersion` and covered by the HMAC signature — auditors reconstructing decisions from stored envelopes months later can identify which policy version applied.
+- **`staleManifest` flag on the verdict envelope.** When a change-affecting source file has been modified after the manifest was written, the signed envelope now carries `staleManifest: true`. A stderr warning alone is easy for CI to swallow; the envelope field is durable and part of the signed body.
+- **Bootstrap phase labels in the TUI.** While the agent initializes, the busy spinner now shows what it is actually doing ("Loading config…", "Discovering tools…", "Thinking…") matching the convention users already know from other agent UIs. Phase events are also emitted as trace spans for server-side latency profiling. (#1002)
+
+### Changed
+
+- **Risky dbt metadata surfaces never auto-approve.** PRs that touch `data_tests:`, `tests:`, `constraints:`, `contract:`, or `unique_combination_of_columns` in schema.yml under `models/marts/` — or files under any configured `riskTierPathTokens` category — are always promoted to `full` review tier. A small metadata diff can no longer slip through on `trivial` classification. Grounded in a corpus study where two PRs auto-approved with 8–11 human-visible findings each. **CI in `gate` mode may see exit-code changes on affected PRs.** (#1028)
+- **`--no-ai` flag now actually disables the LLM lane.** A yargs `boolean-negation` conflict caused a bare `--no-ai` to silently fall into the help path (exit 0, no review ran). The flag is fixed; CI scripts using `--no-ai` since v0.8.x should verify behavior on upgrade — those runs may have been executing the AI lane and getting billed. (#1027)
+- **`altimate review` auto-discovers `target/manifest.json`.** When `--manifest` is not passed and the config-relative path doesn't resolve, the reviewer walks up from `cwd` to the nearest `dbt_project.yml` and uses its adjacent `target/manifest.json`. Discovery is logged to stderr; explicit `--manifest` still wins. Removes the most common "review did nothing" support case for mono-repo invocations. (#1027)
+- **schema.yml test-removal detection is now column-aware.** The detector parses both sides of the diff structurally (YAML parse rather than diff-line pattern) and keys removals by `(model, column, test)` — eliminating false cancellations where a sibling column re-added the same test type. (#1027)
+- **Stale-manifest detection now runs for local working-tree reviews too.** Previously gated behind `--head`, so the local workflow "`dbt compile` once, edit for an hour, then `altimate review`" — where staleness bites in practice — silently under-warned. The check is limited to files that could materially change the manifest (models, schema.yml, seeds, macros, dbt config) so noise stays bounded.
+
 ## [0.9.2] - 2026-07-20
 
 ### Added
 
@@ -106,11 +106,13 @@ Options:
 |------|-------------|
 | `--base <ref>` | Base git ref. Defaults to the merge-base with `origin/main`. |
 | `--head <ref>` | Head git ref. Omit to review the working tree. |
-| `--manifest <path>` | Path to the compiled `manifest.json` (default `target/manifest.json`). |
+| `--manifest <path>` | Path to the compiled `manifest.json`. When omitted, the reviewer walks up from the current directory to find the nearest `dbt_project.yml` and uses its adjacent `target/manifest.json`; the discovered path is logged to stderr. |
 | `--mode comment\|gate` | `comment` never blocks; `gate` exits non-zero on `REQUEST_CHANGES`. |
 | `--severity <level>` | Minimum severity to surface: `critical`, `warning`, `suggestion`. |
 | `--post` | Post the verdict to the GitHub PR (uses `GITHUB_TOKEN` + the Actions event). |
 | `--no-ai` | Disable the advisory LLM reviewer lane (no model calls / cost) — deterministic-only. |
+| `--explain-tier` | Emit the classifier's tier-reason list on the verdict envelope so you can see why a diff was rated `trivial`, `lite`, or `full`. Reasons already surface in the PR comment for `full`-tier runs — this flag adds them to `trivial`/`lite` for debugging. |
+| `--force-tier <tier>` | **[EXPERIMENTAL / bench debug]** Bypass the classifier and force `trivial` / `lite` / `full`. The verdict envelope carries `tierForced: true` and the classifier's original decision for audit. |
 | `--json` / `--output <file>` | Emit the verdict envelope as JSON. |
 
 > **Full vs lint-only.** With a compiled `manifest.json` present, the reviewer
@@ -232,6 +234,9 @@ dataDiff:                     # OFF by default — see "Data-diff in CI" below
   warehouse: ""               # connection name; empty = default connection
 exclude:
   - models/legacy/**
+riskTierPathTokens:           # OFF by default — see "Risk-tier path tokens" below
+  finops: [preset:finops]     # promote any FinOps-named path to `full` tier
+  pii: [ssn, email, phone]    # or supply your own case-insensitive tokens
 rubric:
   blockOn: [lineage_breakage, contract_violation, pii_exposure, semantic_change]
   warningPatternThreshold: 3
@@ -242,6 +247,21 @@ rubric:
     skipNonProdModels: true
 ```
 
+### Risk-tier path tokens
+
+Named categories under `riskTierPathTokens` promote any diff touching a matching
+path to `full` review tier, so those areas never auto-approve on `trivial`
+classification. Tokens are matched **case-insensitively at path, word, or digit
+boundaries** — not as arbitrary substrings. For example, the token `cost` fires
+on `models/marts/mrt_cost_daily.sql` (bounded by `_` and `_`) but **not** on
+`models/broadcaster.sql` (the substring lives inside a longer word). The
+`preset:finops` marker expands to the built-in FinOps keyword list (cost,
+billing, spend, revenue, etc.). Before v0.9.3 the FinOps list was hardcoded and
+always on; it is now opt-in via this config. When a category value is invalid
+the CLI logs a stderr warning AND surfaces the error in the verdict envelope's
+`tierReasons[]` (and in the PR comment), so a typo can't silently kill your
+opt-in.
+
 ## Data-diff in CI
 
 Static equivalence proves a refactor *can't* change results. **Data-diff** goes
 
@@ -180,6 +180,8 @@ export interface OrchestrateInput {
   manifestHash?: string
   coreVersion?: string
   modelVersion?: string
+  /** altimate-code CLI release recorded in engine.cliVersion for audit reconstruction. */
+  cliVersion?: string
   /**
    * Optional LLM reviewer lane. Injected (not imported) so the orchestrator
    * stays pure/unit-testable: production wires `runAiReview` (harness LLM);
@@ -194,6 +196,8 @@ export interface OrchestrateInput {
   explainTier?: boolean
   /** G2 — override the classifier's tier decision. Envelope carries tierForced:true. */
   forceTier?: "trivial" | "lite" | "full"
+  /** Manifest was stale relative to change-affecting files (see run.ts::detectStaleManifest). */
+  staleManifest?: boolean
 }
 
 /** Derive the dbt model name from a model file path. */
@@ -1423,17 +1427,19 @@ export async function runReview(input: OrchestrateInput): Promise<VerdictEnvelop
     tier,
     mode: input.mode,
     rubric: input.rubric,
-    engine: { core: input.coreVersion, model: input.modelVersion },
+    engine: { core: input.coreVersion, model: input.modelVersion, cliVersion: input.cliVersion },
     manifestHash: input.manifestHash,
+    staleManifest: input.staleManifest,
     generatedAt: input.generatedAt,
     degraded,
     // Include tierReasons whenever `--explain-tier` / `--force-tier` is set,
-    // OR when a riskTierPathTokens config error was caught above — the error
-    // must surface in the envelope (and downstream PR comment) even in a
-    // normal `comment`/`gate` run, otherwise a config typo silently kills
-    // the user's opt-in with only a stderr trace they'll never see
-    // (coderabbit review, PR #1028 orchestrate.ts:1129).
-    tierReasons: input.explainTier || tierForced || pathTokenConfigError ? tierReasons : undefined,
+    // when a riskTierPathTokens config error was caught above, OR when the
+    // classifier lands on `full` tier — a naturally-full run is a customer-
+    // visible policy call ("why did my YAML-only diff get REQUEST_CHANGES?")
+    // and the reason list is what makes the verdict debuggable without the
+    // customer having to re-run with --explain-tier. `trivial`/`lite` runs
+    // stay silent to avoid noise on approvals.
+    tierReasons: input.explainTier || tierForced || pathTokenConfigError || tier === "full" ? tierReasons : undefined,
     tierForced: tierForced ? true : undefined,
     tierClassified: tierForced ? classifiedTier : undefined,
   })
 
@@ -1,5 +1,6 @@
 import path from "node:path"
 import { readFile, realpath, stat, access } from "node:fs/promises"
+import { Installation } from "../../installation"
 import { loadReviewConfig, resolveRubric } from "./config"
 import type { Severity } from "./finding"
 import { collectChangedFiles, makeContentResolver, defaultBaseRef, gitRepoRoot, manifestHash } from "./git"
@@ -37,6 +38,8 @@ export interface ReviewPullRequestOptions {
   /** Model identifier recorded in the envelope. */
   modelVersion?: string
   coreVersion?: string
+  /** altimate-code CLI release recorded in engine.cliVersion for audit reconstruction. */
+  cliVersion?: string
   /** Disable the LLM reviewer lane (default: enabled; self-degrades if no model). */
   noAi?: boolean
   /** PR metadata for the AI reviewer's intent check. */
@@ -129,7 +132,7 @@ async function autoDiscoverManifest(cwd: string): Promise<{ path: string; projec
  *  docs markdown blocks) or top-level dbt config. `README.md` at repo root,
  *  `.github/`, `package.json`, etc. are excluded so their mtimes don't
  *  trigger a stale warning. Exported for tests — see review-run-stale.test.ts.
- *  Referenced by `warnIfStale`. */
+ *  Referenced by `detectStaleManifest`. */
 export function isManifestAffecting(rel: string): boolean {
   // Code / schema / seed CSV live under any dbt source directory.
   if (/(^|\/)(models|seeds|snapshots|macros|tests|analyses)\/.*\.(sql|py|yml|yaml|csv)$/i.test(rel)) return true
@@ -144,37 +147,63 @@ export function isManifestAffecting(rel: string): boolean {
   return false
 }
 
-/** Warn to stderr when the manifest looks stale relative to changed files.
- *  Compares the manifest's mtime against every change-affecting path (per
- *  `isManifestAffecting`); a single change newer than the manifest is
- *  enough to fire the warning once and return. Non-fatal — the review
- *  proceeds against the (possibly stale) manifest. */
-async function warnIfStale(manifestAbs: string, changedPaths: string[], fsRoot: string): Promise<void> {
+/** Detect a stale manifest and warn to stderr. Returns `true` when a
+ *  change-affecting file was modified after the manifest — signalling the
+ *  verdict may have been computed against out-of-date metadata. Non-fatal;
+ *  the review proceeds against the (possibly stale) manifest and the
+ *  caller mirrors the flag onto the signed envelope so a downstream
+ *  auditor can distinguish stale-manifest verdicts from clean ones
+ *  (stderr alone is easy for CI to swallow).
+ *
+ *  A deleted or renamed manifest-affecting file is treated as unconditionally
+ *  stale: the file's mtime cannot be checked (the path is gone) but the
+ *  manifest still references it, so the metadata is by definition out of date
+ *  (cubic-review PR #1041 — the prior path-only signature silently reported
+ *  clean for deletions and could false-certify verdicts against ghost models). */
+async function detectStaleManifest(manifestAbs: string, changed: ChangedFile[], fsRoot: string): Promise<boolean> {
   try {
     const manifestMtime = (await stat(manifestAbs)).mtimeMs
-    for (const rel of changedPaths) {
-      if (!isManifestAffecting(rel)) continue
-      // `changedPaths` are repo-root relative (from `git diff --name-status`),
+    for (const f of changed) {
+      // For renamed files, the OLD path is what the manifest still knows about;
+      // check both sides so both a rename-away and a rename-to a
+      // manifest-affecting shape trigger the signal.
+      const paths: string[] = [f.path]
+      if (f.status === "renamed" && f.oldPath) paths.push(f.oldPath)
+      if (!paths.some(isManifestAffecting)) continue
+
+      // Deleted / renamed files no longer exist at their old path on disk, so
+      // stat would fail. The manifest still references them though, so the
+      // metadata is stale by definition — fire immediately.
+      if (f.status === "deleted" || f.status === "renamed") {
+        process.stderr.write(
+          `⚠️  manifest ${manifestAbs} appears stale — ${f.status} \`${f.path}\` is a manifest-affecting path. ` +
+            `Re-run \`dbt compile\` (or \`dbt build\`) to refresh before reviewing.\n`,
+        )
+        return true
+      }
+
+      // `changed` paths are repo-root relative (from `git diff --name-status`),
       // so root at the git top-level rather than the caller's cwd. When the
       // CLI is invoked from a subdir the naive path.join(cwd, rel) points at
       // a non-existent path and the stale check silently no-ops.
-      const abs = path.isAbsolute(rel) ? rel : path.join(fsRoot, rel)
+      const abs = path.isAbsolute(f.path) ? f.path : path.join(fsRoot, f.path)
       try {
         const changedMtime = (await stat(abs)).mtimeMs
         if (changedMtime > manifestMtime) {
           process.stderr.write(
-            `⚠️  manifest ${manifestAbs} appears stale — ${rel} was modified after the manifest was written. ` +
+            `⚠️  manifest ${manifestAbs} appears stale — ${f.path} was modified after the manifest was written. ` +
               `Re-run \`dbt compile\` (or \`dbt build\`) to refresh before reviewing.\n`,
           )
-          return
+          return true
         }
       } catch {
-        /* file not on disk (e.g. removed by the change) — skip */
+        /* file not on disk for an added/modified entry — skip (rare race) */
       }
     }
   } catch {
     /* manifest unreadable → detectDialect will have already returned undefined */
   }
+  return false
 }
 
 export async function reviewPullRequest(opts: ReviewPullRequestOptions): Promise<VerdictEnvelope> {
@@ -241,11 +270,17 @@ export async function reviewPullRequest(opts: ReviewPullRequestOptions): Promise
       }
     }
   }
-  // Freshness check: warn (don't fail) when the manifest predates changed files.
-  // Skip when we're diffing against the working tree (mtime signal is noisy
-  // during active edits) — only warn when the caller explicitly pinned a head
-  // ref, which is the CI / bench shape where a stale manifest is a real risk.
-  if (opts.head) await warnIfStale(manifestAbs, changedFiles.map((f) => f.path), gitRoot)
+  // Freshness check: detect (and warn — non-fatal) when the manifest predates
+  // change-affecting files. Runs for both the pinned-head CI shape AND the
+  // working-tree local shape — the local scenario "dbt compile once, edit for
+  // an hour, then altimate review" is where staleness actually bites in
+  // practice, so gating this behind `--head` (the prior behavior) silently
+  // under-warned the most common developer workflow. mtime granularity on
+  // the working tree can be noisy during a live edit session, but the check
+  // is limited to files that would materially change the manifest
+  // (`isManifestAffecting`), so noise is bounded. Return value is stamped
+  // into the signed envelope so downstream auditors see it too.
+  const staleManifest = await detectStaleManifest(manifestAbs, changedFiles, gitRoot)
 
   // Resolve the SQL dialect: explicit config wins; otherwise auto-detect from
   // the dbt manifest's `adapter_type` (so a BigQuery/Redshift project isn't
@@ -306,10 +341,17 @@ export async function reviewPullRequest(opts: ReviewPullRequestOptions): Promise
     manifestHash: mhash,
     modelVersion: opts.modelVersion,
     coreVersion: opts.coreVersion,
+    // Default cliVersion so agent-invoked callers (dbt_pr_review tool) that
+    // don't thread the version get the audit-trail field populated too.
+    // Only the CLI command explicitly forwards Installation.VERSION today,
+    // so a missing default silently regresses envelope provenance for tool-
+    // path verdicts (cubic-review PR #1041).
+    cliVersion: opts.cliVersion ?? Installation.VERSION,
     aiReview: opts.noAi || config.ai === false ? undefined : runAiReview,
     prTitle: opts.prTitle,
     prBody: opts.prBody,
     explainTier: opts.explainTier,
     forceTier: opts.forceTier,
+    staleManifest,
   })
 }