#97 fix(dbt): historical equity rate as-of join + period test (#92)

[Anonymous]

Originally created by: grynn-in

dbt-side fix for [#92] finding #2 (critical). (konsol-side findings #1/#4/#5/#6 are in companion PR grynn-in/konsol#48.)

Bug

gold_consolidated_trial_balance looked up the historical equity rate with row_number() … order by rate_date desc + rn = 1 — the most-recent tranche ever, ignoring period_date. Early periods got a future rate. The autoname carries rate_date precisely to allow multiple tranches, but only the latest was ever used.

Fix

Replace the rn=1 subquery with an ASOF LEFT JOIN on etb.period_date >= hr.rate_date — mirroring the ownership asof left join already used a few lines above. ASOF picks the closest rate_date not exceeding period_date, and yields NULL (→ closing_rate fallback via the existing case) for periods before the first tranche, instead of silently borrowing a future rate.

Test

New tests/assert_equity_historical_rate_is_asof.sql — fails if a row applied a historical rate with no as-of-eligible tranche (the exact pre-fix bug) or if the applied rate ≠ the as-of expected rate (argMaxIf over tranches with rate_date <= period_date). The existing assert_equity_uses_historical_rate only checks that a rate was used, not the correct-period one.

Verification

• ASOF pattern is identical to the proven ownership_staging join in the same model.
• ⚠️ Full dbt build/test not run from the isolated worktree (live dbt project is the mounted one); should run in CI. epm_staging.historical_equity_rates is empty in the current demo, so this is behaviour-correct on existing data and exercised once tranches exist.

Refs grynn-in/konsolidat#92

🤖 Generated with Claude Code

[Anonymous]

Originally posted by: grynn-in

Code review (×2 independent passes)

Review (ClickHouse ASOF / dbt): verified on the live CH 24.8:

• ✅ Two ASOF joins in one SELECT (ownership + historical) — works on 24.8 (headline risk cleared).
• ✅ ASOF rule (single inequality, last) + direction (>= picks latest tranche ≤ period) — correct, matches the proven ownership pattern.
• ✅ argMaxIf and the build_date_from_year_period macro call — valid; reconstructing period_date in the test is the right call (model doesn't expose it).
• 🔴 CRITICAL finding: ASOF LEFT JOIN miss on the non-nullable toFloat64(historical_rate) returns 0.0, not NULL → hr.historical_rate is not null was always true → a period before the first tranche would translate equity at rate 0 (zeroing the balance) instead of closing_rate. Same defaulting trap the ownership block guards against.

Fix applied (66f734e): cast the rate to Nullable(Float64) so an ASOF miss yields a true NULL — verified on CH 24.8 (miss→\N, match→value). This makes the case fallback, the output column, and the test's is not null filter all correct.

Both ASOF-correctness and the defaulting bug are resolved. ⚠️ Full dbt build still pending CI (can't run against the live mounted project from the isolated worktree).

[Anonymous]

Ticket changed by: grynn-in

• status: open --> closed