#64 Bug: period_net_amount sums magnitudes (debit+credit), not signed net (debit-credit) — breaks balance/movement models

[Anonymous]

Originally created by: pyy3

Summary

The base measure period_net_amount is defined as sum(accounting_currency_amount), and accounting_currency_amount is stored as a positive magnitude (the debit/credit direction lives in the separate is_credit flag → debit_amount/credit_amount). So period_net_amount computes gross transaction volume (debit + credit), not the signed net movement (debit − credit) it is named for and consumed as.

For any account that has both debits and credits within a period — cash above all — this is simply wrong as a balance movement.

Evidence (live epm_gold, after the fiscal-period fix)

Books are balanced double-entry — assert_trial_balance_balances and assert_silver_gl_debit_credit_balance pass, and Σ(period_debit − period_credit) = 0 per entity/period:

USMF 2024 P1, all accounts:
  sum(period_net_amount)            = 19,265,177
  sum(period_debit)+sum(period_credit) = 19,265,177   <- period_net_amount == debit + credit (magnitude)
  sum(period_debit)-sum(period_credit) =          0   <- true signed net

Single account, USMF 2024:

acct 2100 (liability):  period_movement(magnitude) = +57,313   true net (dr-cr) = -57,313   (sign wrong)
acct 1010 (cash):       period_movement(magnitude) = +2,947,438  true net (dr-cr) = -2,696,277  (magnitude AND sign wrong — cash has mixed dr/cr)

Impact

period_net_amount is a generated base measure (dbt_project.yml vars.base_measures, regenerated by konsol's dbt_config.regenerate_vars() from the EPM Measure doctype registry). It feeds essentially the entire gold layer:

• gold_trial_balance.period_net_amount
• gold_balance_sheet.cumulative_balance (running sum of it) — wrong balances for mixed-activity accounts
• gold_bs_movement.period_movement (= it) — wrong movements
• gold_pnl_by_period
• gold_consolidated_trial_balance.local_amount (= it) → all consolidation/FX/IC models
• every quarterly / YTD / variance model

These models are internally consistent (everyone sums the same magnitude), so most existing tests still pass — but any model that needs a true signed balance movement is wrong. Phase 6.1 cash flow is the first model to exercise this rigorously and expose it (the indirect reconciliation cannot tie because cash's magnitude movement ≠ its net movement).

Root cause

accounting_currency_amount is carried as a magnitude through bronze→silver, and period_net_amount sums it directly instead of netting debit vs credit. Either:

• (a) accounting_currency_amount should be signed at staging (negate when is_credit), so sum() yields the net; or
• (b) the measure expression should be sum(debit_amount) - sum(credit_amount).

Proposed fix

Change the period_net_amount Measure expression (in the konsol Measure registry, which regenerates dbt_project.yml) from sum(accounting_currency_amount) to sum(debit_amount) - sum(credit_amount) (option b — localized to the measure, no staging-wide sign rework).

Large blast radius — flips the sign/value of period_net_amount everywhere, so it needs a coordinated pass:

• re-baseline gold_balance_sheet / gold_bs_movement (signed balances; drop any per-account sign hacks built to compensate);
• re-verify consolidation (local_amount), P&L, variance, YTD models and their tests;
• update any test that hard-codes magnitude expectations.

Acceptance criteria

• [ ] period_net_amount equals period_debit − period_credit per row/grain.
• [ ] gold_balance_sheet.cumulative_balance reflects true signed running balances; BS balances (Assets = Liab + Equity) per entity/period.
• [ ] Cash (1010) period movement equals its true net cash change.
• [ ] Existing balance/consolidation tests updated and green; no model relies on the old magnitude semantics.

Context / discovery

Found while building Phase 6.1 Cash Flow Statement (PRD docs/prd/PRD-CASH-FLOW-STATEMENT.md). Surfaced only after fixing a separate fiscal-period regression (silver_gl_entries, commit f969c3f) that had collapsed all periods to 0 and was masking this. Cash flow will be built on debit − credit directly as a localized workaround until this is fixed.

[Anonymous]

Originally posted by: grynn-in

Already resolved in code — recommend close

Triage during the PR sprint found this is fixed on main, after the issue was filed:

• dbt: 020f93a "fix(dbt): compute period_net_amount as debit minus credit" — the base_measures var in dbt_project.yml now defines period_net_amount: sum(debit_amount) - sum(credit_amount), consumed by gold_trial_balance via measure_select(), so the whole gold layer inherits signed net.
• konsol Measure registry: konsol/fixtures/measure.json (and the live registry) already carry sum(debit_amount) - sum(credit_amount); Cube/=EPM() were already correct.
• Guarding test: 94b60c2 added tests/assert_period_net_equals_debit_minus_credit.sql, asserting period_net_amount = period_debit − period_credit per trial-balance row. Its comment even notes it "fails on pre-fix gold until a full refresh rebuilds gold_trial_balance with the new measure."
• No compensating per-account sign-hacks remain in gold_balance_sheet / gold_bs_movement (grep clean), and the reconciliation tests (YTD, IC nets-zero, hierarchy ties) are aligned with signed net.

Remaining is operational, not code: the live epm_gold still shows the magnitude symptom only because it hasn't been rebuilt since the fix landed — a full dbt build (deploy pipeline or a governed full rebuild) clears it, at which point assert_period_net_equals_debit_minus_credit goes green.

No PR opened — nothing to change. Suggest closing once a full rebuild has run and the assertion is confirmed green in CI.

[Anonymous]

Originally posted by: grynn-in

Resolved on main — no code change needed

This is fixed as of the current main. The proposed fix (option b) has already landed:

Measure source-of-truth (konsol/fixtures/measure.json, status Published) and the generated dbt_project.yml vars.base_measures both now define:

period_net_amount :: sum(debit_amount) - sum(credit_amount)

instead of the old sum(accounting_currency_amount) magnitude sum.

And silver_gl_entries.sql no longer stores a magnitude — since konsolidat#112/#121 it derives debit_amount/credit_amount from the sign of the already-signed accounting_currency_amount (positive → debit, negative → credit; is_credit retired). So:

period_net_amount = Σ(positive parts) − Σ(|negative parts|) = Σ(signed amount) = signed net movement ✓

Cash and other mixed-activity accounts now net correctly (dr − cr), which is exactly what the balance/movement/cash-flow models consume. The Phase 6.1 cash flow that first exposed this now ties.

Closing as fixed. (The FX re-zeroing sibling, [#109], has an open fix in [#127].)

[Anonymous]

Ticket changed by: grynn-in

• status: open --> closed