review: --no-ai fix + --explain-tier / --force-tier flags + manifest auto-discovery + column-aware schema.yml test-removal

packages/opencode/src/altimate/review/diff-filter.ts

@@ -61,7 +61,11 @@ export function classifyDbtFile(path: string): DbtFileKind {
   const isYaml = p.endsWith(".yml") || p.endsWith(".yaml")
   if (/(^|\/)(dbt_project|profiles|packages|dependencies)\.ya?ml$/.test(p)) return "project_config"
   if (/(^|\/)macros\//.test(p)) return "macro"
-  if (/(^|\/)snapshots\//.test(p)) return "snapshot"
+  // Snapshots split by extension: `.sql` files are snapshot definitions
+  // (tier-forcing, catalog-scoped), YAML property files under `snapshots/`
+  // are schema files (declared tests + column metadata) and should route to
+  // `schema_yml` so the test-removal detector runs on them.
+  if (/(^|\/)snapshots\//.test(p) && p.endsWith(".sql")) return "snapshot"
   if (/(^|\/)seeds\//.test(p) && p.endsWith(".csv")) return "seed"
   if (/(^|\/)tests\//.test(p) && p.endsWith(".sql")) return "test"
   if (/(^|\/)analyses\//.test(p)) return "analysis"

packages/opencode/src/altimate/review/format.ts

@@ -28,7 +28,12 @@ export function verdictHeadline(env: VerdictEnvelope): string {
     [critical && `${critical} critical`, warning && `${warning} warning`, suggestion && `${suggestion} suggestion`]
       .filter(Boolean)
       .join(", ") || "no findings"
-  return `${VERDICT_LABEL[env.verdict]} — ${counts} (${env.tier} tier)`
+  // tierClassified is optional in the schema — guard against externally-built
+  // envelopes that mark tierForced without threading the original classification.
+  const tierLabel = env.tierForced
+    ? `${env.tier} tier — forced (was ${env.tierClassified ?? "unknown"})`
+    : `${env.tier} tier`
+  return `${VERDICT_LABEL[env.verdict]} — ${counts} (${tierLabel})`
 }
 
 /** Full PR/MR summary comment body (markdown), prefixed with the dedup marker. */
@@ -43,6 +48,37 @@ export function renderSummary(env: VerdictEnvelope): string {
     )
   }
 
+  // G1 (Round 18) — surface classifier reasons when --explain-tier populated
+  // them, so a customer can see why the review ran at this tier. Also surfaces
+  // when --force-tier bypassed the classifier (tierReasons is auto-populated).
+  // Truncate the rendered summary on very large diffs: classifyPR appends one
+  // reason per file forcing FULL tier (e.g. every touched schema.yml or every
+  // PII column), which bloats the comment on wide PRs. The full list stays in
+  // the signed envelope's tierReasons[]; the summary shows the first 8.
+  if (env.tierReasons && env.tierReasons.length) {
+    const RENDER_CAP = 8
+    // Pick an inline-code-span fence longer than any backtick run inside `r`
+    // so a path like `packages/…/foo`bar`.sql` cannot terminate the span
+    // (cubic-review P3).
+    const shown = env.tierReasons
+      .slice(0, RENDER_CAP)
+      .map((r) => {
+        const runs = r.match(/`+/g)
+        const maxRun = runs ? Math.max(...runs.map((run) => run.length)) : 0
+        const fence = "`".repeat(maxRun + 1)
+        // If the reason itself starts/ends with a backtick, pad with a space so
+        // the leading/trailing backtick isn't glued to the fence.
+        const pad = /^`|`$/.test(r) ? " " : ""
+        return `${fence}${pad}${r}${pad}${fence}`
+      })
+      .join(", ")
+    const overflow =
+      env.tierReasons.length > RENDER_CAP
+        ? ` (+${env.tierReasons.length - RENDER_CAP} more in verdict envelope)`
+        : ""
+    lines.push(`> 🧭 **Tier: ${env.tier}** — ${shown}${overflow}`, "")
+  }
+
   if (!env.findings.length) {
     lines.push("No issues found in the changed dbt models. 🎉", "")
   } else {

packages/opencode/src/altimate/review/orchestrate.ts

@@ -190,6 +190,10 @@ export interface OrchestrateInput {
   /** PR metadata passed to the AI reviewer for intent checking. */
   prTitle?: string
   prBody?: string
+  /** G1 — attach the classifier's reason list to the envelope. */
+  explainTier?: boolean
+  /** G2 — override the classifier's tier decision. Envelope carries tierForced:true. */
+  forceTier?: "trivial" | "lite" | "full"
 }
 
 /** Derive the dbt model name from a model file path. */
@@ -1088,14 +1092,31 @@ export async function runReview(input: OrchestrateInput): Promise<VerdictEnvelop
   // A run is degraded when model files exist but none resolved against a manifest.
   const runDegraded = modelFiles.length > 0 ? !anyManifest : reviewable.length === 0
 
-  const tier = classifyPR(reviewable, {
+  const tierResult = classifyPR(reviewable, {
     blastRadiusOf: (p) => {
       const c = ctxByPath.get(p)
       return c ? c.impact.directCount + c.impact.transitiveCount : 0
     },
     touchesPiiOf: (f) => (ctxByPath.get(f.path)?.pii.length ?? 0) > 0,
     isComplexOf: (f) => ctxByPath.get(f.path)?.complex ?? false,
-  }).tier
+  })
+  const classifiedTier = tierResult.tier
+  // G2 — --force-tier overrides the classifier. Envelope records both the
+  // forced tier and the original classification whenever the flag is passed
+  // (regardless of whether the forced value happens to match the classifier),
+  // so audits can see the bypass every time the flag was used. The reasons
+  // list gets a leading "forced" marker so downstream doesn't confuse the
+  // forced tier for a natural one. Codex-R18-review fix — the earlier
+  // `!== classifiedTier` gate silently hid the bypass when the forced tier
+  // matched the classifier's result.
+  const tier = input.forceTier ?? classifiedTier
+  const tierForced = input.forceTier !== undefined
+  const tierReasons = tierForced
+    ? [
+        `forced via --force-tier=${input.forceTier} (classifier said ${classifiedTier})`,
+        ...tierResult.reasons,
+      ]
+    : tierResult.reasons
 
   const lanes = new Set(input.config.reviewers.length ? input.config.reviewers : TIER_LANES[tier])
 
@@ -1196,8 +1217,38 @@ export async function runReview(input: OrchestrateInput): Promise<VerdictEnvelop
 
   // schema.yml-level detectors (test removal) — run on changed YAML files
   // regardless of tier, since deleting a guardrail test is always worth flagging.
-  for (const file of reviewable) {
-    if (file.kind === "schema_yml") all.push(detectSchemaYmlPatterns(file, input.rubric))
+  // Pass old + new content when available so the detector diffs structural YAML
+  // (per-(entity, column, test) tuples) instead of walking the unified diff
+  // (which loses context when the model header is outside the -U3 window and
+  // silently drops sibling-column edge cases). Falls back to diff-only line
+  // detection when a content resolver isn't wired (e.g. unit-test callers).
+  //
+  // `oldContent` is fetched for anything that has an old side — MODIFIED,
+  // RENAMED, and DELETED. A schema.yml being renamed (e.g. moved to a new
+  // subdir) that also drops a `unique`/`not_null` guardrail must still surface
+  // as a finding; the earlier `status === "modified"` gate silently skipped
+  // renames. A whole schema.yml being DELETED removes every test declared in
+  // it — an even bigger removal — so the detector runs against `oldContent`
+  // vs an empty new document (cubic-review P2).
+  //
+  // `newContent` fetch is skipped for deleted files (nothing at HEAD to read;
+  // git-show would fail).
+  //
+  // Fetches run in parallel across schema files (a schema-heavy PR could touch
+  // dozens of yml files; serial `git show` per file adds up).
+  const schemaFiles = reviewable.filter((f) => f.kind === "schema_yml")
+  if (schemaFiles.length) {
+    const schemaFindingSets = await Promise.all(
+      schemaFiles.map(async (file) => {
+        const oldRef = file.oldPath ?? file.path
+        const [oldContent, newContent] = await Promise.all([
+          file.status !== "added" ? getContent?.(oldRef, "old") : Promise.resolve(undefined),
+          file.status !== "deleted" ? getContent?.(file.path, "new") : Promise.resolve(undefined),
+        ])
+        return detectSchemaYmlPatterns(file, input.rubric, { oldContent, newContent })
+      }),
+    )
+    for (const findings of schemaFindingSets) all.push(findings)
   }
 
   // Architectural dedup: the regex `dbt-patterns`/`rule-catalog` layer is a
@@ -1349,6 +1400,9 @@ export async function runReview(input: OrchestrateInput): Promise<VerdictEnvelop
     manifestHash: input.manifestHash,
     generatedAt: input.generatedAt,
     degraded,
+    tierReasons: input.explainTier || tierForced ? tierReasons : undefined,
+    tierForced: tierForced ? true : undefined,
+    tierClassified: tierForced ? classifiedTier : undefined,
   })
   return signEnvelope(envelope)
 }

packages/opencode/src/altimate/review/run.ts

@@ -1,5 +1,5 @@
 import path from "node:path"
-import { readFile } from "node:fs/promises"
+import { readFile, stat, access } from "node:fs/promises"
 import { loadReviewConfig, resolveRubric } from "./config"
 import type { Severity } from "./finding"
 import { collectChangedFiles, makeContentResolver, defaultBaseRef, manifestHash } from "./git"
@@ -42,6 +42,11 @@ export interface ReviewPullRequestOptions {
   /** PR metadata for the AI reviewer's intent check. */
   prTitle?: string
   prBody?: string
+  /** G1 — surface the tier classifier's reasons on the verdict envelope. */
+  explainTier?: boolean
+  /** G2 — [EXPERIMENTAL] bypass the tier classifier with the supplied tier.
+   *  Envelope carries `tierForced: true` when used. Debug / bench only. */
+  forceTier?: "trivial" | "lite" | "full"
 }
 
 /** dbt adapter_type → core SQL dialect. Mostly identity; a few aliases. */
@@ -73,6 +78,85 @@ async function detectDialect(manifestAbs: string): Promise<string | undefined> {
   }
 }
 
+/**
+ * G3 (Round 18) — walk upward from `cwd` looking for the nearest
+ * `dbt_project.yml`; if found, return the adjacent `target/manifest.json`
+ * (when it exists). Anchored on `dbt_project.yml` so we never pick up a
+ * `target/` from an unrelated tool (e.g. an Airflow project) that happens
+ * to live above us on the filesystem.
+ *
+ * We DO NOT re-run `dbt compile` here — that could touch a real warehouse or
+ * consume credentials. The customer is still responsible for compiling; we
+ * just find the existing artifact when they didn't pass `--manifest`.
+ *
+ * Returns undefined when no dbt project is found upward, or when the found
+ * project has no compiled manifest.
+ */
+async function autoDiscoverManifest(cwd: string): Promise<{ path: string; projectRoot: string } | undefined> {
+  let dir = path.resolve(cwd)
+  const root = path.parse(dir).root
+  // Walk up looking for dbt_project.yml so the manifest we auto-discover is
+  // demonstrably tied to a dbt project (never `target/manifest.json` from an
+  // unrelated cwd, e.g. Airflow's `target/`).
+  while (true) {
+    for (const fn of ["dbt_project.yml", "dbt_project.yaml"]) {
+      try {
+        await access(path.join(dir, fn))
+        const candidate = path.join(dir, "target", "manifest.json")
+        try {
+          await access(candidate)
+          return { path: candidate, projectRoot: dir }
+        } catch {
+          return undefined // dbt project found but not compiled — respect that
+        }
+      } catch {
+        /* keep walking */
+      }
+    }
+    if (dir === root) return undefined
+    dir = path.dirname(dir)
+  }
+}
+
+/** Warn to stderr when the manifest looks stale relative to changed files.
+ *  Only considers dbt-relevant files (SQL, YAML, Python models, seed CSV,
+ *  docs markdown) — changes to `README.md` at repo root, `.github/`,
+ *  `package.json`, etc. don't affect whether the compiled manifest is still
+ *  valid, so their mtimes shouldn't trigger a stale warning. */
+/** Exported for tests — see review-run-stale.test.ts. */
+export function isManifestAffecting(rel: string): boolean {
+  // dbt source directories — code (sql/py), schema (yml), seeds (csv), and
+  // docs blocks (md files under models/ / snapshots/ / analyses/ / etc.).
+  if (/(^|\/)(models|seeds|snapshots|macros|tests|analyses)\/.*\.(sql|py|yml|yaml|csv|md)$/i.test(rel)) return true
+  // Top-level dbt project config files.
+  if (/(^|\/)(dbt_project|packages|profiles|dependencies)\.ya?ml$/i.test(rel)) return true
+  return false
+}
+
+async function warnIfStale(manifestAbs: string, changedPaths: string[], cwd: string): Promise<void> {
+  try {
+    const manifestMtime = (await stat(manifestAbs)).mtimeMs
+    for (const rel of changedPaths) {
+      if (!isManifestAffecting(rel)) continue
+      const abs = path.isAbsolute(rel) ? rel : path.join(cwd, rel)
+      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. ` +
+              `Re-run \`dbt compile\` (or \`dbt build\`) to refresh before reviewing.\n`,
+          )
+          return
+        }
+      } catch {
+        /* file not on disk (e.g. removed by the change) — skip */
+      }
+    }
+  } catch {
+    /* manifest unreadable → detectDialect will have already returned undefined */
+  }
+}
+
 export async function reviewPullRequest(opts: ReviewPullRequestOptions): Promise<VerdictEnvelope> {
   const config = await loadReviewConfig(opts.cwd)
   if (opts.manifestPath) config.manifestPath = opts.manifestPath
@@ -97,9 +181,44 @@ export async function reviewPullRequest(opts: ReviewPullRequestOptions): Promise
   // Resolve the manifest against the PROJECT being reviewed (cwd), not the
   // binary's process.cwd() — otherwise a relative path silently misses when the
   // CLI is invoked from elsewhere, degrading every review to lint-only.
-  const manifestAbs = path.isAbsolute(config.manifestPath)
+  let manifestAbs = path.isAbsolute(config.manifestPath)
     ? config.manifestPath
     : path.join(opts.cwd, config.manifestPath)
+  // The "effective dbt project root" for downstream lookups (dbtProjectName,
+  // compiled-SQL resolver). Starts as opts.cwd — G3 auto-discovery updates it
+  // when it finds a dbt_project.yml in an ancestor directory, so the compiled
+  // resolver and project-name lookups target the discovered project rather
+  // than the CLI's working directory (which may be a subdir).
+  let dbtRoot = opts.cwd
+  // G3 (Round 18) — auto-discovery when the caller didn't pass `--manifest` and
+  // the config-relative resolve above doesn't point at an existing file. Walk
+  // upward for the dbt project root and use its `target/manifest.json`. Explicit
+  // `--manifest` always wins (see run.ts opts.manifestPath override at the top).
+  if (!opts.manifestPath) {
+    let manifestExists = false
+    try {
+      await access(manifestAbs)
+      manifestExists = true
+    } catch {
+      /* fall through to auto-discovery */
+    }
+    if (!manifestExists) {
+      const discovered = await autoDiscoverManifest(opts.cwd)
+      if (discovered) {
+        manifestAbs = discovered.path
+        dbtRoot = discovered.projectRoot
+        process.stderr.write(
+          `ℹ️  auto-discovered dbt manifest at ${discovered.path} ` +
+            `(dbt project root: ${discovered.projectRoot}). Pass --manifest to override.\n`,
+        )
+      }
+    }
+  }
+  // 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), opts.cwd)
 
   // Resolve the SQL dialect: explicit config wins; otherwise auto-detect from
   // the dbt manifest's `adapter_type` (so a BigQuery/Redshift project isn't
@@ -117,8 +236,11 @@ export async function reviewPullRequest(opts: ReviewPullRequestOptions): Promise
 
   // Prefer dbt's COMPILED SQL (target/compiled) for the engine lanes — the clean
   // approach (Datafold/Recce/Fusion all render-then-analyze rather than parse Jinja).
-  const projectName = await dbtProjectName(opts.cwd)
-  const getCompiled = opts.getContent ? undefined : makeCompiledResolver({ cwd: opts.cwd, projectName })
+  // Use `dbtRoot` (the discovered project root when G3 auto-discovery fired,
+  // else opts.cwd) so a subdir invocation still finds `target/compiled/…`
+  // next to the discovered manifest instead of falling back to raw Jinja.
+  const projectName = await dbtProjectName(dbtRoot)
+  const getCompiled = opts.getContent ? undefined : makeCompiledResolver({ cwd: dbtRoot, projectName })
 
   return runReview({
     changedFiles,
@@ -135,5 +257,7 @@ export async function reviewPullRequest(opts: ReviewPullRequestOptions): Promise
     aiReview: opts.noAi || config.ai === false ? undefined : runAiReview,
     prTitle: opts.prTitle,
     prBody: opts.prBody,
+    explainTier: opts.explainTier,
+    forceTier: opts.forceTier,
   })
 }

packages/opencode/src/altimate/review/verdict.ts

@@ -92,6 +92,13 @@ export const VerdictEnvelope = z.object({
   idealVerdict: Verdict,
   mode: ReviewMode,
   tier: RiskTier,
+  /** G1 — reasons the classifier assigned this tier (only when --explain-tier). */
+  tierReasons: z.array(z.string()).optional(),
+  /** G2 — true when --force-tier bypassed the classifier. Included in signature so
+   *  a tampered envelope claiming natural tier can't fake a forced run. */
+  tierForced: z.boolean().optional(),
+  /** The classifier's original tier before --force-tier overrode it (G2). */
+  tierClassified: RiskTier.optional(),
   findings: z.array(Finding),
   summary: z.object({
     critical: z.number().int().nonnegative(),
@@ -127,6 +134,12 @@ export interface BuildEnvelopeInput {
   manifestHash?: string
   generatedAt?: string
   degraded?: boolean
+  /** G1 — classifier reasons for the tier (only surfaced when explainTier=true). */
+  tierReasons?: string[]
+  /** G2 — set when --force-tier was applied. */
+  tierForced?: boolean
+  /** G2 — classifier's original tier before the force override. */
+  tierClassified?: RiskTier
 }
 
 function summarize(findings: Finding[], degraded: boolean): VerdictEnvelope["summary"] {
@@ -147,6 +160,9 @@ export function buildEnvelope(input: BuildEnvelopeInput): VerdictEnvelope {
     idealVerdict: ideal,
     mode: input.mode,
     tier: input.tier,
+    tierReasons: input.tierReasons,
+    tierForced: input.tierForced,
+    tierClassified: input.tierClassified,
     findings: input.findings,
     summary: summarize(input.findings, degraded),
     engine: EngineVersions.parse(input.engine ?? {}),