Fix dbt YAML review file classification#920
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR narrows dbt YAML detection: ChangesYAML Classification Precision
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/test/altimate/data-diff-duckdb-e2e.test.ts`:
- Around line 10-16: The test unconditionally deletes
ALTIMATE_TELEMETRY_DISABLED in afterAll which clobbers any pre-existing
environment value; modify the setup/teardown to capture the prior value in
beforeAll (e.g., const prevTelemetry = process.env.ALTIMATE_TELEMETRY_DISABLED)
then set process.env.ALTIMATE_TELEMETRY_DISABLED = "true", and in afterAll
restore that saved value (if prevTelemetry is undefined delete the env var,
otherwise set it back to prevTelemetry) before calling Registry.reset(); update
references in the beforeAll/afterAll blocks accordingly.
🪄 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: e73537bc-66fa-47be-ae98-c2425126aaba
📒 Files selected for processing (16)
docs/docs/usage/dbt-pr-review.mddocs/internal/2026-06-08-dbt-pr-review-self-improvement-loop.mddocs/internal/dbt-pr-review-demo-scenario-corpus.mdpackages/drivers/src/duckdb.tspackages/opencode/src/altimate/native/altimate-core.tspackages/opencode/src/altimate/native/types.tspackages/opencode/src/altimate/review/dbt-patterns.tspackages/opencode/src/altimate/review/diff-filter.tspackages/opencode/src/altimate/review/orchestrate.tspackages/opencode/src/altimate/review/runner.tspackages/opencode/src/cli/cmd/review.tspackages/opencode/test/altimate/data-diff-duckdb-e2e.test.tspackages/opencode/test/altimate/review-ci.test.tspackages/opencode/test/altimate/review-dbt-patterns.test.tspackages/opencode/test/altimate/review-runner.test.tspackages/opencode/test/altimate/review.test.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 16 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="packages/opencode/src/altimate/review/dbt-patterns.ts">
<violation number="1" location="packages/opencode/src/altimate/review/dbt-patterns.ts:822">
P2: `detectSchemaYmlPatterns` now double-reports removed `relationships` tests by combining an existing hard-coded detector with the catalog rule for the same condition.</violation>
</file>
<file name="docs/internal/dbt-pr-review-demo-scenario-corpus.md">
<violation number="1" location="docs/internal/dbt-pr-review-demo-scenario-corpus.md:6">
P3: Machine-specific absolute paths in the runbook make the documented validation command non-reproducible for other developers.</violation>
<violation number="2" location="docs/internal/dbt-pr-review-demo-scenario-corpus.md:68">
P3: Implemented 50-Scenario Corpus table is inconsistent with the defined metadata schema: the evidence column uses the header `deterministic evidence` (taken from the meaning text) instead of the schema field name `evidence`, and the required `validation` column is missing entirely.</violation>
</file>
<file name="packages/opencode/src/altimate/review/orchestrate.ts">
<violation number="1" location="packages/opencode/src/altimate/review/orchestrate.ts:1152">
P1: Fallback PII findings are suppressed based on capability, even when diff-scoped core PII classification produced no result, which can hide real PII exposure findings on classifier failure.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const diffScopedPiiFiles = new Set<string>() | ||
| if (input.runner.classifyPii && input.runner.columnLineage) { | ||
| for (const ctx of ctxByPath.values()) { | ||
| if (ctx.engineNewSql && ctx.file.status !== "deleted") diffScopedPiiFiles.add(ctx.file.path) |
There was a problem hiding this comment.
P1: Fallback PII findings are suppressed based on capability, even when diff-scoped core PII classification produced no result, which can hide real PII exposure findings on classifier failure.
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 1152:
<comment>Fallback PII findings are suppressed based on capability, even when diff-scoped core PII classification produced no result, which can hide real PII exposure findings on classifier failure.</comment>
<file context>
@@ -1138,6 +1146,12 @@ export async function runReview(input: OrchestrateInput): Promise<VerdictEnvelop
+ const diffScopedPiiFiles = new Set<string>()
+ if (input.runner.classifyPii && input.runner.columnLineage) {
+ for (const ctx of ctxByPath.values()) {
+ if (ctx.engineNewSql && ctx.file.status !== "deleted") diffScopedPiiFiles.add(ctx.file.path)
+ }
+ }
</file context>
Contributor
Author
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks for the heads-up — I’ll skip that obsolete comment.
| }) | ||
| out.push(f) | ||
| } | ||
| out.push(...evaluateCatalog(file, "", added, removed, rubric)) |
There was a problem hiding this comment.
P2: detectSchemaYmlPatterns now double-reports removed relationships tests by combining an existing hard-coded detector with the catalog rule for the same condition.
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>`detectSchemaYmlPatterns` now double-reports removed `relationships` tests by combining an existing hard-coded detector with the catalog rule for the same condition.</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( | |
| (f) => | |
| !( | |
| genuinelyRemoved.some((l) => /\brelationships\b/i.test(l)) && | |
| f.evidence?.tool === "rule-catalog" && | |
| (f.evidence?.result as any)?.rule === "relationship-test-removed-yml" | |
| ), | |
| ), | |
| ) |
Contributor
Author
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks for the update — I’ll ignore that obsolete comment.
| with DuckDB, `dbt build`, `dbt compile`, `dbt docs generate`, and the default | ||
| focused review command above. AI must remain disabled for demo acceptance. | ||
|
|
||
| | id | branch | status | title | category | expected | deterministic evidence | artifact_needs | demo_script | |
There was a problem hiding this comment.
P3: Implemented 50-Scenario Corpus table is inconsistent with the defined metadata schema: the evidence column uses the header deterministic evidence (taken from the meaning text) instead of the schema field name evidence, and the required validation column is missing entirely.
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 68:
<comment>Implemented 50-Scenario Corpus table is inconsistent with the defined metadata schema: the evidence column uses the header `deterministic evidence` (taken from the meaning text) instead of the schema field name `evidence`, and the required `validation` column is missing entirely.</comment>
<file context>
@@ -0,0 +1,217 @@
+with DuckDB, `dbt build`, `dbt compile`, `dbt docs generate`, and the default
+focused review command above. AI must remain disabled for demo acceptance.
+
+| id | branch | status | title | category | expected | deterministic evidence | artifact_needs | demo_script |
+|---|---|---|---|---|---|---|---|---|
+| s001 | `demo/safe-refactor` | implemented | Safe CTE refactor is approved | safe_refactor | APPROVE | no findings | manifest, catalog, target-base | Proves the reviewer stays quiet for harmless SQL cleanup. |
</file context>
Contributor
Author
There was a problem hiding this comment.
There was a problem hiding this comment.
Got it, thanks for the scope update.
| Status: active corpus build, started 2026-06-08. | ||
|
|
||
| Goal: build 50 customer-demo-ready dbt PR scenarios in | ||
| `/Users/anandgupta/codebase/altimate-code/demo/dbt-pr-review-demo`. Each |
There was a problem hiding this comment.
P3: Machine-specific absolute paths in the runbook make the documented validation command non-reproducible for other developers.
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 6:
<comment>Machine-specific absolute paths in the runbook make the documented validation command non-reproducible for other developers.</comment>
<file context>
@@ -0,0 +1,217 @@
+Status: active corpus build, started 2026-06-08.
+
+Goal: build 50 customer-demo-ready dbt PR scenarios in
+`/Users/anandgupta/codebase/altimate-code/demo/dbt-pr-review-demo`. Each
+scenario should be a small PR branch that either demonstrates a deterministic
+reviewer catch or proves the reviewer correctly stays quiet for a safe change.
</file context>
Contributor
Author
There was a problem hiding this comment.
There was a problem hiding this comment.
Got it, thanks for the update.
Contributor
❌ Tests — Failures DetectedTypeScript — 15 failure(s)
Next StepPlease address the failing cases above and re-run verification. |
anandgupta42
force-pushed
the
codex/dbt-review-demo-corpus
branch
from
b0c0fce to
6185bfd
Compare
anandgupta42
changed the title
Contributor
Author
|
Addressed review feedback for the current scope:
The older centralized test failure comment was for the prior PR contents; the current branch has been force-updated and checks are re-running on commit 6185bfd. |
anandgupta42
force-pushed
the
codex/dbt-review-demo-corpus
branch
from
6185bfd to
0ac9a2a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PINEAPPLE
Summary
Validation
Summary by CodeRabbit
Bug Fixes
Tests