Skip to content

Add Fresh Dashboard intermediate models and snapshots#3438

Open
anthonygwalters wants to merge 1 commit intomainfrom
claude/add-markdown-upload-mijLN
Open

Add Fresh Dashboard intermediate models and snapshots#3438
anthonygwalters wants to merge 1 commit intomainfrom
claude/add-markdown-upload-mijLN

Conversation

@anthonygwalters
Copy link
Copy Markdown
Member

Pull Request

Summary & Motivation

When merged, this pull request will add two new intermediate dbt models and their corresponding snapshots to support the Fresh Dashboard Tableau extract:

  • int_tableau__fresh_dashboard_enrollment_metrics: Aggregates enrollment data by school and academic year, calculating enrollment counts, targets, and distance-to-target metrics (both absolute and percentage)
  • int_tableau__fresh_dashboard_goal_metrics: Aggregates goal achievement data by school, grade level, and goal type, calculating goal values and tracking whether goals were met

These intermediate models serve as the foundation for Tableau dashboard visualizations and include snapshots to track changes over time.

Changes

New Models

  • int_tableau__fresh_dashboard_enrollment_metrics.sql (403 lines)

    • Sources from rpt_tableau__fresh_dashboard_progress_to_goals
    • Groups by academic year, org, region, school, and school level
    • Calculates enrollment metrics at school level (grade_level = -1) and school+grade level (grade_level != -1)
    • Computes distance to targets (seat, FDOS, budget, new student, re-enrollment) in both count and percentage formats
    • Includes school level sort field for proper ordering (ES=1, MS=2, HS=3)
  • int_tableau__fresh_dashboard_goal_metrics.sql (112 lines)

    • Sources from rpt_tableau__fresh_dashboard_aggregated
    • Groups by academic year, school, grade level, goal granularity, goal type, and enrollment type
    • Calculates goal achievement metrics with conditional formatting for counts vs. percentages
    • Determines met goal status based on goal type (Applications/Offers/Pending Offers vs. Conversion/Assigned School)
    • Includes display-friendly fields for dashboard consumption

Properties & Tests

  • Added comprehensive .yml property files for both models with:
    • Data type definitions for all columns
    • Unique combination tests to ensure data integrity
    • Materialized as tables for performance

Snapshots

  • Added snapshot_tableau__fresh_dashboard_goal_metrics tracking changes to met_goal and display_value
  • Added snapshot_tableau__fresh_dashboard_enrollment_metrics tracking changes to enrollment and distance-to-target metrics
  • Both use check strategy with dbt_valid_to_current set to '9999-12-31'

Self-review

  • Run trunk fmt on all modified files
  • Run uv run dagster definitions validate for any modified code location
  • Run uv run pytest tests/test_dagster_definitions.py for any modified code location
  • Include a [model_name].yml properties file for all new models
  • Include (or update) an exposure for all models consumed by a dashboard

https://claude.ai/code/session_019dpZt3HX2MFW1u9hVogaej

Pre-compute goal-level and school-level enrollment metrics from the
FRESH Dashboard Tableau calculated fields as two intermediate dbt models
(`int_tableau__fresh_dashboard_goal_metrics` and
`int_tableau__fresh_dashboard_enrollment_metrics`), and add SCD type 2
snapshots to track metric changes over the enrollment season.

https://claude.ai/code/session_019dpZt3HX2MFW1u9hVogaej
@anthonygwalters anthonygwalters requested a review from a team as a code owner March 13, 2026 20:25
@anthonygwalters anthonygwalters requested a review from a team March 13, 2026 20:25
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 13, 2026

Claude finished @anthonygwalters's task in 1m 59s —— View job


Code Review

  • Gather context and understand the request
  • Read changed files
  • Review SQL models
  • Review YAML property files
  • Review snapshot definitions
  • Check against project conventions and SQL style rules
  • Post review feedback

Overall the structure, naming, and YAML conventions are solid. However there are a few logic bugs in the enrollment metrics model and a couple of missing items that should be addressed before merge.

Bugs — int_tableau__fresh_dashboard_enrollment_metrics.sql

1. Unfiltered sum() for school-level (_s) budget distance (line 171)

sum(case when enrollment_type = 'All' and row_type = 'Student' and grade_level = -1 then student_count end)
- sum(budget_target) as budget_target_distance_s,

sum(budget_target) has no row_type, enrollment_type, or grade_level filter. Every other _s target sum uses a CASE WHEN ... and row_type = 'Goal' and grade_level = -1 guard. This will mix goal and non-goal rows (and school+grade rows) into the subtraction, producing an incorrect distance. Compare with seat_target_distance_s on line 125 which does filter correctly.

Same issue affects budget_target_distance_pct (line 323–334) where sum(budget_target) appears twice without filters.

2. Unfiltered target sums for school+grade (_sg) distance calculations (lines 220–260)

All five _sg distance calculations filter the student side to grade_level != -1 via CASE WHEN, but leave the target side completely unfiltered:

sum(case when enrollment_type = 'All' and row_type = 'Student' and grade_level != -1 then student_count end)
- sum(seat_target) as seat_target_distance_sg,   -- ← sums ALL seat_target rows including grade_level = -1

This means school-level target values (where grade_level = -1) are included in the _sg distance calculations, inflating the denominator. All five need a CASE WHEN grade_level != -1 THEN ... END guard on the target side, consistent with how _s calculations filter to grade_level = -1.


Missing item — Exposure not updated

The fresh_dashboard exposure in models/exposures/tableau.yml (line 740) currently references:

  • rpt_tableau__fresh_dashboard_aggregated
  • rpt_tableau__fresh_dashboard_progress_to_goals

The two new snapshot models (snapshot_tableau__fresh_dashboard_goal_metrics, snapshot_tableau__fresh_dashboard_enrollment_metrics) are the Tableau-facing outputs of this PR and need to be added to depends_on in the exposure. Per the project conventions, every external tool consuming dbt data requires an exposure listing all depends_on models. The PR checklist also flags this as incomplete.


Minor issues

school_level_sort has no ELSE clause (int_tableau__fresh_dashboard_enrollment_metrics.sql, line 15–19)

case school_level
    when 'ES' then 1
    when 'MS' then 2
    when 'HS' then 3
end as school_level_sort,

If school_level is NULL or an unexpected value, this silently returns NULL. Adding else null (explicit) or else 0 with a comment would make the intent clearer and prevent silent ordering surprises downstream.

budget_target_s column is absent

The four target reference columns exposed for _s are seat_target_s, fdos_target_s, new_student_target_s, re_enroll_projection_s. There is no budget_target_s column, yet budget_target_distance_s is computed from it. If Tableau needs to show "budget target" alongside "distance to budget target," the raw target value should be surfaced too. The properties YAML also omits it.

Redundant inner CASE WHEN in met_goal (int_tableau__fresh_dashboard_goal_metrics.sql, line 71–97)

Since the model is grouped by goal_type, a row is already filtered to a single goal type. The inner CASE WHEN goal_type in ('Applications', ...) THEN goal_value END inside the min() call is equivalent to min(goal_value) in context — the outer WHEN goal_type in (...) already guards the branch. Not a correctness issue, but simplifying would reduce noise.

Snapshot check_cols for enrollment_metrics is narrow

Only 5 of the ~20 metric columns are tracked in check_cols. Many important columns — all _sg enrollment counts, all _pct distances, all _sg distances — will not trigger a snapshot record when they change. If the intent is to capture changes to any enrollment or target metric, check_cols: all (as used in snapshots/people.yml) would be safer.


Self-review checklist

All items on the PR checklist are unchecked. Before merging, please confirm:

  • trunk fmt has been run on all files
  • uv run dagster definitions validate passes for kipptaf
  • uv run pytest tests/test_dagster_definitions.py::test_definitions_kipptaf passes
  • Exposure updated to reference the two new snapshots

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants