Skip to content

Demo s032: ORDER BY added without limiting rows - #33

Open
anandgupta42 wants to merge 1 commit into
mainfrom
demo/s032-order-by-in-cte
Open

Demo s032: ORDER BY added without limiting rows#33
anandgupta42 wants to merge 1 commit into
mainfrom
demo/s032-order-by-in-cte

Conversation

@anandgupta42

Copy link
Copy Markdown
Contributor

Demo scenario

  • Scenario ID: s032
  • Branch: demo/s032-order-by-in-cte
  • Risk category: warehouse_cost
  • Expected reviewer verdict: COMMENT
  • Expected deterministic evidence: dbt-patterns:order-by-no-limit

Customer walkthrough

Shows a sort that adds cost without a stable consumer contract.

When presenting this PR, first show that dbt can build the project, then show the reviewer finding. The point is that the reviewer catches or ignores the change from deterministic dbt/SQL evidence, not from advisory AI text.

Validation command

rm -f demo.duckdb demo.duckdb.wal
dbt build --profiles-dir . --target dev
dbt compile --profiles-dir . --target dev
dbt docs generate --profiles-dir . --target dev
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 \
  --base=main \
  --head=HEAD \
  --mode=gate \
  --manifest=/Users/anandgupta/codebase/altimate-code/demo/dbt-pr-review-demo/target/manifest.json \
  --json \
  --no-ai

Acceptance notes

  • DuckDB default warehouse.
  • AI disabled with --no-ai.
  • Included in the final local corpus matrix: 50/50 branches matched expected verdicts.
  • Real-world corpus floor held at 15/15 caught bad cases and 0/5 false positives.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

💬 Reviewed with comments — 2 warning, 2 suggestion (lite tier)

⚙️ Lint-only run — no dbt manifest/warehouse was available, so lineage, equivalence and
data-impact checks were skipped. Wire manifest_path (and optionally warehouse creds) for the full verdict.

⚠️ Warning (2)

  • fct_customer_orders: refactor could not be proven equivalent
    The logic of fct_customer_orders changed and equivalence could not be decided (no schema, or unsupported SQL). Treat as a potential behavior change and verify with a data-diff.
    models/marts/fct_customer_orders.sql · unverified · semantic_change
  • fct_customer_orders: exposes Name column customer_name
    This change surfaces a Name-classified column customer_name (confidence 75%). Confirm masking / access policy before merging to a non-restricted schema; suggested masking: '***MASKED***'.
    models/marts/fct_customer_orders.sql · pii_exposure

💡 Suggestion (2)

  • fct_customer_orders: top-level ORDER BY without LIMIT — sorts for nothing
    A model-level ORDER BY with no LIMIT pays a full sort on every run while downstream consumers can't rely on order anyway. Remove it (use window functions if order matters).
    models/marts/fct_customer_orders.sql · warehouse_cost
  • fct_customer_orders: missing_table_alias
    Multi-table query without aliases for: orders, customers
    models/marts/fct_customer_orders.sql:8 · sql_quality

altimate dbt-pr-review · verdict COMMENT · signed sha256:b01d094f33d… · manifest 3fb58126c4

@github-actions github-actions 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.

💬 Reviewed with comments — 2 warning, 2 suggestion (lite tier)

@@ -6,6 +6,7 @@ with orders as (
amount,
status
from {{ ref('stg_orders') }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 fct_customer_orders: missing_table_alias

Multi-table query without aliases for: orders, customers

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant