Skip to content

feat: verify-and-escalate shadow capture at the edit seam (phase 1, gated off)#921

Open
anandgupta42 wants to merge 9 commits into
mainfrom
feat/verifier-shadow-capture
Open

feat: verify-and-escalate shadow capture at the edit seam (phase 1, gated off)#921
anandgupta42 wants to merge 9 commits into
mainfrom
feat/verifier-shadow-capture

Conversation

@anandgupta42

@anandgupta42 anandgupta42 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Phase 1: verify-and-escalate shadow capture at the edit seam

First step of wiring the altimate-router verifier platform (verify-and-escalate: run cheap → prove behaviour-equivalence with a sound oracle → escalate the misses) into altimate-code.

Why: the verifier is measured at φ=0 with ~40–50% cost savings on synthetic workloads, and is sound on real edits (0 false-ships), but a real at-scale φ/savings number needs real before/after edits with full file context — which only exist at the edit tool (transcript snippets don't reconstruct into runnable units; see altimate-router/eval/docs/findings/dogfood-substrate-analysis.md).

What this does (SHADOW only):

  • altimate/verify-shadow.ts (new): VerifyShadow.captureEdit appends {ts, file, language, before, after, old/new string} to a local JSONL. OFF unless ALTIMATE_VERIFY_SHADOW is set; fail-safe (never throws — capture must never break an edit).
  • tool/edit.ts: one marker-wrapped, fail-safe capture call after a successful edit (+ the import). No behaviour change, no escalation/acting.

An offline pass in altimate-router (eval/phi) reconstructs runnable units from the captured full-file edits and runs the verifier → the first real-traffic decision distribution + false-ship φ.

Phase 2 (separate PR, later): call the verifier live (verify-daemon / napi addon) and act on the Decision, behind the same flag, once shadow data confirms φ=0 on real traffic.

Verified: upstream marker check clean; bun run typecheck green (0 errors); runtime smoke — OFF writes nothing, ON captures before/after with language detected.

🤖 Generated with Claude Code


Summary by cubic

Adds shadow edit capture for the verifier (phase 1). It’s fully gated and never changes edit behavior; captured before/after edits are written to a local JSONL for offline analysis and φ measurement.

  • New Features

    • Shadow capture: new VerifyShadow.captureEdit writes {ts,file,language,before,after,old/new} to ~/.altimate-router/edit-capture.jsonl. Off by default; enable with ALTIMATE_VERIFY_SHADOW. Hooked once post-edit in tool/edit.ts and never throws.
    • Reviewer improvements: deterministic join-key regressions mapped to critical join_risk; dialect now threaded into core equivalence; diff-scoped PII classification tightened (e.g., low-confidence Name/Address suppressed) and fallback PII rules suppressed when core runs; schema YAML catalog rules run for test/metadata weakening; added DuckDB data-diff e2e via @altimateai/drivers/duckdb; docs clarify producing target/catalog.json for proof-grade equivalence.
  • Bug Fixes

    • DuckDB connector: avoid passing undefined as open options; uses 1-arg constructor when no options are needed to prevent callback hangs in Bun, with lock detection retained.
    • YAML classification only treats dbt resource paths/properties files as schema_yml (e.g., GitHub workflows no longer misclassified).
    • CLI: --ai is now on by default; use --no-ai to disable the advisory lane.

Written for commit 7deabd6. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

Release Notes

  • New Features

    • Added dialect-aware SQL equivalence checking for more accurate query comparisons
    • Enhanced PII classification with layer-based risk exposure detection
    • Improved YAML file classification to prevent false positives
    • Updated CLI flag from --no-ai to --ai for clearer advisory reviewer control
  • Bug Fixes

    • Fixed join-key regression detection to surface as critical findings
    • Eliminated duplicate PII exposure reporting in dbt pattern detection
    • Improved async timeout handling in database connections
  • Tests

    • Added comprehensive end-to-end and integration test coverage
  • Documentation

    • Updated dbt PR review guidance and added demo scenario corpus reference

anandgupta42 and others added 9 commits June 8, 2026 12:01
…ated off)

First step of integrating the altimate-router verifier platform (verify-and-escalate) into
altimate-code. To measure the verifier's real false-ship φ + savings on LIVE traffic we need
real before/after edits WITH full file context (better than mining transcript snippets).

- altimate/verify-shadow.ts (new): VerifyShadow.captureEdit appends {ts, file, language,
  before, after, old/new string} to a local JSONL. OFF unless ALTIMATE_VERIFY_SHADOW is set;
  fail-safe (never throws — capture must never break an edit). An offline pass in
  altimate-router (eval/phi) reconstructs runnable units and runs the verifier.
- tool/edit.ts: one marker-wrapped, fail-safe capture call after a successful edit + the
  marker-wrapped import. SHADOW ONLY — no behaviour change, no escalation/acting yet.

Phase 2 (later): call the verifier live (verify-daemon / napi addon) and act on the Decision,
gated behind the same flag, after shadow data confirms φ=0 on real traffic.

Verified: upstream marker check clean; `bun run typecheck` green (0 errors); runtime smoke —
OFF writes nothing, ON captures before/after with language detected.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR enhances the dbt PR review engine with deterministic quality improvements and test infrastructure. It adds dialect-aware equivalence checking, tightens YAML classification to avoid false positives, refactors schema pattern detection to run catalog rules, improves structural and PII finding severity/deduplication, hardens DuckDB connection handling, normalizes CLI flag defaults, introduces edit capture for verification, and provides comprehensive test coverage across all changes.

Changes

Deterministic Review Improvements

Layer / File(s) Summary
Internal Documentation: Self-Improvement Loop and Scenario Corpus
docs/docs/usage/dbt-pr-review.md, docs/internal/2026-06-08-dbt-pr-review-self-improvement-loop.md, docs/internal/dbt-pr-review-demo-scenario-corpus.md
Documents the dbt PR review improvement loop with quantified goals, baseline metrics, backlog items (join-key regression, demo CI artifacts, PII precision), warehouse e2e policy, and 50-scenario corpus with expected verdicts and deterministic evidence sources.
DuckDB Connection Retry and Timeout Handling
packages/drivers/src/duckdb.ts
Refactored async callback/timeout logic with mutable instance, resolved flag to guard late invocations, access_mode option wiring, and lock-message detection mapping to DUCKDB_LOCKED error.
Equivalence Checking with Dialect Support
packages/opencode/src/altimate/native/types.ts, packages/opencode/src/altimate/native/altimate-core.ts, packages/opencode/src/altimate/review/runner.ts
Extends equivalence interface to accept optional dialect?: string parameter and forwards it through dispatcher handler to core.checkEquivalence for dialect-aware equivalence checking.
YAML File Classification Tightening
packages/opencode/src/altimate/review/diff-filter.ts
Introduces isYaml flag gating YAML classification; routes YAML under models//snapshots//seeds//tests/ as schema_yml and applies regex matching on schema-like filenames to avoid misclassifying non-dbt YAML.
Schema Pattern Detection with Catalog Rule Integration
packages/opencode/src/altimate/review/dbt-patterns.ts
Refactors detectSchemaYmlPatterns to accumulate findings in an out array, conditionally emit test_coverage:removed-tests only when tests are genuinely removed, and unconditionally append evaluateCatalog results for schema.yml diffs without early returns.
Structural and PII Finding Quality
packages/opencode/src/altimate/review/orchestrate.ts
Maps join_key_regression structural rule to join_risk category; escalates engine error findings to critical severity; re-tiers diff-scoped PII with marts/reporting exposure signals, low-risk classification set, confidence-based thresholds, and dynamic severity; conditions piiLane on both classifyPii and columnLineage; suppresses duplicate PII findings on diff-scoped files.
CLI Flag Normalization
packages/opencode/src/cli/cmd/review.ts
Replaces --no-ai option (default disabled) with --ai boolean flag (default enabled) to normalize flag semantics while preserving handler logic that derives noAi: args.ai === false.
Shadow Capture Infrastructure
packages/opencode/src/altimate/verify-shadow.ts, packages/opencode/src/tool/edit.ts
Introduces VerifyShadow.captureEdit namespace function to record edit context (timestamp, file, language, before/after content) to JSONL when ALTIMATE_VERIFY_SHADOW is set; integrates into EditTool post-contentNew with fail-safe error handling.
Comprehensive Integration Tests
packages/opencode/test/altimate/data-diff-duckdb-e2e.test.ts, packages/opencode/test/altimate/review-ci.test.ts, packages/opencode/test/altimate/review-dbt-patterns.test.ts, packages/opencode/test/altimate/review-runner.test.ts, packages/opencode/test/altimate/review.test.ts
Adds dialect forwarding verification, YAML classification edge cases (workflow YAML ignored, catalog rules on schema metadata), join-key regression (SC010) critical finding with suppression for CTE/bridge joins, diff-scoped PII blocking (mart email) and non-blocking (low-confidence name), DuckDB e2e with in-memory joindiff, and CLI flag parsing tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • AltimateAI/altimate-code#856: Builds on the dbt-pr-review foundation from this PR, extending the same implementation surface in review orchestration, runner, and CLI flag wiring.

Suggested labels

contributor, needs-review:blocked, needs:compliance

Suggested reviewers

  • sahrizvi

Poem

🐰 With dialect-aware checks and catalog rules so bright,
Schema patterns now flow through the diff with might,
Structural risks escalate, PII exposure glows,
Shadow captures whisper where the edited code flows,
Tests now guard the path where deterministic verdicts grow!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is comprehensive, explaining the why (verifier needs real edits), what (shadow capture disabled by default), and how (JSONL recording with fail-safe behavior). However, it does NOT include the required PINEAPPLE keyword at the top despite being AI-generated (marked as 'Generated with Claude Code'). Add the word 'PINEAPPLE' at the very top of the PR description before any other content, as required for AI-generated contributions per the template.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: shadow capture infrastructure for edit verification, gated behind a flag, with clear indication this is phase 1.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/verifier-shadow-capture

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/drivers/src/duckdb.ts (1)

83-87: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Close the DB instance on timeout to prevent lock/resource leaks.

At Line 83-87, timeout rejection does not close instance. In the Bun scenario noted by your own comment (open callback never fires), that leaves the native handle open and can keep the file locked in-process.

Suggested fix
       const tryConnect = (accessMode?: string): Promise<any> =>
         new Promise<any>((resolve, reject) => {
           let resolved = false
           let timeout: ReturnType<typeof setTimeout> | undefined
           let instance: any
+          const safeClose = () => {
+            try {
+              if (instance && typeof instance.close === "function") {
+                instance.close(() => {})
+              }
+            } catch {
+              // fail-safe cleanup
+            }
+          }
           const opts = accessMode ? { access_mode: accessMode } : undefined
           const onOpen = (err: Error | null) => {
             if (resolved) {
-              if (instance && typeof instance.close === "function") instance.close()
+              safeClose()
               return
             }
             resolved = true
             if (timeout) clearTimeout(timeout)
             if (err) {
               const msg = err.message || String(err)
               if (msg.toLowerCase().includes("locked") || msg.includes("SQLITE_BUSY") || msg.includes("DUCKDB_LOCKED")) {
                 reject(new Error("DUCKDB_LOCKED"))
               } else {
                 reject(err)
               }
             } else {
               resolve(instance)
             }
           }
           instance = opts
             ? new duckdb.Database(dbPath, opts, onOpen)
             : new duckdb.Database(dbPath, onOpen)
           // Bun: native callback may not fire; fall back after 2s
           timeout = setTimeout(() => {
             if (!resolved) {
               resolved = true
+              safeClose()
               reject(new Error(`Timed out opening DuckDB database "${dbPath}"`))
             }
           }, 2000)
         })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/drivers/src/duckdb.ts` around lines 83 - 87, The timeout rejection
path in the open callback does not close the DuckDB native handle `instance`,
which can leave the DB file locked; update the timeout branch to call
`instance.close()` (or the appropriate close/cleanup method on the DuckDB client
object) before rejecting, guarding that `instance` is defined and
swallowing/logging any error from close to avoid throwing from the timeout
handler; ensure this change is made alongside the existing `resolved` check in
the same asynchronous open routine so the native handle is always released if
the open callback never fires.
packages/opencode/src/tool/edit.ts (1)

63-87: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Move shadow capture outside the lock and run it for all successful edits.

Line 86 returns early for oldString === "", so those edits are never captured. Also, Lines 126-134 execute capture while FileTime.withLock is held, increasing lock contention for non-critical shadow work.

Suggested restructuring
-      // altimate_change start — verify shadow capture (gated off by default; fail-safe, no behavior change)
-      VerifyShadow.captureEdit({
-        file: filePath,
-        before: contentOld,
-        after: contentNew,
-        oldString: params.oldString,
-        newString: params.newString,
-      })
-      // altimate_change end
       FileTime.read(ctx.sessionID, filePath)
     })
+
+    VerifyShadow.captureEdit({
+      file: filePath,
+      before: contentOld,
+      after: contentNew,
+      oldString: params.oldString,
+      newString: params.newString,
+    })

Also applies to: 126-134

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/src/tool/edit.ts` around lines 63 - 87, The shadow-capture
logic is currently executed inside FileTime.withLock and skipped for the
early-return branch when params.oldString === "" (and also increases lock
duration for the other path); refactor edit handling so that FileTime.withLock
only wraps the minimal critical section (checks, ctx.ask permission,
Filesystem.write and Bus.publish calls that must be atomic) and move the shadow
capture call(s) (the FileTime.read / shadow capture logic currently at lines
~126-134) outside and after the withLock block so they run for all successful
edits (both the early-return case and the normal path); ensure you still call
FileTime.read(ctx.sessionID, filePath) once after successful write/publishes and
after releasing the lock, and keep references to FileTime.withLock, ctx.ask,
Filesystem.write, Bus.publish and FileWatcher.Event to locate the spots to
change.
🧹 Nitpick comments (1)
packages/opencode/src/altimate/verify-shadow.ts (1)

13-66: 🏗️ Heavy lift

Refactor this module to Effect services/tracing conventions.

This new service bypasses the project’s Effect patterns (Effect.fn/Effect.gen) and direct-effect service access, which will make tracing/composition inconsistent with surrounding Effectified services.

As per coding guidelines, packages/opencode/**/*.{ts,tsx} should “Use Effect.gen(function* () { ... })”, “Use Effect.fn("Domain.method")”, and “Prefer FileSystem.FileSystem instead of raw fs/promises for effectful file I/O`.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/src/altimate/verify-shadow.ts` around lines 13 - 66, The
module should be converted into an Effect-style service: replace the top-level
imperative captureEdit function with an Effect.fn/Effect.gen-backed service
method and stop using synchronous node fs calls; implement VerifyShadow as an
Effect service that exposes captureEdit (e.g., VerifyShadow.captureEdit)
implemented with Effect.fn("VerifyShadow.captureEdit") or Effect.gen(function*
() { ... }), use the project's FileSystem.FileSystem abstractions for
mkdir/append (instead of mkdirSync/appendFileSync) and use the effectful way to
read env/homedir or a small Env config service (replace logPath with an
effectful resolver), keep EXT_LANG and the record shape but construct and
persist the JSONL inside the Effect while catching/logging errors via the
existing Log service inside the effect so the operation never throws.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/opencode/src/altimate/verify-shadow.ts`:
- Around line 59-61: The shadow log is being created with default permissions
and may be world-readable; update the log creation to enforce owner-only access
by making the directory with mode 0o700 (replace mkdirSync(dirname(path), ...)
to include { recursive: true, mode: 0o700 }) and write the file with owner-only
mode 0o600 instead of relying on default umask — e.g., open the path with
fs.openSync(path, "a", 0o600) then write the JSON + "\n" via fs.writeSync and
fs.closeSync (or call appendFileSync with an explicit { mode: 0o600 } option) so
logPath(), mkdirSync and appendFileSync usage is hardened to owner-only
permissions.

---

Outside diff comments:
In `@packages/drivers/src/duckdb.ts`:
- Around line 83-87: The timeout rejection path in the open callback does not
close the DuckDB native handle `instance`, which can leave the DB file locked;
update the timeout branch to call `instance.close()` (or the appropriate
close/cleanup method on the DuckDB client object) before rejecting, guarding
that `instance` is defined and swallowing/logging any error from close to avoid
throwing from the timeout handler; ensure this change is made alongside the
existing `resolved` check in the same asynchronous open routine so the native
handle is always released if the open callback never fires.

In `@packages/opencode/src/tool/edit.ts`:
- Around line 63-87: The shadow-capture logic is currently executed inside
FileTime.withLock and skipped for the early-return branch when params.oldString
=== "" (and also increases lock duration for the other path); refactor edit
handling so that FileTime.withLock only wraps the minimal critical section
(checks, ctx.ask permission, Filesystem.write and Bus.publish calls that must be
atomic) and move the shadow capture call(s) (the FileTime.read / shadow capture
logic currently at lines ~126-134) outside and after the withLock block so they
run for all successful edits (both the early-return case and the normal path);
ensure you still call FileTime.read(ctx.sessionID, filePath) once after
successful write/publishes and after releasing the lock, and keep references to
FileTime.withLock, ctx.ask, Filesystem.write, Bus.publish and FileWatcher.Event
to locate the spots to change.

---

Nitpick comments:
In `@packages/opencode/src/altimate/verify-shadow.ts`:
- Around line 13-66: The module should be converted into an Effect-style
service: replace the top-level imperative captureEdit function with an
Effect.fn/Effect.gen-backed service method and stop using synchronous node fs
calls; implement VerifyShadow as an Effect service that exposes captureEdit
(e.g., VerifyShadow.captureEdit) implemented with
Effect.fn("VerifyShadow.captureEdit") or Effect.gen(function* () { ... }), use
the project's FileSystem.FileSystem abstractions for mkdir/append (instead of
mkdirSync/appendFileSync) and use the effectful way to read env/homedir or a
small Env config service (replace logPath with an effectful resolver), keep
EXT_LANG and the record shape but construct and persist the JSONL inside the
Effect while catching/logging errors via the existing Log service inside the
effect so the operation never throws.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a6a4f458-2d3f-4a48-8685-a19934e1bbdf

📥 Commits

Reviewing files that changed from the base of the PR and between c2019ba and 7deabd6.

📒 Files selected for processing (18)
  • docs/docs/usage/dbt-pr-review.md
  • docs/internal/2026-06-08-dbt-pr-review-self-improvement-loop.md
  • docs/internal/dbt-pr-review-demo-scenario-corpus.md
  • packages/drivers/src/duckdb.ts
  • packages/opencode/src/altimate/native/altimate-core.ts
  • packages/opencode/src/altimate/native/types.ts
  • packages/opencode/src/altimate/review/dbt-patterns.ts
  • packages/opencode/src/altimate/review/diff-filter.ts
  • packages/opencode/src/altimate/review/orchestrate.ts
  • packages/opencode/src/altimate/review/runner.ts
  • packages/opencode/src/altimate/verify-shadow.ts
  • packages/opencode/src/cli/cmd/review.ts
  • packages/opencode/src/tool/edit.ts
  • packages/opencode/test/altimate/data-diff-duckdb-e2e.test.ts
  • packages/opencode/test/altimate/review-ci.test.ts
  • packages/opencode/test/altimate/review-dbt-patterns.test.ts
  • packages/opencode/test/altimate/review-runner.test.ts
  • packages/opencode/test/altimate/review.test.ts

Comment on lines +59 to +61
const path = logPath()
mkdirSync(dirname(path), { recursive: true })
appendFileSync(path, JSON.stringify(record) + "\n")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Create the shadow log with owner-only permissions.

Line 61 appends full-file before/after payloads, but default create mode can leave this JSONL readable by other local users (umask-dependent). That’s a sensitive-data exposure risk.

Suggested hardening
-      appendFileSync(path, JSON.stringify(record) + "\n")
+      appendFileSync(path, JSON.stringify(record) + "\n", { mode: 0o600 })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const path = logPath()
mkdirSync(dirname(path), { recursive: true })
appendFileSync(path, JSON.stringify(record) + "\n")
const path = logPath()
mkdirSync(dirname(path), { recursive: true })
appendFileSync(path, JSON.stringify(record) + "\n", { mode: 0o600 })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/src/altimate/verify-shadow.ts` around lines 59 - 61, The
shadow log is being created with default permissions and may be world-readable;
update the log creation to enforce owner-only access by making the directory
with mode 0o700 (replace mkdirSync(dirname(path), ...) to include { recursive:
true, mode: 0o700 }) and write the file with owner-only mode 0o600 instead of
relying on default umask — e.g., open the path with fs.openSync(path, "a",
0o600) then write the JSON + "\n" via fs.writeSync and fs.closeSync (or call
appendFileSync with an explicit { mode: 0o600 } option) so logPath(), mkdirSync
and appendFileSync usage is hardened to owner-only permissions.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 issues found across 18 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/internal/dbt-pr-review-demo-scenario-corpus.md">

<violation number="1" location="docs/internal/dbt-pr-review-demo-scenario-corpus.md:48">
P3: Hardcoded developer-specific absolute paths make the validation command non-portable for other contributors and CI.</violation>
</file>

<file name="packages/opencode/src/tool/edit.ts">

<violation number="1" location="packages/opencode/src/tool/edit.ts:127">
P2: Shadow capture is branch-limited; successful `oldString === ""` edits skip capture entirely. This undercounts edit traffic and can skew verifier φ/savings measurement.</violation>
</file>

<file name="packages/opencode/src/altimate/verify-shadow.ts">

<violation number="1" location="packages/opencode/src/altimate/verify-shadow.ts:61">
P2: Synchronous disk writes in `captureEdit` block the edit path when shadow mode is enabled. Use async fs writes to keep edit latency non-blocking.</violation>
</file>

<file name="packages/opencode/src/altimate/review/dbt-patterns.ts">

<violation number="1" location="packages/opencode/src/altimate/review/dbt-patterns.ts:822">
P2: This adds duplicate reporting for `relationships` test removals in schema.yml. Duplicate findings inflate noise and can over-weight a single issue in review output.</violation>
</file>

<file name="packages/opencode/src/altimate/review/orchestrate.ts">

<violation number="1" location="packages/opencode/src/altimate/review/orchestrate.ts:1081">
P1: PII fallback is effectively disabled whenever classifier APIs exist, causing silent misses when diff-scoped classification returns empty.</violation>

<violation number="2" location="packages/opencode/src/altimate/review/orchestrate.ts:1181">
P2: Regex PII findings are suppressed based on capability presence, not actual classifier coverage, creating false negatives.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

if (lanes.has("test_coverage")) tasks.push(missingGrainTestLane(ctx, input.runner))
if (lanes.has("pii_exposure")) all.push(piiLane(ctx.file, ctx.pii))
const hasDiffScopedPii = !!input.runner.classifyPii && !!input.runner.columnLineage
if (lanes.has("pii_exposure") && !hasDiffScopedPii) all.push(piiLane(ctx.file, ctx.pii))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: PII fallback is effectively disabled whenever classifier APIs exist, causing silent misses when diff-scoped classification returns empty.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/review/orchestrate.ts, line 1081:

<comment>PII fallback is effectively disabled whenever classifier APIs exist, causing silent misses when diff-scoped classification returns empty.</comment>

<file context>
@@ -1070,7 +1077,8 @@ export async function runReview(input: OrchestrateInput): Promise<VerdictEnvelop
     if (lanes.has("test_coverage")) tasks.push(missingGrainTestLane(ctx, input.runner))
-    if (lanes.has("pii_exposure")) all.push(piiLane(ctx.file, ctx.pii))
+    const hasDiffScopedPii = !!input.runner.classifyPii && !!input.runner.columnLineage
+    if (lanes.has("pii_exposure") && !hasDiffScopedPii) all.push(piiLane(ctx.file, ctx.pii))
     // PII classification always runs (cheap name-pattern check, diff-scoped to
     // newly-introduced columns) — exposing PII is a hard-floor concern that
</file context>

createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)),
)
// altimate_change start — verify shadow capture (gated off by default; fail-safe, no behavior change)
VerifyShadow.captureEdit({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Shadow capture is branch-limited; successful oldString === "" edits skip capture entirely. This undercounts edit traffic and can skew verifier φ/savings measurement.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tool/edit.ts, line 127:

<comment>Shadow capture is branch-limited; successful `oldString === ""` edits skip capture entirely. This undercounts edit traffic and can skew verifier φ/savings measurement.</comment>

<file context>
@@ -120,6 +123,15 @@ export const EditTool = Tool.define("edit", {
         createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)),
       )
+      // altimate_change start — verify shadow capture (gated off by default; fail-safe, no behavior change)
+      VerifyShadow.captureEdit({
+        file: filePath,
+        before: contentOld,
</file context>

}
const path = logPath()
mkdirSync(dirname(path), { recursive: true })
appendFileSync(path, JSON.stringify(record) + "\n")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Synchronous disk writes in captureEdit block the edit path when shadow mode is enabled. Use async fs writes to keep edit latency non-blocking.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/verify-shadow.ts, line 61:

<comment>Synchronous disk writes in `captureEdit` block the edit path when shadow mode is enabled. Use async fs writes to keep edit latency non-blocking.</comment>

<file context>
@@ -0,0 +1,66 @@
+      }
+      const path = logPath()
+      mkdirSync(dirname(path), { recursive: true })
+      appendFileSync(path, JSON.stringify(record) + "\n")
+    } catch (e) {
+      log.error("edit capture failed", { error: e instanceof Error ? e.message : String(e) })
</file context>

})
out.push(f)
}
out.push(...evaluateCatalog(file, "", added, removed, rubric))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This adds duplicate reporting for relationships test removals in schema.yml. Duplicate findings inflate noise and can over-weight a single issue in review output.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/review/dbt-patterns.ts, line 822:

<comment>This adds duplicate reporting for `relationships` test removals in schema.yml. Duplicate findings inflate noise and can over-weight a single issue in review output.</comment>

<file context>
@@ -794,29 +794,33 @@ export function detectSchemaYmlPatterns(file: ChangedFile, rubric: Rubric): Find
+    })
+    out.push(f)
+  }
+  out.push(...evaluateCatalog(file, "", added, removed, rubric))
+  return out
+    .map((f) => ({ ...f, severity: clampSeverity(f.category, f.severity, f.confidence) }))
</file context>
Suggested change
out.push(...evaluateCatalog(file, "", added, removed, rubric))
out.push(
...evaluateCatalog(file, "", added, removed, rubric).filter(
(x) => !(x.ruleKey === "test_coverage:relationship-test-removed-yml" && genuinelyRemoved.some((l) => /\brelationships\b/i.test(l))),
),
)

const fallbackRule = String((f.evidence?.result as any)?.rule ?? "").replace(/_/g, "-")
if (
(tool === "dbt-patterns" || tool === "rule-catalog") &&
diffScopedPiiFiles.has(f.file) &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Regex PII findings are suppressed based on capability presence, not actual classifier coverage, creating false negatives.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/review/orchestrate.ts, line 1181:

<comment>Regex PII findings are suppressed based on capability presence, not actual classifier coverage, creating false negatives.</comment>

<file context>
@@ -1160,13 +1174,22 @@ export async function runReview(input: OrchestrateInput): Promise<VerdictEnvelop
+    const fallbackRule = String((f.evidence?.result as any)?.rule ?? "").replace(/_/g, "-")
+    if (
+      (tool === "dbt-patterns" || tool === "rule-catalog") &&
+      diffScopedPiiFiles.has(f.file) &&
+      f.category === "pii_exposure" &&
+      (fallbackRule === "pii-into-mart" || fallbackRule === "select-pii-columns")
</file context>
Suggested change
diffScopedPiiFiles.has(f.file) &&
diffScopedPiiFiles.has(f.file) && flat.some((x) => x.file === f.file && x.evidence?.tool === "altimate_core.classify_pii") &&

Default focused validation command:

```bash
ALTIMATE_LOCAL_CORE=/Users/anandgupta/codebase/altimate-core-internal/crates/altimate-core-node \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Hardcoded developer-specific absolute paths make the validation command non-portable for other contributors and CI.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/internal/dbt-pr-review-demo-scenario-corpus.md, line 48:

<comment>Hardcoded developer-specific absolute paths make the validation command non-portable for other contributors and CI.</comment>

<file context>
@@ -0,0 +1,217 @@
+Default focused validation command:
+
+```bash
+ALTIMATE_LOCAL_CORE=/Users/anandgupta/codebase/altimate-core-internal/crates/altimate-core-node \
+  bun --conditions=browser /Users/anandgupta/codebase/altimate-code/packages/opencode/src/index.ts review \
+  --cwd /Users/anandgupta/codebase/altimate-code/demo/dbt-pr-review-demo \
</file context>

@dev-punia-altimate dev-punia-altimate left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multi-Persona Review — Verdict: skipped

Multi-persona review completed.

0/0 agents completed · 1s · 0 findings (0 critical, 0 high, 0 medium)


Multi-Persona Review · vllm:qwen3-next-80b (waves) + vllm-fallback (synth) ·

@dev-punia-altimate

Copy link
Copy Markdown
Contributor

❌ Tests — Failures Detected

TypeScript — 15 failure(s)

  • connection_refused [1.00ms]
  • timeout
  • permission_denied
  • parse_error
  • network_error
  • auth_failure
  • rate_limit [1.00ms]
  • internal_error
  • empty_error
  • connection_refused [1.00ms]
  • timeout
  • permission_denied
  • parse_error
  • network_error
  • auth_failure

Next Step

Please address the failing cases above and re-run verification.

cc @anandgupta42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants