Menu ▾ ▴

#115 feat(dbt): opt-in scope + period filters for orchestrator closes

closed
nobody
None
2026-06-29
2026-06-28
Anonymous
No

Originally created by: grynn-in

What

Make the konsol-exec Execute plane's Scope and Fiscal Year/Period launch options actually filter the consolidation.

Two opt-in macros in macros/orchestrator_filters.sql, applied at the consolidation chokepoint gold_consolidated_trial_balance.entity_tb:

  • scope_filter (entity_scope var) — consolidate one entity or one group. Resolves the code against gold_consolidation_hierarchy so a group expands to all descendant entities via the materialised path (e.g. GROUP_CORP/GROUP_EMEA/DEMF), while an entity data_area_id matches just itself.
  • period_filter (fiscal_year / fiscal_period vars) — single-period closes.

Both emit nothing when their var is unset → full builds are byte-for-byte unchanged. Applied at the chokepoint so every downstream consolidation model (fully-consolidated TB, cash flow, YTD, NCI movement) inherits the slice, while foundational gold_trial_balance stays complete.

Why

Previously the orchestrator mapped scope to dbt run --select <scope>, which selects graph nodes — an entity/group code matches no node, so it silently built nothing. And fiscal_year/fiscal_period rode as dbt vars but no model read them. This wires both into real predicates. Orchestrator side: scope → entity_scope var (konsol#60).

Verification (live, real D365 data)

--vars "{entity_scope: GROUP_EMEA, fiscal_year: 2023}" narrowed the consolidated TB from 13,483 rows (DEMF/JPMF/USMF, 23 yrs) to 95 (DEMF, 2023) — GBMF correctly absent (no GL in real D365). A no-var rebuild restored the table to 13,483 exactly (PASS=12 ERROR=0).

Note

Gold consolidation models are full-table materialisations, so a scoped/period run overwrites the consolidated tables with that slice (rebuild with no vars to restore full). True incremental single-period closes (insert/replace by period) are a follow-up.

🤖 Generated with Claude Code

Related

Tickets: #116
Tickets: #119
Tickets: #121

Discussion

  • Anonymous

    Anonymous - 2026-06-29

    Originally posted by: grynn-in

    Review — feat(dbt): opt-in scope + period filters for orchestrator closes

    Reviewed origin/feat/orchestrator-scope-period-filters against main in an isolated worktree. Both files land together (the macro file and the entity_tb references are both absent on main), so there is no broken-reference / arg-mismatch risk — the call sites and definitions are introduced atomically and they line up:

    • period_filter('tb.fiscal_year', 'tb.fiscal_period') ↔ period_filter(year_col, period_col) ✓
    • scope_filter('tb.data_area_id') ↔ scope_filter(data_area_col) ✓

    The core design is sound: both macros are genuinely opt-in (var(..., '') default → empty string → no predicate), placement is at the entity_tb chokepoint feeding rated/consolidated, and gold_trial_balance stays complete. Verified live by the author (13,483 → 95 rows; no-var rebuild restores 13,483).

    Blocking issues

    None that break the build or the verified flow.

    Non-blocking nits (recommend addressing)

    1. scope_filter interpolates entity_scope raw into SQL string literals — no quote-escaping — macros/orchestrator_filters.sql:31-36. entity_scope is placed directly inside '{{ s }}', unlike period_filter which is hardened via | int. This is an injection / query-manipulation vector if the scope source is ever user-controllable. Exploitability today is low (the var is set server-side by the konsol orchestrator and ClickHouse-HTTP runs a single statement), but the fix is a one-liner and removes the asymmetry with period_filter:
      jinja {%- set s = scope | string | trim | replace("'", "''") %}
      This also neutralises stray ' and is the right default before the scope code is ever sourced from free text. (The bare s would still allow %/_ to act as LIKE wildcards — see nit 4.)

    2. PR body + code comment overstate slice coverage — models/gold/gold_consolidated_trial_balance.sql:38-40 claims "every downstream consolidation model (fully-consolidated TB, cash flow, YTD, NCI) inherits the slice." That holds for models reading gold_consolidated_trial_balance (gold_fully_consolidated_tb, gold_nci_movement_schedule, gold_ic_*, gold_fx_revaluation, acquisition/disposal adjustments). It does not hold for gold_cash_flow_indirect and gold_ytd_trial_balance, which ref('gold_trial_balance') directly and therefore build full during a scoped/period close — the named "cash flow, YTD" examples are exactly the two that bypass the chokepoint. Either apply the filters in those two models too, or correct the comment/PR body so a single-period close isn't assumed to produce a sliced cash flow / YTD. Affects close-output consistency, not correctness of this model.

    3. period_filter silently coerces non-numeric vars to 0 — macros/orchestrator_filters.sql:20-21. 'abc' | int → 0, so a malformed fiscal_year/fiscal_period yields = 0 (empty result) rather than an error. Safe, but fails silently. Consider validating numeric input or documenting that these vars must be integers.

    4. scope_filter LIKE patterns don't escape % / _ — macros/orchestrator_filters.sql:34-36. If entity_scope ever contained a LIKE metacharacter it would match unexpectedly. Entity/group codes don't today; bundle with nit 1 if hardening.

    5. No tests for the macros — no unit/data test asserts (a) no-var build emits no predicate and (b) a GROUP code expands to its descendants. A small test would lock in the opt-in contract against future edits.

    6. Seed-fallback path is only 2-segment — gold_consolidation_hierarchy builds path as concat(consolidation_group,'/',data_area_id) in the seed branch; multi-level paths only come from staging. Scoping to a top-level group while the hierarchy is on the seed fallback would miss grandchildren via the path-LIKE branches (the consolidation_group = branch only catches direct members). Data-dependent, pre-existing property of the hierarchy model — noted for awareness.

    Verification

    • dbt parse --no-partial-parse --profiles-dir . (no vars): PASS (only pre-existing MissingArgumentsPropertyInGenericTestDeprecation warnings, unrelated to this PR).
    • Macro logic traced by hand for {entity_scope: GROUP_EMEA, fiscal_year: 2023, fiscal_period: 6} → valid where 1 = 1 and tb.fiscal_year = 2023 and tb.fiscal_period = 6 and tb.data_area_id in (select ...). dbt compile not run to completion (needs a live ClickHouse connection — not exercised to avoid touching prod). dbt run/build intentionally not run.
    • period_filter confirmed injection-proof via | int ('2023; DROP' | int → 0); scope_filter is the only raw-interpolation path.

    MERGE RECOMMENDATION

    MERGE-WITH-NITS — opt-in design is correct, parses clean, and is verified live; please add quote-escaping to scope_filter (nit 1) and fix the cash-flow/YTD coverage claim (nit 2) before/with merge.

     
  • Anonymous

    Anonymous - 2026-06-29

    Ticket changed by: grynn-in

    • status: open --> closed
     

Log in to post a comment.