v1.0 - #7
Merged
Merged
Conversation
phase: synthetic data generation
…upport idempotent loads
phase: data ingestion
… fix README fence
phase: eda analytics
phase: power bi dashboard
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds a complete payment transaction analytics platform: repository scaffolding and workflows, synthetic Indian payment data generation with fraud calibration, PostgreSQL ingestion with incremental watermarking, dbt transformations from staging through fraud-scored intermediate models to core dimensions/facts and analytics marts (cohort, merchant risk, velocity anomaly, segments), reporting SQL views over marts, Excel export with formatting, Jupyter notebooks with EDA visualizations, comprehensive domain documentation, and ingestion validation tests. ChangesPlatform implementation
Documentation and validation
Sequence Diagram(s)sequenceDiagram
participant Generator as generator/transaction_generator.py
participant CSVFiles as data/raw/*.csv
participant Loader as ingestion/load_to_postgres.py
participant Watermark as ingestion/watermark.py
participant RawDB as raw schema
participant dbt as dbt (payment_dbt)
participant Marts as marts schema
participant Views as reporting schema
participant Excel as excel/generate_excel_report.py
Generator->>CSVFiles: write users.csv, merchants.csv, transactions.csv (seeded, fraud-calibrated)
Loader->>Watermark: get_watermark('transactions')
Watermark-->>Loader: last_loaded_ts or None
Loader->>CSVFiles: read CSV, filter rows > watermark
Loader->>RawDB: bulk INSERT via execute_values (10k chunks, ON CONFLICT DO NOTHING)
Loader->>Watermark: update_watermark(max(transaction_ts), rows_loaded)
dbt->>RawDB: read raw.transactions, raw.users, raw.merchants
dbt->>dbt: stg_transactions (derive flags, rename, extract date/hour)
dbt->>dbt: int_transactions_enriched (window velocity, compute fraud_risk_score, risk_level, reason)
dbt->>dbt: int_user_metrics (aggregate per-user lifetime metrics)
dbt->>Marts: dim_users, dim_merchants (with surrogate keys)
dbt->>Marts: fct_transactions (join enriched to dimensions)
dbt->>Marts: analytic marts (cohort, daily, hourly, merchant_risk, velocity, segments, fraud_analysis)
Views->>Marts: CREATE OR REPLACE VIEW reporting.* (select *, compute ratios, add view_generated_at)
Excel->>Views: SELECT * FROM reporting.* with SET search_path
Excel->>Excel: normalize datetimetz, format cohort_month/transaction_ts, drop view_generated_at
Excel->>Excel: write Payment_Transaction_Analytics_Dashboard.xlsx (styled worksheets, autofilter, borders)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 32
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
dbt/payment_dbt/tests/assert_fraud_score_range.sql (1)
1-4: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winDuplicate test: remove or consolidate.
This custom test validates
fraud_risk_scoreis between 0 and 100, butintermediate/_schema.ymlalready includes adbt_utils.accepted_rangetest (lines 20–23) enforcing the same constraint. Duplicate tests add maintenance overhead and unnecessary test execution time.Remove this custom test since the schema-level range test already provides coverage.
🤖 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 `@dbt/payment_dbt/tests/assert_fraud_score_range.sql` around lines 1 - 4, The custom test assert_fraud_score_range.sql is redundant because the intermediate/_schema.yml already contains a dbt_utils.accepted_range test that enforces the same fraud_risk_score constraint between 0 and 100. Remove the entire assert_fraud_score_range.sql test file to eliminate duplicate test coverage and reduce unnecessary test execution time.dbt/payment_dbt/models/marts/analytics/daily_merchant_kpis.sql (1)
1-36:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
config()block for analytics mart materialization.This analytics mart is missing a
{{ config() }}block. Per coding guidelines, mart models should be materialized astablein themartsschema. Without explicit config, the model defaults to a view, which degrades performance for reporting queries.🔧 Proposed fix
Add at the top of the file:
{{ config( materialized='table', schema='marts', tags=['marts', 'analytics', 'kpi'] ) }}🤖 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 `@dbt/payment_dbt/models/marts/analytics/daily_merchant_kpis.sql` around lines 1 - 36, The daily_merchant_kpis.sql model is missing a config block that specifies materialization settings required for analytics marts. Add a config block at the very top of the file before the CTE definitions (before the "with fct_transactions as" statement) that sets the materialized property to 'table', specifies the schema as 'marts', and includes appropriate tags like 'marts', 'analytics', and 'kpi' to follow the coding guidelines for mart models.Source: Coding guidelines
dbt/payment_dbt/models/marts/analytics/fraud_analysis_mart.sql (1)
1-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
config()block for analytics mart materialization.This analytics mart is missing a
{{ config() }}block. Per coding guidelines, mart models should be materialized astablein themartsschema. Without explicit config, the model defaults to a view, which degrades performance for reporting queries.🔧 Proposed fix
Add at the top of the file:
{{ config( materialized='table', schema='marts', tags=['marts', 'analytics', 'fraud'] ) }}🤖 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 `@dbt/payment_dbt/models/marts/analytics/fraud_analysis_mart.sql` around lines 1 - 33, The fraud_analysis_mart model is missing a config() block at the top of the file, causing it to default to a view materialization instead of the required table materialization for analytics marts. Add a config() block at the very top of the fraud_analysis_mart.sql file (before the with fct_transactions CTE) that specifies materialized='table', sets the schema to 'marts', and includes tags for 'marts', 'analytics', and 'fraud' for proper categorization and performance optimization of reporting queries.Source: Coding guidelines
dbt/payment_dbt/models/marts/analytics/daily_overview_kpis.sql (1)
1-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
config()block for analytics mart materialization.This analytics mart is missing a
{{ config() }}block. Per coding guidelines, mart models should be materialized astablein themartsschema. Without explicit config, the model defaults to a view, which degrades performance for reporting queries.🔧 Proposed fix
Add at the top of the file:
{{ config( materialized='table', schema='marts', tags=['marts', 'analytics', 'kpi'] ) }}🤖 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 `@dbt/payment_dbt/models/marts/analytics/daily_overview_kpis.sql` around lines 1 - 24, The daily_overview_kpis model is missing a dbt config() block at the top of the file, which causes it to default to a view materialization instead of a table. Add a config() block before the fct_transactions CTE definition that specifies materialized='table', schema='marts', and appropriate tags like 'marts', 'analytics', and 'kpi' to ensure this analytics mart is properly materialized as a table for optimal reporting performance.Source: Coding guidelines
dbt/payment_dbt/models/marts/core/dim_merchants.sql (1)
1-19:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAll mart models are missing
_schema.ymlentries with descriptions and tests.The coding guidelines require "Every model must have a description" and "Add tests for surrogate keys (unique + not_null) and key categorical columns." None of the 8 mart models in this PR have accompanying schema definitions.
Required for each model:
dbt/payment_dbt/models/marts/core/dim_merchants.sql#L1-L19: Add_schema.ymlentry with description, tests formerchant_sk(unique, not_null), andis_high_risk_category(accepted_values).dbt/payment_dbt/models/marts/core/fct_transactions.sql#L1-L40: Add_schema.ymlentry with description, tests fortransaction_sk(unique, not_null), relationship tests foruser_sk/merchant_skto dimension tables.dbt/payment_dbt/models/marts/analytics/daily_overview_kpis.sql#L1-L24: Add_schema.ymlentry with description, tests forday(unique, not_null), and range checks for KPI metrics.dbt/payment_dbt/models/marts/analytics/daily_merchant_kpis.sql#L1-L36: Add_schema.ymlentry with description, tests for composite key(merchant_sk, day)uniqueness, relationship test formerchant_sk.dbt/payment_dbt/models/marts/analytics/fraud_analysis_mart.sql#L1-L33: Add_schema.ymlentry with description, tests forfraud_risk_level(accepted_values),fraud_risk_scorerange (0-100).dbt/payment_dbt/models/marts/analytics/hourly_fraud_trends.sql#L1-L20: Add_schema.ymlentry with description, tests for composite key(tx_hour, merchant_category, payment_method)uniqueness, range checks forfraud_rate_pct.dbt/payment_dbt/models/marts/analytics/fraud_customer_segments.sql#L1-L61: Add_schema.ymlentry with description, tests foruser_id(unique, not_null), accepted_values for segment columns.dbt/payment_dbt/models/marts/analytics/merchant_risk_profiling.sql#L1-L72: Add_schema.ymlentry with description, tests formerchant_id(unique, not_null),merchant_risk_category(accepted_values),fraud_rate_pctrange checks.🤖 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 `@dbt/payment_dbt/models/marts/core/dim_merchants.sql` around lines 1 - 19, Create _schema.yml files for each of the 8 mart models to satisfy the coding guidelines requiring descriptions and tests. For dbt/payment_dbt/models/marts/core/dim_merchants.sql: add schema entry with description and tests for merchant_sk (unique and not_null constraints) plus is_high_risk_category (accepted_values test). For dbt/payment_dbt/models/marts/core/fct_transactions.sql: add schema entry with description, tests for transaction_sk (unique and not_null), and relationship tests linking user_sk and merchant_sk to their respective dimension tables. For dbt/payment_dbt/models/marts/analytics/daily_overview_kpis.sql: add schema entry with description, tests for day column (unique and not_null), and range validation for KPI metric columns. For dbt/payment_dbt/models/marts/analytics/daily_merchant_kpis.sql: add schema entry with description, composite uniqueness test for merchant_sk and day combination, and relationship test for merchant_sk referencing the dimension table. For dbt/payment_dbt/models/marts/analytics/fraud_analysis_mart.sql: add schema entry with description, accepted_values test for fraud_risk_level, and range constraint (0-100) for fraud_risk_score column. For dbt/payment_dbt/models/marts/analytics/hourly_fraud_trends.sql: add schema entry with description, composite uniqueness test for the tx_hour, merchant_category, and payment_method combination, and range validation for fraud_rate_pct values. For dbt/payment_dbt/models/marts/analytics/fraud_customer_segments.sql: add schema entry with description, tests for user_id (unique and not_null), and accepted_values tests for all segment classification columns. For dbt/payment_dbt/models/marts/analytics/merchant_risk_profiling.sql: add schema entry with description, tests for merchant_id (unique and not_null), accepted_values for merchant_risk_category, and range checks (0-100) for fraud_rate_pct.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 `@dbt/payment_dbt/dbt_project.yml`:
- Line 20: The fraud_score_threshold variable is set to 0.7, but the
calculate_fraud_risk_score function emits a 0–100 scale score, creating a two
orders of magnitude mismatch. Update the fraud_score_threshold value from 0.7 to
70 to align with the 0–100 score scale produced by calculate_fraud_risk_score.
Additionally, verify that any model or code comparing scores against this
threshold is using the score value directly without additional normalization to
ensure consistency.
In `@dbt/payment_dbt/macros/fraud_scoring.sql`:
- Around line 1-8: The macro calculate_fraud_risk_score has hardcoded weight
values (30, 25, 20, 15, 10) that should be made configurable. Move these weights
into dbt_project.yml as variables (for example, fraud_weight_high_amount,
fraud_weight_risky_merchant, fraud_weight_odd_hour, fraud_weight_velocity_1h,
fraud_weight_velocity_24h) and reference them using var() calls in place of the
hardcoded numbers, similar to how fraud_velocity_threshold_1h and
fraud_velocity_threshold_24h are already used. Additionally, add a Jinja macro
docstring at the top of the calculate_fraud_risk_score macro to document its
purpose, parameters, and the score scale (0-100 or whatever the maximum possible
score is) so callers understand how the fraud risk score is calculated.
In `@dbt/payment_dbt/models/marts/analytics/cohort_analysis.sql`:
- Around line 68-72: The avg_risk_score calculation on line 71 is computing a
simple average of per-user-month averages, which treats each user-month equally
regardless of transaction volume. This causes the metric to drift from the true
cohort-wide average. Replace the simple avg(ca.avg_risk_score) with a weighted
average that accounts for transaction volume by dividing the sum of risk scores
weighted by their respective transaction counts (sum of ca.avg_risk_score
multiplied by ca.tx_count) by the total transaction count (sum of ca.tx_count).
This ensures months with more transactions have proportionally more influence on
the final average.
- Around line 73-99: The cohort maturity gates currently include a cohort as
soon as the measured month starts, but they should wait until the full month has
closed. In the retained_1m_users, retained_3m_users, retained_6m_users,
mature_1m_cohort_size, mature_3m_cohort_size, and mature_6m_cohort_size
aggregations, change the date comparison operators from `<=` to `<` in all six
conditions. This ensures that a cohort is only counted as mature after the
retention month has fully elapsed, not from the first day of that month, which
will prevent underreporting of retention metrics due to partial-month activity.
In `@dbt/payment_dbt/models/marts/analytics/fraud_customer_segments.sql`:
- Around line 1-61: The fraud_customer_segments.sql model is missing a config
block that specifies materialization and schema settings. Add a config block at
the very top of the file (before the with rfm_base CTE) that sets the
materialization to table, specifies the marts schema, and includes appropriate
tags for marts, analytics, and segmentation to ensure this analytics model is
properly materialized as a table for reporting performance rather than
defaulting to a view.
- Around line 39-59: Replace all hardcoded segmentation thresholds in the CASE
statements (recency_segment, frequency_segment, monetary_segment, and
fraud_segment) with dbt variables. First, add configuration variables to
dbt_project.yml for all thresholds: rfm_recency_active_days,
rfm_recency_recent_days, rfm_recency_lapsed_days, rfm_frequency_high,
rfm_frequency_medium, rfm_monetary_high, rfm_monetary_medium, rfm_fraud_repeat,
and rfm_fraud_single. Then, in the fraud_customer_segments.sql file, replace the
numeric literals (7, 30, 90, 8, 30, 25000, 200000, 3, 1) in each CASE expression
with their corresponding {{ var() }} references to make the thresholds
configurable.
In `@dbt/payment_dbt/models/marts/analytics/hourly_fraud_trends.sql`:
- Around line 1-20: The hourly_fraud_trends model is missing a config block at
the top of the file, which causes it to default to a view instead of being
materialized as a table. Add a config() block before the select statement that
specifies materialized='table', sets the schema to 'marts', and includes
appropriate tags such as 'marts', 'analytics', and 'fraud' to ensure the model
is properly configured for analytics reporting performance.
- Line 16: The hourly_fraud_trends analytics mart model violates data governance
rules by querying the intermediate layer table `int_transactions_enriched`
directly instead of using the marts layer. In the FROM clause at line 16,
replace the reference to `{{ ref('int_transactions_enriched') }}` with `{{
ref('fct_transactions') }}` to query the proper marts layer fact table. After
making this change, verify that the fct_transactions fact table includes all
required columns used by hourly_fraud_trends (specifically tx_hour, is_odd_hour,
and fraud_risk_score). If any of these columns are missing from
fct_transactions' explicit column selection list, add them to ensure the
downstream model has access to the data it needs.
In `@dbt/payment_dbt/models/marts/analytics/merchant_risk_profiling.sql`:
- Around line 1-72: The merchant_risk_profiling.sql model is missing a {{
config() }} block that specifies materialization settings. Add a config() block
at the very top of the file, before the variable declarations that start with
`{% set high_fraud_pct`, and set materialized to 'table' to ensure proper
performance for this analytics mart, also specify the schema as 'marts' and add
appropriate tags including 'marts', 'analytics', and 'risk' for organization and
tracking.
In `@dbt/payment_dbt/models/marts/analytics/velocity_anomaly_detection.sql`:
- Around line 75-95: The `is_critical` CASE statement is missing one of the
three conditions that define the `Critical` alert level in the
`velocity_alert_level` CASE statement. Specifically, the condition `(tx_count_1h
>= {{ tx_1h_high }} and tx_count_24h >= {{ tx_24h_crit_combo }})` is present in
the `velocity_alert_level` Critical branch but absent from the `is_critical`
when clause. Add this missing predicate condition to the `is_critical` CASE
statement's when clause to ensure all rows classified as Critical also have
`is_critical = true`.
In `@dbt/payment_dbt/models/marts/core/dim_merchants.sql`:
- Around line 13-16: The is_high_risk_category CASE statement in
dim_merchants.sql hardcodes merchant categories ('Travel', 'Electronics')
instead of using dbt variables, creating maintenance drift with
stg_transactions.sql which already uses {{ var('risky_merchant_categories') }}.
Add a new variable high_risk_merchant_categories to dbt_project.yml with the
value ['Travel', 'Electronics'], then update the CASE logic in dim_merchants.sql
to replace the hardcoded category list with {{
var('high_risk_merchant_categories') }} to ensure both models reference the same
centralized configuration.
In `@dbt/payment_dbt/models/marts/core/fct_transactions.sql`:
- Line 1: The fct_transactions model is missing a config() block at the
beginning of the file, which causes it to default to a view materialization
instead of a table, impacting query performance and preventing incremental
builds. Add a config() block before the "with enriched as" clause that specifies
materialized as table, schema as marts, unique_key as transaction_sk, and tags
as marts, core, and fact to ensure proper materialization and metadata
configuration according to the coding guidelines.
In `@dbt/payment_dbt/README.md`:
- Around line 1-9: The README.md file is missing required documentation
governance header fields. Add the missing fields to the existing table in the
dbt/payment_dbt/README.md file to include Version, Owner, Reviewer, and Version
History in addition to the existing Last updated and Validated baseline fields.
Follow the standard header pattern where these fields are presented in a
structured format, maintaining consistency with the coding guidelines for
narrative documentation.
In `@docs/kpi_definitions.md`:
- Around line 37-38: The KPI definitions for GMV (INR) and Success GMV (INR) are
identical duplicates with the same formula SUM(amount) WHERE status = 'success'.
Remove the Success GMV (INR) row entirely from the table in
docs/kpi_definitions.md, or if Success GMV should represent a different concept,
rename it and update its definition and formula to be semantically distinct from
the base GMV metric. Ensure only one measure exists for successful transaction
value to prevent confusion and drift across downstream reports.
In `@docs/README.md`:
- Line 32: The Baseline recalibration checklist in the README is incomplete.
When a major version refresh occurs with make refresh, the checklist must
include updating `problem_statement.md` along with `initial_observation.md` and
`kpi_definitions.md`. This is because `problem_statement.md` also locks the
baseline KPIs, so failing to update it during recalibration creates an
inconsistency across the documentation set. Update the text in the Baseline
recalibration row to include `problem_statement.md` in the list of files that
need to be updated.
In `@docs/staging_data_dictionary.md`:
- Around line 27-29: In the Slim facts section of the staging_data_dictionary.md
file, reword the description of hour columns to explicitly clarify that while
hour fields are used in staging and available for intermediate calculations and
joins in downstream transformations, they are not persisted or stored in the
final `fct_transactions` fact table. The current phrasing "Hour columns stay
here (and downstream marts)" creates ambiguity about whether they are
materialized in downstream marts; instead, clarify that they are present in
staging and used during mart transformation logic but are not part of the final
persisted `fct_transactions` schema, making the staging-to-mart contract
unambiguous.
In `@excel/generate_excel_report.py`:
- Around line 52-61: Remove the schema prefixes from all the SQL queries in the
SHEETS list definition. Since the search_path is already configured to include
marts, reporting, and public on line 86, you can simplify each SheetConfig by
replacing schema-prefixed table names (like reporting.cohort_analysis,
marts.daily_overview_kpis) with bare table names (like cohort_analysis,
daily_overview_kpis). This will reduce duplication and rely on the existing
search_path configuration for table resolution.
In `@generator/transaction_generator.py`:
- Line 14: The TARGET_FRAUD_RATE_PCT constant is set to 3.5, which exceeds the
coding guideline specification of ~2.5% for realistic fraud rates. This
discrepancy causes a 40% relative increase in fraud cases that skews downstream
analytics and model expectations. Reduce the TARGET_FRAUD_RATE_PCT value from
3.5 to approximately 2.5 to align with the realistic fraud rate guideline and
maintain data integrity for fraud model training.
- Line 163: The 'created_at' field in the metadata dictionary uses
datetime.now() which creates a timezone-naive timestamp, potentially causing
ambiguity. Update the datetime.now() call to datetime.now(timezone.utc) to
ensure consistent, timezone-aware timestamps across all metadata creation. Make
sure to import timezone from the datetime module if it is not already imported.
In `@ingestion/create_raw_tables.py`:
- Around line 83-85: The exception handler in the try-except block is using
logger.error which does not automatically capture the full exception traceback.
Replace the logger.error call with logger.exception to automatically include the
exception context and traceback information. The logger.exception method will
provide complete debugging information without requiring manual extraction of
the exception details.
In `@ingestion/db_utils.py`:
- Around line 42-46: In the exception handler where the database error is caught
as Exception, replace the logger.error call with logger.exception to
automatically capture and include the full exception traceback in the log
output. This will significantly improve debuggability for connection and
transaction failures without requiring manual traceback inclusion. The
logger.exception method should be called with the same error message format, but
it will automatically append the traceback information.
In `@ingestion/load_to_postgres.py`:
- Around line 92-94: The exception handler in the data loading failure path uses
logger.error which does not automatically capture the full exception traceback.
Replace the logger.error call with logger.exception to automatically include the
complete exception context and traceback, which will provide better debugging
information when data loading fails. Keep the formatted message about the
table_name and error, as logger.exception will append the full traceback
automatically.
- Around line 29-45: The pd.to_datetime call in the incremental load logic does
not explicitly specify timezone handling, which creates a fragile assumption
that all input timestamps will be naive. If timezone-aware timestamps are
provided in the CSV, the comparison with the naive watermark timestamp will fail
with a TypeError. Either add explicit timezone handling by passing utc=False to
pd.to_datetime and add validation logic to reject timezone-aware input strings,
or alternatively use utc=True with documentation explaining why UTC enforcement
is chosen. Ensure the chosen approach is clearly documented to prevent future
timezone-related bugs when comparing timestamps on line 40.
In `@ingestion/reset_raw.py`:
- Around line 12-17: The reset_raw_tables() function executes two TRUNCATE
operations without explicit error handling, making it difficult to debug if
either operation fails. Wrap the cur.execute calls in a try/except block that
catches database exceptions and logs the specific error details including which
TRUNCATE operation failed and the exception message. This will provide better
context for troubleshooting permission issues, table locks, or other failures
during the truncation process.
In `@ingestion/watermark.py`:
- Around line 27-56: The update_watermark function uses timezone-naive
datetime.now() calls for audit timestamps, which can lead to inconsistent
timestamp storage depending on server timezone. Replace both datetime.now()
calls (one in the if block when cur is provided, and one in the else block when
creating a new connection) with datetime.now(timezone.utc) to ensure consistent
UTC-based audit timestamps regardless of server timezone configuration. Make
sure to import timezone from the datetime module if not already imported.
In `@Makefile`:
- Line 135: The `all` target on line 135 does not depend on the database startup
task (typically `up`), so it fails on a clean environment when `setup-db` runs
because the Postgres container is not yet running. Add the database startup
target (likely `up`) as a dependency to the `all` target, placing it before
`setup-db` in the dependency list to ensure the Postgres container is running
before any database setup tasks execute.
In `@notebooks/eda_transactional.ipynb`:
- Around line 19-24: Remove the execution metadata block to ensure committed
notebooks are reproducible. In notebooks/eda_transactional.ipynb at lines 19-24,
delete the entire execution object containing iopub.execute_input,
iopub.status.busy, iopub.status.idle, and shell.execute_reply timestamp fields.
Apply the same change to notebooks/eda_fraud_risk.ipynb at lines 19-24, removing
the identical execution metadata block. These run-specific timestamps should not
be committed as they create unnecessary diff churn and violate reproducibility
guidelines.
- Around line 295-306: The code at line 296 filters sr to only include payment
methods with at least 100 transactions, which can result in an empty DataFrame
for smaller or filtered datasets. When sr is empty, rates_pct becomes an empty
array, causing the calls to rates_pct.min() and rates_pct.max() on lines 305-306
to fail and break notebook execution. Add a guard condition after the filtering
on line 296 to check if sr is not empty before proceeding with the visualization
code (from the figure creation at line 303 through the axis limit setting at
line 306). If sr is empty, either skip the visualization or provide appropriate
handling to gracefully handle smaller datasets.
In `@notebooks/utils/db_connector.py`:
- Around line 8-28: Add a docstring to the get_db_connection() context manager
function that clearly documents its read-only nature and explicitly states that
it never commits changes. The docstring should explain that this context manager
is designed for SELECT queries only and warn against using it for write
operations (INSERT, UPDATE, DELETE) to prevent data consistency issues in the
future.
In `@requirements.txt`:
- Line 4: Upgrade the python-dotenv dependency in requirements.txt from 1.0.1 to
version 1.2.2 or higher to patch CVE-2026-28684, and locate the pytest
dependency in requirements.txt and upgrade it from 8.2.2 to version 9.0.3 or
higher to patch CVE-2025-71176. Both upgrades must be completed before release
to ensure no known vulnerabilities are shipped.
In `@sql/cohort_analysis.sql`:
- Around line 1-21: Remove the schema prefixes from all five SQL view files to
comply with the coding guideline that requires bare table names. In
sql/cohort_analysis.sql (lines 1-21), change the FROM clause from `FROM
marts.cohort_analysis` to `FROM cohort_analysis`. In
sql/fraud_customer_segments.sql (lines 1-14), change `FROM
marts.fraud_customer_segments` to `FROM fraud_customer_segments`. In
sql/hourly_fraud_trends.sql (lines 1-14), change `FROM
marts.hourly_fraud_trends` to `FROM hourly_fraud_trends`. In
sql/merchant_risk_profiling.sql (lines 1-25), change `FROM
marts.merchant_risk_profiling` to `FROM merchant_risk_profiling`. In
sql/velocity_anomaly_detection.sql (lines 1-26), change `FROM
marts.velocity_anomaly_detection` to `FROM velocity_anomaly_detection`. The
deployment script will set the search_path to allow these views to resolve bare
table names to the marts schema at runtime.
In `@sql/deploy_views.py`:
- Around line 36-44: The view deployment script does not set the database
search_path before executing the view definitions, which can cause bare table
names in the SQL view files to fail resolution. After the cur.execute() call
that creates the reporting schema and its associated logger.info() call, add a
cur.execute() call to set the search_path to the correct schema order (marts,
reporting, public) before entering the loop that iterates through VIEW_FILES and
executes the view SQL definitions.
---
Outside diff comments:
In `@dbt/payment_dbt/models/marts/analytics/daily_merchant_kpis.sql`:
- Around line 1-36: The daily_merchant_kpis.sql model is missing a config block
that specifies materialization settings required for analytics marts. Add a
config block at the very top of the file before the CTE definitions (before the
"with fct_transactions as" statement) that sets the materialized property to
'table', specifies the schema as 'marts', and includes appropriate tags like
'marts', 'analytics', and 'kpi' to follow the coding guidelines for mart models.
In `@dbt/payment_dbt/models/marts/analytics/daily_overview_kpis.sql`:
- Around line 1-24: The daily_overview_kpis model is missing a dbt config()
block at the top of the file, which causes it to default to a view
materialization instead of a table. Add a config() block before the
fct_transactions CTE definition that specifies materialized='table',
schema='marts', and appropriate tags like 'marts', 'analytics', and 'kpi' to
ensure this analytics mart is properly materialized as a table for optimal
reporting performance.
In `@dbt/payment_dbt/models/marts/analytics/fraud_analysis_mart.sql`:
- Around line 1-33: The fraud_analysis_mart model is missing a config() block at
the top of the file, causing it to default to a view materialization instead of
the required table materialization for analytics marts. Add a config() block at
the very top of the fraud_analysis_mart.sql file (before the with
fct_transactions CTE) that specifies materialized='table', sets the schema to
'marts', and includes tags for 'marts', 'analytics', and 'fraud' for proper
categorization and performance optimization of reporting queries.
In `@dbt/payment_dbt/models/marts/core/dim_merchants.sql`:
- Around line 1-19: Create _schema.yml files for each of the 8 mart models to
satisfy the coding guidelines requiring descriptions and tests. For
dbt/payment_dbt/models/marts/core/dim_merchants.sql: add schema entry with
description and tests for merchant_sk (unique and not_null constraints) plus
is_high_risk_category (accepted_values test). For
dbt/payment_dbt/models/marts/core/fct_transactions.sql: add schema entry with
description, tests for transaction_sk (unique and not_null), and relationship
tests linking user_sk and merchant_sk to their respective dimension tables. For
dbt/payment_dbt/models/marts/analytics/daily_overview_kpis.sql: add schema entry
with description, tests for day column (unique and not_null), and range
validation for KPI metric columns. For
dbt/payment_dbt/models/marts/analytics/daily_merchant_kpis.sql: add schema entry
with description, composite uniqueness test for merchant_sk and day combination,
and relationship test for merchant_sk referencing the dimension table. For
dbt/payment_dbt/models/marts/analytics/fraud_analysis_mart.sql: add schema entry
with description, accepted_values test for fraud_risk_level, and range
constraint (0-100) for fraud_risk_score column. For
dbt/payment_dbt/models/marts/analytics/hourly_fraud_trends.sql: add schema entry
with description, composite uniqueness test for the tx_hour, merchant_category,
and payment_method combination, and range validation for fraud_rate_pct values.
For dbt/payment_dbt/models/marts/analytics/fraud_customer_segments.sql: add
schema entry with description, tests for user_id (unique and not_null), and
accepted_values tests for all segment classification columns. For
dbt/payment_dbt/models/marts/analytics/merchant_risk_profiling.sql: add schema
entry with description, tests for merchant_id (unique and not_null),
accepted_values for merchant_risk_category, and range checks (0-100) for
fraud_rate_pct.
In `@dbt/payment_dbt/tests/assert_fraud_score_range.sql`:
- Around line 1-4: The custom test assert_fraud_score_range.sql is redundant
because the intermediate/_schema.yml already contains a dbt_utils.accepted_range
test that enforces the same fraud_risk_score constraint between 0 and 100.
Remove the entire assert_fraud_score_range.sql test file to eliminate duplicate
test coverage and reduce unnecessary test execution time.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3f32ed99-634d-4ca4-b2dd-517ec60d8e35
⛔ Files ignored due to path filters (27)
dashboard/screenshots/01_Executive_Summary.pngis excluded by!**/*.pngdashboard/screenshots/02_Transaction_Overview.pngis excluded by!**/*.pngdashboard/screenshots/03_Fraud_Risk_Monitoring.pngis excluded by!**/*.pngdashboard/screenshots/04_Merchant_Risk.pngis excluded by!**/*.pngdashboard/screenshots/05_Customer_Analytics.pngis excluded by!**/*.pngdashboard/screenshots/06_Operations_Alerts.pngis excluded by!**/*.pngdashboard/screenshots/07_Recommendations.pngis excluded by!**/*.pngdashboard/screenshots/model_relationships.pngis excluded by!**/*.pngdashboard/screenshots/payment_dbt_lineage.pngis excluded by!**/*.pngnotebooks/charts/customer_segments/cohort_retention_matrix.pngis excluded by!**/*.pngnotebooks/charts/customer_segments/segment_users_fraud_loss_share.pngis excluded by!**/*.pngnotebooks/charts/fraud_risk/fraud_heatmap_category_payment.pngis excluded by!**/*.pngnotebooks/charts/fraud_risk/fraud_heatmap_hour_dow.pngis excluded by!**/*.pngnotebooks/charts/fraud_risk/fraud_loss_by_amount_bucket.pngis excluded by!**/*.pngnotebooks/charts/fraud_risk/monthly_fraud_loss_inr.pngis excluded by!**/*.pngnotebooks/charts/fraud_risk/monthly_fraud_rate.pngis excluded by!**/*.pngnotebooks/charts/transactional/daily_success_rate.pngis excluded by!**/*.pngnotebooks/charts/transactional/fail_decline_rate_by_payment_method.pngis excluded by!**/*.pngnotebooks/charts/transactional/monthly_attempt_volume_vs_success_gmv.pngis excluded by!**/*.pngnotebooks/charts/transactional/monthly_atv.pngis excluded by!**/*.pngnotebooks/charts/transactional/payment_method_mix_over_time.pngis excluded by!**/*.pngnotebooks/charts/transactional/status_mix.pngis excluded by!**/*.pngnotebooks/charts/transactional/success_rate_by_payment_method.pngis excluded by!**/*.pngnotebooks/charts/warehouse/merchant_risk_scatter.pngis excluded by!**/*.pngnotebooks/charts/warehouse/risk_score_calibration.pngis excluded by!**/*.pngnotebooks/charts/warehouse/top_fraud_merchants.pngis excluded by!**/*.pngnotebooks/charts/warehouse/velocity_fraud_uplift.pngis excluded by!**/*.png
📒 Files selected for processing (64)
.coderabbit.yaml.env.example.gitignoreMakefiledbt/payment_dbt/.gitignoredbt/payment_dbt/.user.ymldbt/payment_dbt/README.mddbt/payment_dbt/dbt_project.ymldbt/payment_dbt/macros/fraud_scoring.sqldbt/payment_dbt/macros/generate_schema_name.sqldbt/payment_dbt/macros/generate_surrogate_key.sqldbt/payment_dbt/models/intermediate/_schema.ymldbt/payment_dbt/models/intermediate/int_transactions_enriched.sqldbt/payment_dbt/models/intermediate/int_user_metrics.sqldbt/payment_dbt/models/marts/analytics/cohort_analysis.sqldbt/payment_dbt/models/marts/analytics/daily_merchant_kpis.sqldbt/payment_dbt/models/marts/analytics/daily_overview_kpis.sqldbt/payment_dbt/models/marts/analytics/fraud_analysis_mart.sqldbt/payment_dbt/models/marts/analytics/fraud_customer_segments.sqldbt/payment_dbt/models/marts/analytics/hourly_fraud_trends.sqldbt/payment_dbt/models/marts/analytics/merchant_risk_profiling.sqldbt/payment_dbt/models/marts/analytics/velocity_anomaly_detection.sqldbt/payment_dbt/models/marts/core/dim_merchants.sqldbt/payment_dbt/models/marts/core/dim_users.sqldbt/payment_dbt/models/marts/core/fct_transactions.sqldbt/payment_dbt/models/staging/_schema.ymldbt/payment_dbt/models/staging/_sources.ymldbt/payment_dbt/models/staging/stg_transactions.sqldbt/payment_dbt/package-lock.ymldbt/payment_dbt/packages.ymldbt/payment_dbt/profiles.ymldbt/payment_dbt/tests/assert_fraud_score_range.sqldocker-compose.ymldocs/README.mddocs/initial_observation.mddocs/intermediate_data_dictionary.mddocs/kpi_definitions.mddocs/marts_data_dictionary.mddocs/problem_statement.mddocs/raw_data_dictionary.mddocs/staging_data_dictionary.mdexcel/generate_excel_report.pygenerator/transaction_generator.pyingestion/create_raw_tables.pyingestion/db_utils.pyingestion/load_to_postgres.pyingestion/reset_raw.pyingestion/watermark.pynotebooks/eda_customer_segments.ipynbnotebooks/eda_fraud_risk.ipynbnotebooks/eda_transactional.ipynbnotebooks/eda_warehouse.ipynbnotebooks/utils/config.pynotebooks/utils/data_loader.pynotebooks/utils/db_connector.pynotebooks/utils/style.pynotebooks/utils/visualization.pyrequirements.txtsql/cohort_analysis.sqlsql/deploy_views.pysql/fraud_customer_segments.sqlsql/hourly_fraud_trends.sqlsql/merchant_risk_profiling.sqlsql/velocity_anomaly_detection.sql
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@tests/conftest.py`:
- Around line 57-58: The nested with statements for get_db_connection() and
conn.cursor() should be combined into a single with statement. Replace the two
separate with statements with a single with statement that lists both context
managers separated by a comma, making the code more concise and following Python
best practices.
- Around line 48-61: The db_cursor fixture in tests/conftest.py has a docstring
claiming "Read-only database cursor" but does not actually enforce read-only
mode. The connection from get_db_connection() commits on success, so writes
would persist. Either enforce read-only mode by configuring the connection
object to use read-only transaction isolation level or read-only mode before
yielding the cursor, or remove the "Read-only" claim from the docstring if
read-only enforcement is not required. Choose based on the intended test
requirements.
In `@tests/test_ingestion.py`:
- Around line 65-68: In the test_csv_fraud_rate_within_generator_tolerance
function, replace the hardcoded assertion bounds of 3.0 and 4.0 with values
dynamically derived from the generator's configuration constants. Import the
TARGET_FRAUD_RATE_PCT and TARGET_FRAUD_TOLERANCE_PCT constants from the
generator module, then calculate the lower bound as TARGET_FRAUD_RATE_PCT minus
TARGET_FRAUD_TOLERANCE_PCT and the upper bound as TARGET_FRAUD_RATE_PCT plus
TARGET_FRAUD_TOLERANCE_PCT. This ensures the test validates that the generated
fraud rate falls within the generator's intended tolerance range of [3.25, 3.75]
rather than the wider [3.0, 4.0] range.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ff096756-6698-4ed3-bc65-7e83935ce05a
📒 Files selected for processing (5)
Makefilepytest.initests/conftest.pytests/constants.pytests/test_ingestion.py
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
1 participant
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.
#6