Build end-to-end dbt warehouse with cleaned staging, marts, tests, an…#76
Conversation
…d transformations
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThis PR adds a complete three-tier Silver layer following Medallion architecture: staging models clean and standardize raw bronze data, intermediate models enrich and curate those records, and marts deliver business-ready dimensions, facts, and reporting views with aggregated metrics across customer, product, inventory, and transaction domains. ChangesSilver Layer Build with Medallion Architecture
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Ritik574-coder
left a comment
Ritik574-coder
left a comment
There was a problem hiding this comment.
- Fixed:
- invalid SQL syntax
- quote handling issues
- TRIM on INT datatype errors
- varchar arithmetic errors
- duplicate transaction handling
- upstream/downstream dependency failures
|
Failed to generate code suggestions for PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb9c747a41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| LEFT JOIN sales s ON c.customer_id = s.customer_id | ||
| LEFT JOIN returns r ON c.customer_id = r.customer_id |
There was a problem hiding this comment.
Joining fct_sales and fct_returns directly to dim_customers on customer_id creates a many-to-many fanout whenever a customer has multiple sales and multiple returns, which inflates SUM(s.quantity_ordered), SUM(s.line_total_with_tax), and SUM(r.refund_amount) (and therefore return_rate_pct). Pre-aggregate each fact table to one row per customer in separate CTEs before joining to avoid multiplying rows.
Useful? React with 👍 / 👎.
| dimensions: | ||
| +materialized: table | ||
| +schema: gold |
There was a problem hiding this comment.
These configuration keys use lowercase path segments (dimensions/facts/reporting), but the actual model folders are models/marts/Dimensions, models/marts/Facts, and models/marts/Reporting; dbt treats unmatched paths as unused config, so the intended materialization/schema settings for marts will not be applied to those models.
Useful? React with 👍 / 👎.
| ROW_NUMBER() OVER ( | ||
| PARTITION BY transaction_id | ||
| ORDER BY transaction_id | ||
| ) AS _row_flag |
There was a problem hiding this comment.
The deduplication window partitions by transaction_id and orders by the same transaction_id, so duplicate rows tie on the sort expression and SQL Server can pick an arbitrary survivor. That makes which record is kept non-deterministic when duplicates differ in other columns, causing run-to-run metric drift; include a stable tie-breaker (for example last_modified_ts or record_created_ts) in the ORDER BY.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (12)
script/silver/dbt_project/models/staging/src_customers.sql (1)
36-39: ⚡ Quick winDeduplication ORDER BY is non-deterministic.
When multiple rows exist for the same
customer_id,ORDER BY customer_idprovides no tiebreaker—SQL Server may return any row as_rn = 1. Consider ordering by a timestamp column (e.g.,record_created_ts DESC) or another deterministic attribute to ensure consistent results across runs.🤖 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 `@script/silver/dbt_project/models/staging/src_customers.sql` around lines 36 - 39, The ROW_NUMBER() window that assigns _rn is non-deterministic because it ORDERs only by customer_id; change the ORDER BY clause inside ROW_NUMBER() to include a deterministic tiebreaker such as a timestamp or unique key (for example use ORDER BY customer_id, record_created_ts DESC or ORDER BY customer_id, updated_at DESC, or include a unique surrogate like customer_pk DESC) so that ROW_NUMBER() OVER (PARTITION BY customer_id ORDER BY ...) consistently picks the same row for _rn = 1.script/silver/dbt_project/models/staging/src_inventory.sql (1)
33-43: 💤 Low valueMissing TRIM before empty-string check may miss whitespace-only values.
Lines 35 and 41 check
product_name = ''andsku = ''without trimming first. A value like' 'would not match= ''and would pass through untrimmed. Consider usingTRIM(product_name) = ''for consistency with the pattern used elsewhere in this codebase.🤖 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 `@script/silver/dbt_project/models/staging/src_inventory.sql` around lines 33 - 43, The CASE expressions for product_name and sku currently check product_name = '' and sku = '' which misses whitespace-only values; update each check to trim before comparison (e.g., use TRIM(product_name) = '' and TRIM(sku) = ''), and ensure the ELSE branch still returns TRIM(product_name) / TRIM(sku) so whitespace-only strings are normalized to 'Unknown' while non-empty values are trimmed; locate the CASE blocks labeled for product_name and sku in the staging src_inventory.sql and apply these trimmed-empty checks.script/silver/dbt_project/models/staging/src_returns.sql (1)
122-125: ⚡ Quick winApplying TitleCase to free-form notes may corrupt content.
Notes fields typically contain free-form text that shouldn't have case transformations applied. TitleCase could alter intentional casing, technical terms, acronyms, or formatted content. Consider just trimming and stripping CR/LF without case modification.
Proposed fix
,CASE WHEN notes IS NULL OR LEN(TRIM(notes)) < 3 OR TRIM(notes) = '' THEN 'Unknown' - ELSE TRIM(dbo.TitleCase(REPLACE(REPLACE(notes, CHAR(10), ''), CHAR(13), ''))) + ELSE TRIM(REPLACE(REPLACE(notes, CHAR(10), ' '), CHAR(13), '')) END as notes🤖 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 `@script/silver/dbt_project/models/staging/src_returns.sql` around lines 122 - 125, The notes transformation currently forces TitleCase via dbo.TitleCase which can corrupt free-form content; update the CASE expression for the notes column to stop calling dbo.TitleCase and instead return either 'Unknown' when notes is NULL/very short or the trimmed notes with CR/LF removed (use TRIM and nested REPLACE for CHAR(10)/CHAR(13)) preserving original casing; adjust the expression that references notes, dbo.TitleCase, REPLACE, TRIM and LEN accordingly.script/silver/dbt_project/models/staging/src_transactions.sql (1)
12-14: ⚡ Quick winDeduplication ORDER BY is non-deterministic.
Same issue as in
src_customers.sqlandsrc_employees.sql—without a tiebreaker column likerecord_created_tsorlast_modified_ts, the row selected for_row_flag = 1is arbitrary.🤖 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 `@script/silver/dbt_project/models/staging/src_transactions.sql` around lines 12 - 14, The ROW_NUMBER() partition for deduplication is non-deterministic because ORDER BY transaction_id alone has no tiebreaker; update the ROW_NUMBER() expression (used to produce the _row_flag = 1 dedupe) to include a deterministic tiebreaker such as record_created_ts or last_modified_ts (e.g., ORDER BY transaction_id, record_created_ts DESC) so the chosen row is deterministic and the same logic as in src_customers.sql/src_employees.sql is applied.script/silver/dbt_project/models/staging/src_employees.sql (2)
29-32: ⚡ Quick winDeduplication ORDER BY is non-deterministic.
Same issue as in
src_customers.sql—without a proper tiebreaker column, the row selected for_rn = 1is arbitrary when duplicates exist.🤖 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 `@script/silver/dbt_project/models/staging/src_employees.sql` around lines 29 - 32, The ROW_NUMBER() window uses ORDER BY employee_id which is non-deterministic for ties; update the window in src_employees.sql so the ORDER BY includes a deterministic tiebreaker (e.g., a timestamp or unique key) rather than just employee_id. Locate the ROW_NUMBER() AS _rn expression and change ORDER BY employee_id to ORDER BY employee_id, <deterministic_column> (for example last_updated_at DESC or created_at DESC or the surrogate PK) so that _rn = 1 consistently picks the intended row.
46-59: 💤 Low valueName parsing is stricter than src_customers and drops multi-word names.
This logic only extracts names when
full_namecontains exactly one space. Names with middle names or multiple parts (e.g., "Mary Jane Watson") result in NULL for bothfirst_nameandlast_name. In contrast,src_customers.sqlhandles 2-space names by extracting the first and last parts. Consider aligning the logic for consistency.🤖 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 `@script/silver/dbt_project/models/staging/src_employees.sql` around lines 46 - 59, The parsing only handles exactly one space; change the logic for first_name and last_name to handle full_name with one or more spaces by checking ">= 1" and extracting the first token and the last token instead of relying on PARSENAME for two-part-only names: for first_name use LEFT(TRIM(full_name), CHARINDEX(' ', TRIM(full_name)) - 1) (with NULL guard if no space) and for last_name use RIGHT(TRIM(full_name), CHARINDEX(' ', REVERSE(TRIM(full_name))) - 1); update the CASEs that reference full_name, first_name, last_name, PARSENAME, LEN, REPLACE accordingly so multi-part names return first and last parts like src_customers.script/silver/dbt_project/models/staging/src_reviews.sql (2)
24-24: ⚡ Quick winRating lacks range validation.
ratingis cast to INT but not validated for typical rating bounds (e.g., 1-5). Invalid ratings like -1 or 100 would pass through. Consider adding range validation similar tosrc_products.sqlline 122 which validatesrating_avgbetween 0 and 5.Proposed fix
- TRY_CAST(rating AS INT) AS rating, + CASE + WHEN TRY_CAST(rating AS INT) IS NULL THEN NULL + WHEN TRY_CAST(rating AS INT) < 1 OR TRY_CAST(rating AS INT) > 5 THEN NULL + ELSE TRY_CAST(rating AS INT) + END AS rating,🤖 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 `@script/silver/dbt_project/models/staging/src_reviews.sql` at line 24, The rating column is only cast to INT (TRY_CAST(rating AS INT) AS rating) but isn't constrained to an acceptable range; change the expression to validate bounds (e.g., 1-5) and return NULL for out-of-range values similar to the rating_avg check in src_products.sql; for example, replace TRY_CAST(rating AS INT) AS rating with a guarded expression that assigns the cast value only if it falls between 1 and 5 (otherwise NULL) using a CASE/WHEN or equivalent so invalid ratings like -1 or 100 are filtered out.
11-12: 💤 Low valuereview_id passed through without validation.
Unlike other staging models that validate primary keys with
TRY_CASTand NULL/negative checks,review_idis passed directly without validation. Consider adding validation for consistency with other models.🤖 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 `@script/silver/dbt_project/models/staging/src_reviews.sql` around lines 11 - 12, review_id is being passed through without validation; update the SELECT to validate and cast it like other staging models by using TRY_CAST(review_id AS BIGINT) AS review_id and then filter out invalid values (e.g. WHERE TRY_CAST(review_id AS BIGINT) IS NOT NULL AND TRY_CAST(review_id AS BIGINT) >= 0) or perform the TRY_CAST in a CTE and drop rows with NULL/negative review_id so the model only emits valid primary keys.script/silver/dbt_project/models/staging/src_stores.sql (1)
51-65: ⚖️ Poor tradeoffIncomplete state abbreviation mapping creates inconsistent output.
Only 11 states are explicitly mapped to abbreviations. Other states like "New York" or "Florida" pass through as full names, creating a mix of formats (e.g., "CA" alongside "New York"). Consider either mapping all 50 states or using a reference table join.
🤖 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 `@script/silver/dbt_project/models/staging/src_stores.sql` around lines 51 - 65, The CASE expression producing "END as state" in src_stores.sql only maps 11 states and leaves others as full names (via ELSE state), causing mixed formats; replace this hardcoded CASE with a deterministic solution by either (a) joining to a canonical state_abbreviations reference table (lookup by TRIM(state) or normalized name) and selecting COALESCE(ref.abbreviation, 'Unknown') as state, or (b) expand the CASE to cover all 50 states mapping TRIM(state) -> two-letter codes and keep the existing fallback for NULL/empty via LEN(TRIM(state)) < 2; update references to TRIM(state) and the final alias "state" accordingly so output is consistently two-letter abbreviations.script/silver/dbt_project/models/staging/src_products.sql (1)
144-147: 💤 Low valueLowercasing entire URL may break case-sensitive paths.
While domain names are case-insensitive, URL paths can be case-sensitive on Unix-based servers. Lowercasing the entire URL could produce broken links. Consider lowercasing only the domain portion, or preserving the original case for the path.
🤖 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 `@script/silver/dbt_project/models/staging/src_products.sql` around lines 144 - 147, The CASE expression that transforms product_url currently lowercases the entire URL via LOWER(product_url), which can break case-sensitive paths; modify the transformation for product_url to only lowercase the domain/host portion while preserving the original case of the path: parse the trimmed product_url into scheme ('https://'), host (up to the first '/'), and path (rest), apply LOWER() only to the host portion, then reconstruct scheme + lowercased host + original path and still remove CR/LF with the existing REPLACE calls; keep the same NULL/empty/NOT LIKE check and the column alias product_url.script/silver/dbt_project/models/marts/schema.yml (1)
117-118: ⚡ Quick winEnforce the documented
ratingdomain with anaccepted_valuestest.The description says ratings are
1-5, but that constraint is not validated.Suggested schema test patch
- name: fct_reviews description: "Customer review fact table with ratings and engagement metrics" columns: @@ - name: rating description: "Customer rating (1-5)" + tests: + - not_null + - accepted_values: + values: [1, 2, 3, 4, 5]🤖 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 `@script/silver/dbt_project/models/marts/schema.yml` around lines 117 - 118, The schema declares rating as "1-5" but no test enforces it; edit the model's column entry for rating in schema.yml and add a dbt accepted_values test (e.g., under the model that defines the rating column add tests: - accepted_values: values: [1,2,3,4,5]) so the column-level symbol "rating" has an accepted_values constraint validating only 1–5; ensure the test is indented under the same model/columns block where "name: rating" appears.script/silver/dbt_project/models/marts/Facts/fct_reviews.sql (1)
8-10: ⚡ Quick winUse a conformed transaction key name in marts outputs.
This fact uses
txn_idwhile sales fact usestransaction_id. Aligning the exposed name improves downstream join consistency.♻️ Proposed change
SELECT review_id, - txn_id, + txn_id AS transaction_id, customer_id,🤖 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 `@script/silver/dbt_project/models/marts/Facts/fct_reviews.sql` around lines 8 - 10, The marts output in fct_reviews.sql exposes txn_id which is not conformed with other marts (they use transaction_id); update the select/output schema in fct_reviews.sql to expose transaction_id instead of txn_id (or alias txn_id AS transaction_id) so downstream joins match the sales fact; ensure any references in the same model (e.g., joins or tests) are updated to use transaction_id.
🤖 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 `@script/silver/dbt_project/dbt_project.yml`:
- Around line 39-46: The marts model config uses lowercase keys (dimensions,
facts, reporting) which don't match the actual model directory names
(Dimensions, Facts, Reporting) so dbt won't apply +materialized/+schema; update
the config keys under the marts block to match exact case of the directories
(use Dimensions, Facts, Reporting) so that the existing +materialized and
+schema settings are applied to those models.
In
`@script/silver/dbt_project/models/intermediate/Employees/int_employees_standardized.sql`:
- Line 26: The calculated_tenure_years uses DATEDIFF(YEAR, hire_date, GETDATE())
which can be off-by-one; change it to compute completed years by taking the
DATEDIFF(YEAR, hire_date, GETDATE()) value and subtracting 1 when the current
date is before the hire anniversary in the current year. Concretely, replace the
direct DATEDIFF(YEAR, hire_date, GETDATE()) expression used for
calculated_tenure_years with a conditional that checks whether DATEADD/YEAR of
that DATEDIFF applied to hire_date is greater than GETDATE() (or alternatively
compare month/day of GETDATE() vs hire_date) and subtracts one in that case so
calculated_tenure_years reflects full completed years.
In
`@script/silver/dbt_project/models/intermediate/Inventory/int_inventory_summary.sql`:
- Line 20: The current CASE expression for needs_reorder treats NULLs as 'No'
because comparisons with NULL yield UNKNOWN; update the logic in the SELECT
producing needs_reorder (referencing stock_available, reorder_level, and alias
needs_reorder) to explicitly preserve unknown by returning NULL when either
stock_available or reorder_level IS NULL, otherwise return 'Yes' if
stock_available <= reorder_level and 'No' otherwise.
In
`@script/silver/dbt_project/models/intermediate/Products/int_products_standardized.sql`:
- Line 23: The low_stock_flag CASE treats NULL comparisons as ELSE 'No',
understating risk; update the logic in int_products_standardized.sql so
low_stock_flag returns 'Yes' when stock_quantity or reorder_level is NULL or
when stock_quantity <= reorder_level, and only returns 'No' when both values are
non-NULL and stock_quantity > reorder_level; change the CASE tied to columns
stock_quantity and reorder_level that produces low_stock_flag accordingly.
In
`@script/silver/dbt_project/models/intermediate/Stores/int_stores_standardized.sql`:
- Line 23: The years_operating field overcounts when the current date is before
the store's anniversary; replace the simple DATEDIFF(YEAR, opened_date,
GETDATE()) logic with a two-step calculation: compute diff = DATEDIFF(YEAR,
opened_date, GETDATE()) and then subtract 1 if the anniversary this year (use
DATEADD to add diff years to opened_date) is after GETDATE(); update the
expression that defines years_operating to use DATEDIFF, DATEADD and a
conditional subtraction so stores haven't reached their anniversary yet are not
overcounted.
In `@script/silver/dbt_project/models/marts/Reporting/customer_analysis.sql`:
- Around line 7-12: The sales and returns CTEs (sales, returns) pull raw
fct_sales and fct_returns and are later joined at customer grain causing fanout
and inflated SUMs (e.g., SUM(s.quantity_ordered), SUM(s.line_total_with_tax),
SUM(r.refund_amount)). Fix by pre-aggregating each CTE to the customer-level (or
the exact grain used for reporting) before the join: replace sales := SELECT ...
FROM {{ ref('fct_sales') }} with an aggregated sales_by_customer CTE (GROUP BY
customer_id and any report-level dimensions, computing SUM(quantity_ordered),
SUM(line_total_with_tax), COUNT(*) as needed) and do the same for returns using
{{ ref('fct_returns') }}, then join these aggregated CTEs in the final select to
avoid row multiplication.
In `@script/silver/dbt_project/models/marts/schema.yml`:
- Around line 74-79: Add not_null and relationships tests for the foreign keys
in the fct_sales model: ensure each column customer_id, product_id, and store_id
has a tests: - not_null and a relationships test pointing to its source
dimension (e.g., customer_dim/id, product_dim/id, store_dim/id) in the
schema.yml for the fct_sales model; update the fct_sales entry to include these
tests so orphaned facts are caught and joins validated.
In `@script/silver/dbt_project/models/staging/sources.yml`:
- Around line 8-14: The sources.yml currently sets freshness but leaves
loaded_at_field explicitly as YAML null (loaded_at_field: ~) for multiple
sources; remove the null-valued loaded_at_field entries or replace them with
valid values so dbt freshness works correctly: either (a) set loaded_at_field to
the actual timestamp column/expression for each source, (b) delete the
loaded_at_field key entirely to allow metadata-based freshness, or (c) remove
loaded_at_field and add a loaded_at_query per source to use query-based
freshness; update each table block that contains loaded_at_field: ~ accordingly.
In `@script/silver/dbt_project/models/staging/src_products.sql`:
- Line 149: Replace the hardcoded table reference "[bronze].[products]" with
dbt's source macro so dbt can track lineage and freshness; locate the SELECT/
FROM clause that currently uses the literal [bronze].[products] in this staging
model and change it to call the source macro {{ source('bronze', 'products') }}
(i.e., use source('bronze','products') instead of the bracketed table), ensuring
any surrounding quoting/aliasing remains correct and run dbt compile to verify
the model resolves.
In `@script/silver/dbt_project/models/staging/src_returns.sql`:
- Line 127: Replace the hardcoded table reference [bronze].[returns] in the FROM
clause with the dbt source macro to enable source tracking; locate the FROM
[bronze].[returns] occurrence in staging/src_returns.sql and change it to use {{
source('bronze', 'returns') }} so dbt can manage dependencies and documentation.
In `@script/silver/dbt_project/models/staging/src_stores.sql`:
- Line 152: Replace the hardcoded table reference FROM [bronze].[stores] with
dbt's source macro by using {{ source('bronze', 'stores') }} in the model
(locate the FROM clause in src_stores.sql). Also ensure the corresponding source
is declared in your sources schema.yml (source name "bronze" and table "stores")
so dbt can track and test the source.
In `@script/silver/dbt_project/models/staging/src_transactions.sql`:
- Line 52: The CASE for customer_zip currently uses LEN(customer_zip) which can
misbehave if customer_zip is numeric; update the validation in the CASE
expression that defines customer_zip to compute length on a string cast (e.g.,
LEN(CAST(customer_zip AS VARCHAR))) or use a numeric range check (e.g., BETWEEN
10000 AND 99999) instead of LEN, so the line that reads the CASE WHEN
TRY_CAST(customer_zip AS INT)... should be changed to validate against the
cast-to-varchar length or a numeric range before returning TRY_CAST(customer_zip
AS INT) or NULL.
- Line 23: The CASE that computes order_id uses LEN(order_id) which fails if
order_id is numeric; update the predicate to check length after converting to
string or validate numeric range instead—for example, use LEN(CAST(order_id AS
VARCHAR)) < 5 or perform TRY_CAST(order_id AS INT) IS NULL OR (TRY_CAST(order_id
AS INT) < 10000) — modify the CASE/condition around TRY_CAST(order_id AS INT) in
staging/src_transactions.sql to use one of these safe checks so the expression
works whether order_id is stored as text or numeric.
---
Nitpick comments:
In `@script/silver/dbt_project/models/marts/Facts/fct_reviews.sql`:
- Around line 8-10: The marts output in fct_reviews.sql exposes txn_id which is
not conformed with other marts (they use transaction_id); update the
select/output schema in fct_reviews.sql to expose transaction_id instead of
txn_id (or alias txn_id AS transaction_id) so downstream joins match the sales
fact; ensure any references in the same model (e.g., joins or tests) are updated
to use transaction_id.
In `@script/silver/dbt_project/models/marts/schema.yml`:
- Around line 117-118: The schema declares rating as "1-5" but no test enforces
it; edit the model's column entry for rating in schema.yml and add a dbt
accepted_values test (e.g., under the model that defines the rating column add
tests: - accepted_values: values: [1,2,3,4,5]) so the column-level symbol
"rating" has an accepted_values constraint validating only 1–5; ensure the test
is indented under the same model/columns block where "name: rating" appears.
In `@script/silver/dbt_project/models/staging/src_customers.sql`:
- Around line 36-39: The ROW_NUMBER() window that assigns _rn is
non-deterministic because it ORDERs only by customer_id; change the ORDER BY
clause inside ROW_NUMBER() to include a deterministic tiebreaker such as a
timestamp or unique key (for example use ORDER BY customer_id, record_created_ts
DESC or ORDER BY customer_id, updated_at DESC, or include a unique surrogate
like customer_pk DESC) so that ROW_NUMBER() OVER (PARTITION BY customer_id ORDER
BY ...) consistently picks the same row for _rn = 1.
In `@script/silver/dbt_project/models/staging/src_employees.sql`:
- Around line 29-32: The ROW_NUMBER() window uses ORDER BY employee_id which is
non-deterministic for ties; update the window in src_employees.sql so the ORDER
BY includes a deterministic tiebreaker (e.g., a timestamp or unique key) rather
than just employee_id. Locate the ROW_NUMBER() AS _rn expression and change
ORDER BY employee_id to ORDER BY employee_id, <deterministic_column> (for
example last_updated_at DESC or created_at DESC or the surrogate PK) so that _rn
= 1 consistently picks the intended row.
- Around line 46-59: The parsing only handles exactly one space; change the
logic for first_name and last_name to handle full_name with one or more spaces
by checking ">= 1" and extracting the first token and the last token instead of
relying on PARSENAME for two-part-only names: for first_name use
LEFT(TRIM(full_name), CHARINDEX(' ', TRIM(full_name)) - 1) (with NULL guard if
no space) and for last_name use RIGHT(TRIM(full_name), CHARINDEX(' ',
REVERSE(TRIM(full_name))) - 1); update the CASEs that reference full_name,
first_name, last_name, PARSENAME, LEN, REPLACE accordingly so multi-part names
return first and last parts like src_customers.
In `@script/silver/dbt_project/models/staging/src_inventory.sql`:
- Around line 33-43: The CASE expressions for product_name and sku currently
check product_name = '' and sku = '' which misses whitespace-only values; update
each check to trim before comparison (e.g., use TRIM(product_name) = '' and
TRIM(sku) = ''), and ensure the ELSE branch still returns TRIM(product_name) /
TRIM(sku) so whitespace-only strings are normalized to 'Unknown' while non-empty
values are trimmed; locate the CASE blocks labeled for product_name and sku in
the staging src_inventory.sql and apply these trimmed-empty checks.
In `@script/silver/dbt_project/models/staging/src_products.sql`:
- Around line 144-147: The CASE expression that transforms product_url currently
lowercases the entire URL via LOWER(product_url), which can break case-sensitive
paths; modify the transformation for product_url to only lowercase the
domain/host portion while preserving the original case of the path: parse the
trimmed product_url into scheme ('https://'), host (up to the first '/'), and
path (rest), apply LOWER() only to the host portion, then reconstruct scheme +
lowercased host + original path and still remove CR/LF with the existing REPLACE
calls; keep the same NULL/empty/NOT LIKE check and the column alias product_url.
In `@script/silver/dbt_project/models/staging/src_returns.sql`:
- Around line 122-125: The notes transformation currently forces TitleCase via
dbo.TitleCase which can corrupt free-form content; update the CASE expression
for the notes column to stop calling dbo.TitleCase and instead return either
'Unknown' when notes is NULL/very short or the trimmed notes with CR/LF removed
(use TRIM and nested REPLACE for CHAR(10)/CHAR(13)) preserving original casing;
adjust the expression that references notes, dbo.TitleCase, REPLACE, TRIM and
LEN accordingly.
In `@script/silver/dbt_project/models/staging/src_reviews.sql`:
- Line 24: The rating column is only cast to INT (TRY_CAST(rating AS INT) AS
rating) but isn't constrained to an acceptable range; change the expression to
validate bounds (e.g., 1-5) and return NULL for out-of-range values similar to
the rating_avg check in src_products.sql; for example, replace TRY_CAST(rating
AS INT) AS rating with a guarded expression that assigns the cast value only if
it falls between 1 and 5 (otherwise NULL) using a CASE/WHEN or equivalent so
invalid ratings like -1 or 100 are filtered out.
- Around line 11-12: review_id is being passed through without validation;
update the SELECT to validate and cast it like other staging models by using
TRY_CAST(review_id AS BIGINT) AS review_id and then filter out invalid values
(e.g. WHERE TRY_CAST(review_id AS BIGINT) IS NOT NULL AND TRY_CAST(review_id AS
BIGINT) >= 0) or perform the TRY_CAST in a CTE and drop rows with NULL/negative
review_id so the model only emits valid primary keys.
In `@script/silver/dbt_project/models/staging/src_stores.sql`:
- Around line 51-65: The CASE expression producing "END as state" in
src_stores.sql only maps 11 states and leaves others as full names (via ELSE
state), causing mixed formats; replace this hardcoded CASE with a deterministic
solution by either (a) joining to a canonical state_abbreviations reference
table (lookup by TRIM(state) or normalized name) and selecting
COALESCE(ref.abbreviation, 'Unknown') as state, or (b) expand the CASE to cover
all 50 states mapping TRIM(state) -> two-letter codes and keep the existing
fallback for NULL/empty via LEN(TRIM(state)) < 2; update references to
TRIM(state) and the final alias "state" accordingly so output is consistently
two-letter abbreviations.
In `@script/silver/dbt_project/models/staging/src_transactions.sql`:
- Around line 12-14: The ROW_NUMBER() partition for deduplication is
non-deterministic because ORDER BY transaction_id alone has no tiebreaker;
update the ROW_NUMBER() expression (used to produce the _row_flag = 1 dedupe) to
include a deterministic tiebreaker such as record_created_ts or last_modified_ts
(e.g., ORDER BY transaction_id, record_created_ts DESC) so the chosen row is
deterministic and the same logic as in src_customers.sql/src_employees.sql is
applied.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 89ff3218-5900-4f80-9037-1b931ab7e374
📒 Files selected for processing (34)
script/silver/dbt_project/dbt_project.ymlscript/silver/dbt_project/models/intermediate/Customers/int_customers_standardized.sqlscript/silver/dbt_project/models/intermediate/Employees/int_employees_standardized.sqlscript/silver/dbt_project/models/intermediate/Inventory/int_inventory_summary.sqlscript/silver/dbt_project/models/intermediate/Products/int_products_standardized.sqlscript/silver/dbt_project/models/intermediate/Returns/int_returns_summary.sqlscript/silver/dbt_project/models/intermediate/Reviews/int_reviews_summary.sqlscript/silver/dbt_project/models/intermediate/Stores/int_stores_standardized.sqlscript/silver/dbt_project/models/intermediate/Transactions/int_transactions_enriched.sqlscript/silver/dbt_project/models/marts/Dimensions/dim_customers.sqlscript/silver/dbt_project/models/marts/Dimensions/dim_employees.sqlscript/silver/dbt_project/models/marts/Dimensions/dim_products.sqlscript/silver/dbt_project/models/marts/Dimensions/dim_stores.sqlscript/silver/dbt_project/models/marts/Facts/fct_inventory.sqlscript/silver/dbt_project/models/marts/Facts/fct_returns.sqlscript/silver/dbt_project/models/marts/Facts/fct_reviews.sqlscript/silver/dbt_project/models/marts/Facts/fct_sales.sqlscript/silver/dbt_project/models/marts/Reporting/customer_analysis.sqlscript/silver/dbt_project/models/marts/Reporting/employee_performance.sqlscript/silver/dbt_project/models/marts/Reporting/inventory_summary.sqlscript/silver/dbt_project/models/marts/Reporting/sales_summary.sqlscript/silver/dbt_project/models/marts/schema.ymlscript/silver/dbt_project/models/staging/schema.ymlscript/silver/dbt_project/models/staging/sources.ymlscript/silver/dbt_project/models/staging/src_customers.sqlscript/silver/dbt_project/models/staging/src_employees.sqlscript/silver/dbt_project/models/staging/src_inventory.sqlscript/silver/dbt_project/models/staging/src_products.sqlscript/silver/dbt_project/models/staging/src_returns.sqlscript/silver/dbt_project/models/staging/src_reviews.sqlscript/silver/dbt_project/models/staging/src_stores.sqlscript/silver/dbt_project/models/staging/src_transactions.sqlscript/silver/dbt_project/package-lock.ymlscript/silver/dbt_project/packages.yml
| dimensions: | ||
| +materialized: table | ||
| +schema: gold | ||
| facts: | ||
| +materialized: table | ||
| +schema: gold | ||
| reporting: | ||
| +materialized: view |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== marts directory names =="
fd -t d . script/silver/dbt_project/models/marts
echo
echo "== marts config keys in dbt_project.yml =="
rg -n '^\s{6}(dimensions|facts|reporting|Dimensions|Facts|Reporting):' script/silver/dbt_project/dbt_project.ymlRepository: Ritik574-coder/dbt_learning_project
Length of output: 355
🌐 Web query:
dbt project.yml models path config keys match directory names case sensitive
💡 Result:
In dbt, the dbt_project.yml keys under models: are resource paths (project-name namespaced), and the path segments must match the actual model filename/subdirectory structure including case. 1) Where model path config keys live in dbt_project.yml - You configure models in dbt_project.yml under models: using a namespaced dictionary where the top-level key is your project name, and nested keys represent subdirectories under your models path. This is described in dbt’s dbt_project.yml and model configuration docs. [1][2] 2) Case sensitivity: model filenames/subdirectories and config matching - dbt requires the model name to match the filename (including case sensitivity). If the casing doesn’t match, dbt may not apply configurations correctly. [2] - Additionally, when defining model properties in YAML via define properties, dbt notes that the name in the YAML must match the model filename “including case sensitivity.” This is another explicit statement about case-sensitive matching between dbt config/property entries and model files. [3] 3) Directory path matching rules (practical implication) - Because the nested models: keys correspond to the relative filesystem path segments under your configured model-paths directories, and because dbt’s model name/filename matching is case sensitive, treat your directory names and filenames as case sensitive when matching them to the keys in dbt_project.yml. [1][4][2][3] 4) Keys under models: are not arbitrary - The doc for model-configs emphasizes that resource paths must be expressed as nested dictionary keys (with the most specific/ deepest configuration taking precedence). [1] So for your query: yes—config keys that represent directory/file path segments in dbt_project.yml must match the actual model paths (including case), because dbt’s model name/filename matching is explicitly case sensitive and dbt’s models: configuration uses those path segments to target models. [2][3][1]
Citations:
- 1: https://docs.getdbt.com/reference/model-configs
- 2: https://docs.getdbt.com/docs/build/sql-models
- 3: https://docs.getdbt.com/reference/define-properties
- 4: https://docs.getdbt.com/reference/project-configs/model-paths
Fix casing mismatch for marts model config keys (dbt is case-sensitive)
In script/silver/dbt_project/dbt_project.yml (lines 39-46), the marts: config keys are dimensions/facts/reporting (lowercase), but the actual model directories are models/marts/Dimensions, models/marts/Facts, and models/marts/Reporting (capitalized). dbt’s config/path matching is case-sensitive, so +materialized/+schema may not apply to the intended models.
Suggested fix
marts:
- dimensions:
+ Dimensions:
+materialized: table
+schema: gold
- facts:
+ Facts:
+materialized: table
+schema: gold
- reporting:
+ Reporting:
+materialized: view
+schema: gold📝 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.
| dimensions: | |
| +materialized: table | |
| +schema: gold | |
| facts: | |
| +materialized: table | |
| +schema: gold | |
| reporting: | |
| +materialized: view | |
| Dimensions: | |
| materialized: table | |
| schema: gold | |
| Facts: | |
| materialized: table | |
| schema: gold | |
| Reporting: | |
| materialized: view | |
| schema: gold |
🤖 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 `@script/silver/dbt_project/dbt_project.yml` around lines 39 - 46, The marts
model config uses lowercase keys (dimensions, facts, reporting) which don't
match the actual model directory names (Dimensions, Facts, Reporting) so dbt
won't apply +materialized/+schema; update the config keys under the marts block
to match exact case of the directories (use Dimensions, Facts, Reporting) so
that the existing +materialized and +schema settings are applied to those
models.
| is_active, | ||
| performance_rating, | ||
| manager_id, | ||
| DATEDIFF(YEAR, hire_date, GETDATE()) AS calculated_tenure_years, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="script/silver/dbt_project/models/intermediate/Employees/int_employees_standardized.sql"
echo "== File header =="
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,120p'
echo
echo "== Context around calculated_tenure_years (lines 1..80 from around line 26) =="
# Print a safe window around line 26 (line numbers shown via nl)
START=10
END=60
nl -ba "$FILE" | sed -n "${START},${END}p"
echo
echo "== Search for hire_date usage/casting in this model =="
rg -n "hire_date|DATEDIFF\\(YEAR|calculated_tenure_years|tenure" "$FILE" || true
echo
echo "== Search repo for other tenure-year calculations patterns =="
rg -n "DATEDIFF\\(YEAR,\\s*hire_date|calculated_tenure_years|tenure_years" script/silver/dbt_project/models || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="script/silver/dbt_project/models/intermediate/Employees/int_employees_standardized.sql"
echo "== Exists? =="
ls -la "$FILE"
echo
echo "== File contents around start (first 120 lines) with line numbers =="
awk '{printf "%5d\t%s\n", NR, $0}' "$FILE" | sed -n '1,120p'
echo
echo "== Context around calculated_tenure_years (around line 26) =="
awk '{printf "%5d\t%s\n", NR, $0}' "$FILE" | sed -n '10,60p'
echo
echo "== Search within file for hire_date / calculated_tenure_years / tenure =="
rg -n "hire_date|DATEDIFF\(YEAR|calculated_tenure_years|tenure" "$FILE" || true
echo
echo "== Search repo for other tenure-year calculations patterns =="
rg -n "DATEDIFF\(YEAR,\s*hire_date|calculated_tenure_years|tenure_years" script/silver/dbt_project/models || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 2360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILES=(
"script/silver/dbt_project/models/marts/Reporting/employee_performance.sql"
"script/silver/dbt_project/models/marts/Dimensions/dim_employees.sql"
)
for f in "${FILES[@]}"; do
echo "==================== $f ===================="
ls -la "$f"
echo "-- contents (with line numbers) --"
awk '{printf "%5d\t%s\n", NR, $0}' "$f" | sed -n '1,120p'
echo
done
echo "== Search for calculated_tenure_years definitions/overrides =="
rg -n "calculated_tenure_years\s*=|AS\s+calculated_tenure_years|DATEDIFF\\(YEAR,\\s*hire_date|tenure_years" script/silver/dbt_project/models || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 2984
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "years_employed|calculated_tenure_years" script/silver/dbt_project/models || true
echo "== Search for hire_date type/casts in src_employees model =="
# Try common model names; list files that might define src_employees
fd "src_employees" script/silver/dbt_project/models || true
fd "src_employees*" script/silver/dbt_project/models || true
# Search for the src_employees definition if present
rg -n "ref\\('src_employees'\\)" script/silver/dbt_project/models || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 1987
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="script/silver/dbt_project/models/staging/src_employees.sql"
echo "== Exists? =="
ls -la "$FILE"
echo
echo "== Find hire_date handling in src_employees.sql =="
rg -n "hire_date" "$FILE" || true
echo
echo "== Show section around hire_date casts/logic (lines 1..220, then narrowed to matches) =="
awk '{printf "%5d\t%s\n", NR, $0}' "$FILE" | sed -n '1,220p' | tail -n +1
echo
echo "== Show near hire_date occurrences =="
# Extract line ranges around each match (simple approach: print fixed window around first few matches)
MATCH_LINES=$(rg -n "hire_date" "$FILE" | cut -d: -f1 | head -n 10 | tr '\n' ' ')
echo "Match lines: $MATCH_LINES"
for l in $MATCH_LINES; do
START=$((l-15)); END=$((l+25));
awk '{printf "%5d\t%s\n", NR, $0}' "$FILE" | sed -n "${START},${END}p"
echo "---"
doneRepository: Ritik574-coder/dbt_learning_project
Length of output: 31416
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find all usages of years_employed and calculated_tenure_years across models =="
rg -n "years_employed|calculated_tenure_years" script/silver/dbt_project/models || true
echo
echo "== Check for any tenure calculation helpers (e.g., DATEADD/DATEDIFF YEAR adjustments) =="
rg -n "DATEDIFF\\(YEAR,\\s*|DATEADD\\(YEAR,\\s*DATEDIFF\\(YEAR" script/silver/dbt_project/models || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 2152
Fix calculated_tenure_years off-by-one from DATEDIFF(YEAR, ...)
script/silver/dbt_project/models/intermediate/Employees/int_employees_standardized.sql line 26 uses DATEDIFF(YEAR, hire_date, GETDATE()), which counts year boundaries and can overstate completed tenure before the anniversary date.
💡 Proposed fix
- DATEDIFF(YEAR, hire_date, GETDATE()) AS calculated_tenure_years,
+ DATEDIFF(YEAR, hire_date, CAST(GETDATE() AS DATE))
+ - CASE
+ WHEN DATEADD(YEAR, DATEDIFF(YEAR, hire_date, CAST(GETDATE() AS DATE)), hire_date) > CAST(GETDATE() AS DATE)
+ THEN 1 ELSE 0
+ END AS calculated_tenure_years,📝 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.
| DATEDIFF(YEAR, hire_date, GETDATE()) AS calculated_tenure_years, | |
| DATEDIFF(YEAR, hire_date, CAST(GETDATE() AS DATE)) | |
| - CASE | |
| WHEN DATEADD(YEAR, DATEDIFF(YEAR, hire_date, CAST(GETDATE() AS DATE)), hire_date) > CAST(GETDATE() AS DATE) | |
| THEN 1 ELSE 0 | |
| END AS calculated_tenure_years, |
🤖 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
`@script/silver/dbt_project/models/intermediate/Employees/int_employees_standardized.sql`
at line 26, The calculated_tenure_years uses DATEDIFF(YEAR, hire_date,
GETDATE()) which can be off-by-one; change it to compute completed years by
taking the DATEDIFF(YEAR, hire_date, GETDATE()) value and subtracting 1 when the
current date is before the hire anniversary in the current year. Concretely,
replace the direct DATEDIFF(YEAR, hire_date, GETDATE()) expression used for
calculated_tenure_years with a conditional that checks whether DATEADD/YEAR of
that DATEDIFF applied to hire_date is greater than GETDATE() (or alternatively
compare month/day of GETDATE() vs hire_date) and subtracts one in that case so
calculated_tenure_years reflects full completed years.
| unit_cost, | ||
| unit_price, | ||
| inventory_value, | ||
| CASE WHEN stock_available <= reorder_level THEN 'Yes' ELSE 'No' END AS needs_reorder, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show the exact line context around the reported CASE expression
file="script/silver/dbt_project/models/intermediate/Inventory/int_inventory_summary.sql"
if [ -f "$file" ]; then
echo "=== Context for $file (lines 1-80) ==="
sed -n '1,120p' "$file" | nl -ba | sed -n '1,120p'
else
echo "File not found: $file" >&2
exit 1
fi
# 2) Find other occurrences of needs_reorder / reorder_level / stock_available logic
echo "=== rg: needs_reorder ==="
rg -n "needs_reorder" script/silver/dbt_project/models || true
echo "=== rg: stock_available"===
rg -n "stock_available" script/silver/dbt_project/models/intermediate/Inventory || true
echo "=== rg: reorder_level"===
rg -n "reorder_level" script/silver/dbt_project/models/intermediate/Inventory || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 255
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="script/silver/dbt_project/models/intermediate/Inventory/int_inventory_summary.sql"
if [ -f "$file" ]; then
echo "=== Context for $file (lines 1-80) ==="
sed -n '1,120p' "$file" | awk '{printf "%d\t%s\n", NR, $0}'
else
echo "File not found: $file" >&2
exit 1
fi
echo "=== rg: needs_reorder ==="
rg -n "needs_reorder" script/silver/dbt_project/models || true
echo "=== rg: stock_available ==="
rg -n "stock_available" script/silver/dbt_project/models || true
echo "=== rg: reorder_level ==="
rg -n "reorder_level" script/silver/dbt_project/models || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 4583
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant parts of the intermediate model
file="script/silver/dbt_project/models/intermediate/Inventory/int_inventory_summary.sql"
echo "=== int_inventory_summary.sql (full) ==="
sed -n '1,200p' "$file" | awk '{printf "%d\t%s\n", NR, $0}'
# Show how stock_available + reorder_level are produced in staging
file2="script/silver/dbt_project/models/staging/src_inventory.sql"
echo "=== src_inventory.sql (stock_available + reorder_level area) ==="
# Print around the relevant region
sed -n '50,120p' "$file2" | awk '{printf "%d\t%s\n", NR, $0}'
# Show downstream usage of needs_reorder
file3="script/silver/dbt_project/models/marts/Reporting/inventory_summary.sql"
echo "=== inventory_summary.sql (uses needs_reorder) ==="
sed -n '1,120p' "$file3" | awk '{printf "%d\t%s\n", NR, $0}'
file4="script/silver/dbt_project/models/marts/Facts/fct_inventory.sql"
echo "=== fct_inventory.sql (selects needs_reorder) ==="
sed -n '1,120p' "$file4" | awk '{printf "%d\t%s\n", NR, $0}'
# Look for schema/tests/docs mentioning needs_reorder
echo "=== rg: needs_reorder in schema.yml ==="
rg -n "needs_reorder" script/silver/dbt_project/models/marts/schema.yml || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 5558
Preserve unknown reorder status when stock_available or reorder_level is NULL.
CASE WHEN stock_available <= reorder_level ... ELSE 'No' labels rows as 'No' when either input is NULL (comparison yields UNKNOWN), producing a false negative for missing data.
💡 Proposed fix
- CASE WHEN stock_available <= reorder_level THEN 'Yes' ELSE 'No' END AS needs_reorder,
+ CASE
+ WHEN stock_available IS NULL OR reorder_level IS NULL THEN NULL
+ WHEN stock_available <= reorder_level THEN 'Yes'
+ ELSE 'No'
+ END AS needs_reorder,📝 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.
| CASE WHEN stock_available <= reorder_level THEN 'Yes' ELSE 'No' END AS needs_reorder, | |
| CASE | |
| WHEN stock_available IS NULL OR reorder_level IS NULL THEN NULL | |
| WHEN stock_available <= reorder_level THEN 'Yes' | |
| ELSE 'No' | |
| END AS needs_reorder, |
🤖 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
`@script/silver/dbt_project/models/intermediate/Inventory/int_inventory_summary.sql`
at line 20, The current CASE expression for needs_reorder treats NULLs as 'No'
because comparisons with NULL yield UNKNOWN; update the logic in the SELECT
producing needs_reorder (referencing stock_available, reorder_level, and alias
needs_reorder) to explicitly preserve unknown by returning NULL when either
stock_available or reorder_level IS NULL, otherwise return 'Yes' if
stock_available <= reorder_level and 'No' otherwise.
| is_available, | ||
| stock_quantity, | ||
| reorder_level, | ||
| CASE WHEN stock_quantity <= reorder_level THEN 'Yes' ELSE 'No' END AS low_stock_flag, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show the exact section around the referenced line in the file
FILE="script/silver/dbt_project/models/intermediate/Products/int_products_standardized.sql"
echo "=== File: $FILE (lines around 23) ==="
if [ -f "$FILE" ]; then
nl -ba "$FILE" | sed -n '1,120p' | sed -n '15,45p'
else
echo "Missing file: $FILE"
fi
# 2) Find all occurrences of low_stock_flag and the specific CASE predicate
echo
echo "=== Search: low_stock_flag occurrences ==="
rg -n "low_stock_flag" script/silver/dbt_project/models || true
echo
echo "=== Search: stock_quantity <= reorder_level occurrences ==="
rg -n "stock_quantity\s*<=\s*reorder_level" script/silver/dbt_project/models || true
# 3) Search for a likely inventory model that may use similar logic
echo
echo "=== Search: reorder_level usage (context) ==="
rg -n "reorder_level" script/silver/dbt_project/models/intermediate script/silver/dbt_project/models/silver script/silver/dbt_project/models || true
# 4) Inspect any candidate inventory/stock-risk logic files around similar flags
echo
echo "=== Search: low stock / low_stock / stock risk related ==="
rg -n "(low_stock|low stock|stock[_ ]risk|reorder_level)" script/silver/dbt_project/models || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="script/silver/dbt_project/models/intermediate/Products/int_products_standardized.sql"
echo "=== Check file exists: $FILE ==="
ls -l "$FILE" || true
echo
echo "=== File: $FILE (lines 15-45 with line numbers) ==="
if [ -f "$FILE" ]; then
awk 'NR>=15 && NR<=45 {printf "%d:%s\n", NR, $0}' "$FILE"
fi
echo
echo "=== Search: low_stock_flag occurrences ==="
rg -n "low_stock_flag" script/silver/dbt_project/models || true
echo
echo "=== Search: stock_quantity <= reorder_level occurrences ==="
rg -n "stock_quantity\s*<=\s*reorder_level" script/silver/dbt_project/models || true
echo
echo "=== Search: reorder_level usage (intermediate scope) ==="
rg -n "reorder_level" script/silver/dbt_project/models/intermediate || true
echo
echo "=== Search: stock risk / low stock related ==="
rg -n "(low_stock|low stock|stock[_ ]risk|reorder_level)" script/silver/dbt_project/models/intermediate || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 3089
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="script/silver/dbt_project/models/intermediate/Inventory/int_inventory_summary.sql"
echo "=== File: $FILE (lines 1-80) ==="
if [ -f "$FILE" ]; then
awk 'NR>=1 && NR<=80 {printf "%d:%s\n", NR, $0}' "$FILE"
else
echo "Missing file: $FILE"
fi
echo
echo "=== Search for IS NULL / NULL-safe CASE patterns near stock fields ==="
rg -n "(IS NULL|COALESCE|NULLIF).*stock_quantity|stock_quantity.*(IS NULL|COALESCE|NULLIF)" script/silver/dbt_project/models/intermediate || true
rg -n "(IS NULL|COALESCE|NULLIF).*reorder_level|reorder_level.*(IS NULL|COALESCE|NULLIF)" script/silver/dbt_project/models/intermediate || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 818
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="script/silver/dbt_project/models/marts/Dimensions/dim_products.sql"
echo "=== File: $FILE (lines 1-120) ==="
if [ -f "$FILE" ]; then
awk 'NR>=1 && NR<=120 {printf "%d:%s\n", NR, $0}' "$FILE"
else
echo "Missing file: $FILE"
fiRepository: Ritik574-coder/dbt_learning_project
Length of output: 817
Avoid classifying NULL stock inputs as 'No'.
script/silver/dbt_project/models/intermediate/Products/int_products_standardized.sql line 23 uses CASE WHEN stock_quantity <= reorder_level THEN 'Yes' ELSE 'No' END, which will emit 'No' when either stock_quantity or reorder_level is NULL (NULL comparison → ELSE), understating low-stock risk.
💡 Proposed fix
- CASE WHEN stock_quantity <= reorder_level THEN 'Yes' ELSE 'No' END AS low_stock_flag,
+ CASE
+ WHEN stock_quantity IS NULL OR reorder_level IS NULL THEN NULL
+ WHEN stock_quantity <= reorder_level THEN 'Yes'
+ ELSE 'No'
+ END AS low_stock_flag,📝 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.
| CASE WHEN stock_quantity <= reorder_level THEN 'Yes' ELSE 'No' END AS low_stock_flag, | |
| CASE | |
| WHEN stock_quantity IS NULL OR reorder_level IS NULL THEN NULL | |
| WHEN stock_quantity <= reorder_level THEN 'Yes' | |
| ELSE 'No' | |
| END AS low_stock_flag, |
🤖 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
`@script/silver/dbt_project/models/intermediate/Products/int_products_standardized.sql`
at line 23, The low_stock_flag CASE treats NULL comparisons as ELSE 'No',
understating risk; update the logic in int_products_standardized.sql so
low_stock_flag returns 'Yes' when stock_quantity or reorder_level is NULL or
when stock_quantity <= reorder_level, and only returns 'No' when both values are
non-NULL and stock_quantity > reorder_level; change the CASE tied to columns
stock_quantity and reorder_level that produces low_stock_flag accordingly.
| phone, | ||
| manager_name, | ||
| opened_date, | ||
| DATEDIFF(YEAR, opened_date, GETDATE()) AS years_operating, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant section of the file
FILE="script/silver/dbt_project/models/intermediate/Stores/int_stores_standardized.sql"
echo "== File: $FILE =="
python3 - <<'PY'
import itertools,sys,os
path="script/silver/dbt_project/models/intermediate/Stores/int_stores_standardized.sql"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i in range(15,35):
if i<=len(lines):
print(f"{i:4d}: {lines[i-1].rstrip()}")
PY
# Search for years_operating and similar DATEDIFF usage nearby/elsewhere
echo
echo "== rg: years_operating =="
rg -n "years_operating" script/silver/dbt_project/models || true
echo
echo "== rg: DATEDIFF\(YEAR, opened_date =="
rg -n "DATEDIFF\s*\(\s*YEAR\s*,\s*opened_date" script/silver/dbt_project/models || true
echo
echo "== rg: DATEDIFF\(YEAR, .* GETDATE\(\) =="
rg -n "DATEDIFF\s*\(\s*YEAR\s*,[^)]*GETDATE\(\)\s*\)" script/silver/dbt_project/models || true
# Look for employee tenure logic if it exists (same alleged bug pattern)
echo
echo "== rg: tenure / employment years =="
rg -n "(tenure|employee.*tenure|years.*tenure|employment.*years)" script/silver/dbt_project/models || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 2107
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print more context around opened_date usage and the rest of the model
FILE="script/silver/dbt_project/models/intermediate/Stores/int_stores_standardized.sql"
echo "== Context around opened_date in $FILE =="
python3 - <<'PY'
path="script/silver/dbt_project/models/intermediate/Stores/int_stores_standardized.sql"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,line in enumerate(lines, start=1):
if 'opened_date' in line:
start=max(1,i-5)
end=min(len(lines),i+5)
print(f"\n--- {path}:{start}-{end} (around line {i}) ---")
for j in range(start,end+1):
print(f"{j:4d}: {lines[j-1].rstrip()}")
PY
# Inspect employee tenure model for parallel pattern
EFILE="script/silver/dbt_project/models/intermediate/Employees/int_employees_standardized.sql"
echo
echo "== Context around hire_date / calculated_tenure_years in $EFILE =="
python3 - <<'PY'
path="script/silver/dbt_project/models/intermediate/Employees/int_employees_standardized.sql"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,line in enumerate(lines, start=1):
if 'hire_date' in line or 'calculated_tenure_years' in line:
start=max(1,i-5)
end=min(len(lines),i+8)
print(f"\n--- {path}:{start}-{end} (around line {i}) ---")
for j in range(start,end+1):
print(f"{j:4d}: {lines[j-1].rstrip()}")
PY
# Search for casts/conversions of opened_date anywhere else in Stores intermediate/marts
echo
echo "== rg 'opened_date' across Stores intermediate/marts =="
rg -n "opened_date" script/silver/dbt_project/models || true
# Search for TRY_CAST/CAST patterns on opened_date specifically
echo
echo "== rg cast/opened_date patterns =="
rg -n "(CAST|TRY_CAST|CONVERT)\s*\(\s*opened_date" script/silver/dbt_project/models || trueRepository: Ritik574-coder/dbt_learning_project
Length of output: 4526
Compute completed operating years (avoid anniversary overcount)
DATEDIFF(YEAR, opened_date, GETDATE()) in script/silver/dbt_project/models/intermediate/Stores/int_stores_standardized.sql counts crossed year boundaries and can overstate by 1 when a store hasn’t reached its anniversary yet.
💡 Proposed fix
- DATEDIFF(YEAR, opened_date, GETDATE()) AS years_operating,
+ DATEDIFF(YEAR, opened_date, CAST(GETDATE() AS DATE))
+ - CASE
+ WHEN DATEADD(YEAR, DATEDIFF(YEAR, opened_date, CAST(GETDATE() AS DATE)), opened_date) > CAST(GETDATE() AS DATE)
+ THEN 1 ELSE 0
+ END AS years_operating,📝 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.
| DATEDIFF(YEAR, opened_date, GETDATE()) AS years_operating, | |
| DATEDIFF(YEAR, opened_date, CAST(GETDATE() AS DATE)) | |
| - CASE | |
| WHEN DATEADD(YEAR, DATEDIFF(YEAR, opened_date, CAST(GETDATE() AS DATE)), opened_date) > CAST(GETDATE() AS DATE) | |
| THEN 1 ELSE 0 | |
| END AS years_operating, |
🤖 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
`@script/silver/dbt_project/models/intermediate/Stores/int_stores_standardized.sql`
at line 23, The years_operating field overcounts when the current date is before
the store's anniversary; replace the simple DATEDIFF(YEAR, opened_date,
GETDATE()) logic with a two-step calculation: compute diff = DATEDIFF(YEAR,
opened_date, GETDATE()) and then subtract 1 if the anniversary this year (use
DATEADD to add diff years to opened_date) is after GETDATE(); update the
expression that defines years_operating to use DATEDIFF, DATEADD and a
conditional subtraction so stores haven't reached their anniversary yet are not
overcounted.
| ELSE REPLACE(REPLACE(TRIM(LOWER(product_url)), CHAR(13), ''),CHAR(10), '') | ||
| END as product_url | ||
|
|
||
| FROM [bronze].[products] |
There was a problem hiding this comment.
Use dbt source macro instead of hardcoded table reference.
Direct table reference [bronze].[products] bypasses dbt's source tracking, lineage, and freshness checks. Use {{ source('bronze', 'products') }} for consistency with other staging models.
Proposed fix
- FROM [bronze].[products]
+ FROM {{ source('bronze', 'products') }}📝 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.
| FROM [bronze].[products] | |
| FROM {{ source('bronze', 'products') }} |
🤖 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 `@script/silver/dbt_project/models/staging/src_products.sql` at line 149,
Replace the hardcoded table reference "[bronze].[products]" with dbt's source
macro so dbt can track lineage and freshness; locate the SELECT/ FROM clause
that currently uses the literal [bronze].[products] in this staging model and
change it to call the source macro {{ source('bronze', 'products') }} (i.e., use
source('bronze','products') instead of the bracketed table), ensuring any
surrounding quoting/aliasing remains correct and run dbt compile to verify the
model resolves.
| ELSE TRIM(dbo.TitleCase(REPLACE(REPLACE(notes, CHAR(10), ''), CHAR(13), ''))) | ||
| END as notes | ||
|
|
||
| FROM [bronze].[returns] |
There was a problem hiding this comment.
Use dbt source macro instead of hardcoded table reference.
Direct table reference [bronze].[returns] bypasses dbt's source tracking. Use {{ source('bronze', 'returns') }} for consistency.
Proposed fix
- FROM [bronze].[returns]
+ FROM {{ source('bronze', 'returns') }}📝 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.
| FROM [bronze].[returns] | |
| FROM {{ source('bronze', 'returns') }} |
🤖 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 `@script/silver/dbt_project/models/staging/src_returns.sql` at line 127,
Replace the hardcoded table reference [bronze].[returns] in the FROM clause with
the dbt source macro to enable source tracking; locate the FROM
[bronze].[returns] occurrence in staging/src_returns.sql and change it to use {{
source('bronze', 'returns') }} so dbt can manage dependencies and documentation.
| WHEN REPLACE(REPLACE(TRIM(LOWER(has_cafe)), CHAR(13), ''),CHAR(10), '') IN ('0', 'no', 'n','false') THEN 'False' | ||
| ELSE 'Unknown' | ||
| END as has_cafe | ||
| FROM [bronze].[stores] |
There was a problem hiding this comment.
Use dbt source macro instead of hardcoded table reference.
Direct table reference [bronze].[stores] bypasses dbt's source tracking. Use {{ source('bronze', 'stores') }} for consistency.
Proposed fix
- FROM [bronze].[stores]
+ FROM {{ source('bronze', 'stores') }}📝 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.
| FROM [bronze].[stores] | |
| FROM {{ source('bronze', 'stores') }} |
🤖 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 `@script/silver/dbt_project/models/staging/src_stores.sql` at line 152, Replace
the hardcoded table reference FROM [bronze].[stores] with dbt's source macro by
using {{ source('bronze', 'stores') }} in the model (locate the FROM clause in
src_stores.sql). Also ensure the corresponding source is declared in your
sources schema.yml (source name "bronze" and table "stores") so dbt can track
and test the source.
| SELECT | ||
| UPPER(TRIM(transaction_id)) AS transaction_id, | ||
|
|
||
| CASE WHEN TRY_CAST(order_id AS INT) IS NULL OR LEN(order_id) < 5 THEN NULL ELSE TRY_CAST(order_id AS INT) END AS order_id, |
There was a problem hiding this comment.
LEN() check may fail if order_id is numeric type.
LEN(order_id) < 5 assumes order_id is a string. If the source column is numeric, this will either fail or produce unexpected results. Consider checking length after explicit string conversion: LEN(CAST(order_id AS VARCHAR)) or validating the numeric range directly (e.g., < 10000).
Proposed fix
- CASE WHEN TRY_CAST(order_id AS INT) IS NULL OR LEN(order_id) < 5 THEN NULL ELSE TRY_CAST(order_id AS INT) END AS order_id,
+ CASE WHEN TRY_CAST(order_id AS INT) IS NULL OR TRY_CAST(order_id AS INT) < 10000 THEN NULL ELSE TRY_CAST(order_id AS INT) END AS order_id,🤖 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 `@script/silver/dbt_project/models/staging/src_transactions.sql` at line 23,
The CASE that computes order_id uses LEN(order_id) which fails if order_id is
numeric; update the predicate to check length after converting to string or
validate numeric range instead—for example, use LEN(CAST(order_id AS VARCHAR)) <
5 or perform TRY_CAST(order_id AS INT) IS NULL OR (TRY_CAST(order_id AS INT) <
10000) — modify the CASE/condition around TRY_CAST(order_id AS INT) in
staging/src_transactions.sql to use one of these safe checks so the expression
works whether order_id is stored as text or numeric.
| customer_phone, | ||
| customer_city, | ||
| customer_state, | ||
| CASE WHEN TRY_CAST(customer_zip AS INT) IS NULL OR LEN(customer_zip) != 5 THEN NULL ELSE TRY_CAST(customer_zip AS INT) END AS customer_zip, |
There was a problem hiding this comment.
Same LEN() issue for customer_zip if source is numeric.
If customer_zip is stored as a numeric type in bronze, LEN(customer_zip) may not behave as expected. Consider using LEN(CAST(customer_zip AS VARCHAR)) or range-based validation.
🤖 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 `@script/silver/dbt_project/models/staging/src_transactions.sql` at line 52,
The CASE for customer_zip currently uses LEN(customer_zip) which can misbehave
if customer_zip is numeric; update the validation in the CASE expression that
defines customer_zip to compute length on a string cast (e.g.,
LEN(CAST(customer_zip AS VARCHAR))) or use a numeric range check (e.g., BETWEEN
10000 AND 99999) instead of LEN, so the line that reads the CASE WHEN
TRY_CAST(customer_zip AS INT)... should be changed to validate against the
cast-to-varchar length or a numeric range before returning TRY_CAST(customer_zip
AS INT) or NULL.
1 participant
Summary
Built a complete end-to-end dbt analytics engineering pipeline using a medallion-style architecture on SQL Server.
Implemented staging, intermediate, dimension, fact, and reporting models with extensive data cleaning, transformation, deduplication, and validation logic.
Resolved multiple SQL Server compatibility, datatype conversion, dependency, and testing issues across the project.
Changes made
Created staging models for:
Implemented:
Built intermediate transformation models:
Built marts layer:
Added:
Configured:
Fixed:
Validation
Related issues
Notes for reviewers
Project follows a medallion-style warehouse architecture:
All dbt models, tests, and dependencies are successfully passing.
Final successful build result: