[OLMIS-8265] Fill schedule_type and make the trend charts cadence-aware - #4
[OLMIS-8265] Fill schedule_type and make the trend charts cadence-aware#4denys1204 wants to merge 9 commits into
Conversation
schedule_type was added to mart_stock_status, an incremental model, so ClickHouse created the column and left every existing row at the String default. Only rows written after that deploy carry a real value. The accepted_values test on the column therefore fails, and because dbt build skips whatever sits downstream of a failing test, the marts built from this one stop refreshing. A --full-refresh would recompute the column, but it would also re-apply the model's retention window to history that is already materialised and drop whatever now falls outside it. On Malawi dev that is 583,446 rows. Updating the column in place cannot lose one: schedule_type is not part of the sort key, so ClickHouse permits the mutation, and _cdc_ts is untouched, so the incremental watermark survives. The value comes from period_start_date and period_end_date, both already on the mart, through the same schedule_type macro the model applies to new rows, so the result is what a rebuild would have written. Idempotent, synchronous, and it raises rather than reporting success if any row is left behind. bash scripts/dbt/run.sh run-operation backfill_schedule_type Verified by blanking 1087 of 5068 rows on a local stack: all refilled, row count unchanged, zero mismatches against a fresh recompute, second run a no-op.
mart_logistics_summary is select s.* off mart_stock_status, so the physical table carries every column the stock mart has, while the Superset dataset only declared twelve of them. Native filters resolve against the dataset rather than the table, so the Schedule Type filter was silently dropped for the Logistics Summary Report chart, with rejected_filters: not_in_datasource. Picking a Schedule Type on the Consumption dashboard narrowed five charts and left that one showing every cadence, with nothing on screen to say so. Scope worth stating plainly for review: this repairs that one filter. The charts that group by cadence read mart_stock_status, which has declared the column since it was introduced. The wildcard select means any column added upstream in future will be invisible here the same way. Narrowing the mart to an explicit list would close that, but it changes the mart's shape and belongs in its own change.
Every trend chart fixes its time grain in its own config, so the bucket size is whatever the chart author chose and a reader cannot change it. That is a poor fit for a dashboard whose programmes report on different frequencies: comparing one cadence against another needs a bucket both of them fill, while looking at a single cadence wants the finest bucket that cadence produces. No single default serves both readings. This adds the switch so the reader picks. It also matters beyond this deployment, since adopters fork this package and will have their own mix of reporting frequencies and their own charts. Verified against a running Superset rather than written from the documentation. The block round-trips through the asset import into both dashboards, and the override reaches the query: P3M renders toStartOfQuarter and P1M renders toStartOfMonth over the same dataset.
Stockout Rate Over Time and Consumption Trend aggregated every programme into a single series. That is where mixed reporting frequencies bite on these two. There are no gaps to see, because some programme reports in every month, but the composition of the aggregate changes from month to month: programmes reporting quarterly only contribute in the months that close a quarter. The line looks continuous and steps every third month, and a reader has no way to tell why. Splitting the series by schedule_type leaves each one internally consistent and labelled. Nothing is interpolated and no value is invented, which is what ruled out resampling with a forward fill; that would also have changed nothing here, since these series already have a point in every month. Both keep a monthly axis, which is the right default once the frequencies are no longer averaged together. Legends are turned on, since each chart now carries more than one series. Tooltips move to smart_date because the grain is switchable per view, and a fixed month format would label a quarter by the month it happens to start in.
Months of Stock Over Time and Stock Status Distribution Over Time cannot be treated the way the pooled trend charts are. Their series dimension is already spent, on district and on stock status, so adding reporting frequency to it would multiply rather than clarify: twenty nine districts against two frequencies is fifty eight lines, and a stacked bar broken down twice stops meaning anything. Changing the bucket solves the same problem from the other side. In a quarter a monthly reporter contributes three observations and a quarterly reporter contributes one, so both land in every bucket and neither leaves the holes this ticket describes. Nothing is interpolated. That the two pairs end up with different defaults is deliberate. A chart that can separate frequencies into series keeps the finer bucket; a chart that cannot moves to the bucket where frequencies stop colliding. Axis titles stay as Period rather than Quarter, because the grain is not fixed from the reader's point of view. Tooltips use smart_date for the same reason.
| filterState: | ||
| value: null | ||
| ownState: {} | ||
| - id: NATIVE_FILTER-time-grain |
There was a problem hiding this comment.
Time Grain is scoped to ROOT_ID, so it re-grains every time-series chart, including consumption_current_year, consumption_last_year and consumption_per_district. Selecting quarter breaks the month-by-month comparisons these charts are intended for.
Can we scope it only to the intended trend chart(s)?
There was a problem hiding this comment.
Done. The filter is now scoped to Consumption Trend only; the month-by-month charts are excluded and keep their fixed axis.
One thing worth knowing about the mechanics: scoping lives in scope.excluded as slice ids, and the importer remaps those through the chartId/uuid pairs of the position tree. Our exports had chartId: 0 on every chart, which collapses that mapping into a single entry, so the charts on both dashboards now carry distinct placeholder ids. I verified the round-trip on a local stack import: the excluded list arrives remapped to the target environment's real slice ids.
Two caveats that come with exclude-based scoping: ids that fail to resolve are dropped silently (the filter just widens back), and any chart added to the dashboard later is in scope by default. Worth a quick look at the filter scope after each deploy.
| filterState: | ||
| value: null | ||
| ownState: {} | ||
| - id: NATIVE_FILTER-time-grain |
There was a problem hiding this comment.
Same ROOT_ID scope here: quarter also re-grains stockout_rate_over_time, while month reintroduces the quarterly-reporter gaps this PR addresses.
Is this month ↔ quarter switch intentional?
There was a problem hiding this comment.
Intentional for Months of Stock and Stock Status Distribution: quarter is the default because it is complete for both cadences, and the filter is the way back to monthly resolution once a single cadence is selected. Not intentional for Stockout Rate, that was the same over-broad scope as on Consumption. The filter is now scoped to the two quarter-bucketed charts only, and Stockout Rate keeps its fixed monthly axis with one series per cadence. The filter description also spells out what the month view does to the two charts that remain in scope: the bars dip because quarterly facilities drop out of the count, and the averages are computed over the remaining reporters only.
| viz_type: echarts_timeseries_bar | ||
| x_axis: period_end_date | ||
| time_grain_sqla: P1M | ||
| time_grain_sqla: P3M |
There was a problem hiding this comment.
Quarterly COUNT(DISTINCT facility) changes what the bars measure. A facility with a one-month stockout counts the same as one that is understocked for all three months, so quarterly bars aren't comparable with the monthly view.
Is this intended? If so, worth noting that the metric now represents facilities that had this status at any point during the quarter.
There was a problem hiding this comment.
Intended, and you are right it needed saying. The chart description now spells it out: within a bucket a facility counts once per status it held at any point in it, a one-month stockout weighs the same as a three-month one, and a facility that changed status (or has products sitting in different categories) lands in more than one segment, so stacked totals can exceed the number of reporting facilities.
I did look at making the quarterly count comparable instead of documenting it. Dividing by the months in the bucket breaks as soon as someone flips the grain filter, and a quarter-end snapshot needs SQL in the mart, which turns a chart-only change into a dbt deploy. If strict comparability matters to the PMs, I would rather track the snapshot as its own ticket.
| groupby: | ||
| - schedule_type | ||
| adhoc_filters: [] | ||
| time_range: "Last 6 months" |
There was a problem hiding this comment.
Last 6 months leaves the quarterly series with only ~1–2 points. Also, stockout_rate_over_time has no time_range, so the two pooled trend charts cover different windows.
Maybe it's worth aligning the time ranges?
There was a problem hiding this comment.
Aligned. Both pooled trend charts now use Last 3 years: Consumption Trend grows from six months, and Stockout Rate gets an explicit window instead of the unbounded default, which was effectively rendering the same three years the mart retains anyway. Three years also matches what the Orders charts already use. Expect a denser Consumption Trend line after this.
| y_axis_format: ",.0f" | ||
| rich_tooltip: true | ||
| tooltipTimeFormat: "%b %Y" | ||
| tooltipTimeFormat: smart_date |
There was a problem hiding this comment.
tooltipTimeFormat changed from %b %Y to smart_date. On the monthly charts, this removes the guaranteed Jan 2024-style format.
Is it intentional?
There was a problem hiding this comment.
Split by what the grain does now. Stockout Rate is out of the Time Grain filter's scope, so its grain is fixed monthly again and it is back on %b %Y. The three charts that stay grain-switchable keep smart_date, because a fixed %b %Y stamps a quarter with its opening month: Q1 2025 renders as Jan 2025. I checked the 6.1.0 formatter code rather than guessing: in tooltips smart_date resolves to the verbose variant, which prints the same Mar 2025 style as %b %Y for monthly points, with one wart, points on a year boundary show a bare 2025. Accepting that wart seemed better than mislabelling quarters.
On Consumption the filter now re-grains only the Consumption Trend chart; the month-by-month comparison charts keep their fixed axis. On Stock Status it re-grains only the two quarter-bucketed charts, so it no longer touches Stockout Rate Over Time. Scoping works through scope.excluded, which holds slice ids that the importer remaps via the position tree's chartId/uuid pairs. The zero placeholders this repo used so far collapse that mapping, so the charts on both dashboards now carry distinct placeholder ids. Verified by a full import round-trip: the excluded lists arrive remapped to the target environment's real slice ids, nothing is dropped. Unresolvable excluded ids are dropped silently on import (the filter would quietly widen), so keep every chart of these dashboards in the bundle and re-check the scope after deployment.
A series with one point per quarter never has two adjacent monthly buckets, so with markers disabled the chart drew neither line segments nor symbols: the quarterly cadence was invisible. Markers make it show up as one dot per quarter. The window also grows from six months to three years, matching the stockout trend and the retention of the mart, so a quarterly series has enough points to read. The description now notes that a quarterly point carries a whole quarter's total and is therefore not directly comparable in level to the monthly series; switching the Time Grain filter to quarter is the way to compare levels.
…window The chart is excluded from the Time Grain filter, so its grain is fixed monthly again and the tooltip can go back to the explicit month-year format instead of smart_date. An explicit three-year window replaces the unbounded default so both pooled trend charts cover the same range. The description no longer points at a filter that does not target this chart.
In a quarter bucket a facility counts once per status it held at any point in the quarter, so a one-month stockout weighs the same as a three-month one, and a facility that changed status, or whose products sit in different categories, lands in more than one segment. Spelled out in the chart description so stacked totals exceeding the facility count read as intended behaviour, not a bug.
2 participants
Ticket: https://openlmis.atlassian.net/browse/OLMIS-8265
Follows the QA round on openlmis-reporting#2. Two things came out of it: the Schedule Type filter had nothing to filter on, and the trend charts named in the ticket still put every reporting frequency on one monthly axis.
The blank column
schedule_typewas added tomart_stock_status, which is incremental, so ClickHouse created the column and left every existing row at the String default. Only rows written after that deploy got a value. On Malawi dev that was 8 rows out of 9,430,378.That also stalled the pipeline. The
accepted_valuestest on the column fails on an empty string, anddbt buildskips whatever sits downstream of a failing test, so the marts built from this one stopped refreshing.mart_malawi_stock_fullwas a month behind on CDC before this was found.The obvious fix is
--full-refresh, and it is the wrong one. A rebuild re-applies the model's retention window to history that is already materialised and drops whatever now falls outside it, which on Malawi dev is 583,446 rows. So this adds a run-operation that updates the column in place instead. It cannot lose a row:schedule_typeis not part of the sort key, so ClickHouse permits the mutation, and_cdc_tsis untouched, so the incremental watermark survives.Already run on Malawi dev. Column filled (Monthly 9,289,441 / Weekly 140,937), row count unchanged, zero mismatches against a fresh recompute, and the next build came back
PASS=156 ERROR=0 SKIP=0with the downstream watermarks caught up.The trend charts
The four charts the ticket names do not all have the same problem, so they do not get the same fix.
Stockout Rate Over TimeandConsumption Trendpool every programme into one series. There are no gaps to see, because something reports in every month, but the composition of that aggregate changes month to month: quarterly reporters only contribute in the months that close a quarter. The line looks continuous and steps every third month. These two now split their series byschedule_typeand keep a monthly axis. Nothing is interpolated, which is what ruled out resampling with a forward fill; that would also have changed nothing here, since these series already have a point in every month.Months of Stock Over TimeandStock Status Distribution Over Timecannot be treated that way. Their series dimension is already spent, on district and on stock status. Twenty nine districts against two frequencies is fifty eight lines, and a stacked bar broken down twice stops meaning anything. These two change the bucket instead: in a quarter a monthly reporter contributes three observations and a quarterly one contributes one, so both land in every bucket and neither leaves holes.The two pairs ending up with different defaults is deliberate. A chart that can separate frequencies into series keeps the finer bucket; a chart that cannot moves to the bucket where frequencies stop colliding. A Time Grain filter on both dashboards lets the reader switch either way, which also matters for adopters forking this package with their own mix of frequencies and their own charts.
Also here
mart_logistics_summaryisselect s.*offmart_stock_status, so the physical table has every column the stock mart does, while the Superset dataset declared twelve of them. Native filters resolve against the dataset, not the table, so Schedule Type was being dropped for the Logistics Summary Report chart withrejected_filters: not_in_datasource. Picking a cadence on the Consumption dashboard narrowed five charts and left that one showing everything, with nothing on screen to say so. Registering the column fixes that one filter; it is not what enables the grouping on the trend charts.Deploying
Everything except the run-operation is chart and dashboard YAML, so a Jenkins job with the reporting stack covers it. Run the backfill before importing the assets, otherwise the cadence split renders a series with an empty name.
Verified
Local stack, against a real build:
filter_timegrainround-trips through the asset import into both dashboards, and the override reaches the query: P3M givestoStartOfQuarter, P1M givestoStartOfMonthWHERE schedule_type IN (...)withapplied_filtersset andrejected_filtersemptyOn Malawi dev, splitting by cadence surfaces something the pooled line was hiding: EPI reports weekly and sits around 94% stockout against roughly 73% for the monthly programmes, and it disappears into the average because it is about 1% of rows.
Not in here
Malawi has no quarterly programmes, only monthly and weekly, so the specific scenario in the ticket description cannot be demonstrated there. UAT has both but its mart is empty for unrelated reasons. The cadence handling is exercised on monthly against weekly instead.
mart_adjustments,mart_reporting_statusandmart_non_reporting_facilitiescarry the samenow()-relative window that keepsmart_adjustmentsempty on UAT. Left alone here since touching them turns this into a mart change with a different deploy path.