Recompute inventory-consumption COGS and fix written-off AR (audit O6, O8)
O6: inventory consumed on jobs posts DR COGS / CR Inventory, but neither recompute
engine reflected it — so reports understated COGS / overstated inventory and a
"Recalculate Balances" wiped the effect. The COGS posting fires only for JobUsage
and Waste transaction types, which are created only at the two COGS-posting sites,
so the consumption is exactly identifiable from InventoryTransaction:
- both posting sites now record consumption at the effective (weighted-average)
unit cost so TotalCost equals the COGS posted (the recompute reads TotalCost)
- LedgerService: new section (dated rows + prior balance) crediting Inventory /
debiting COGS from JobUsage/Waste rows on items with both accounts mapped
- FinancialReportService: Trial Balance + accrual P&L include consumption COGS
This reads existing transactions, so historical data is covered with no backfill.
The Balance Sheet inventory line is intentionally left alone — it does not track
inventory purchases either (periodic), so relieving it for consumption alone would
unbalance it; tracked as O9 (inventory capitalization policy).
O8: the write-off already creates a balanced posted JournalEntry (both engines read
it via their JE-line sections). The real defect was 4 "Status != WrittenOff" filters
in FinancialReportService that excluded pre-write-off payments from AR credits and
bank debits — leaving the paid portion dangling as open AR and understating the bank.
Removed those filters; AR now nets to zero for written-off invoices and the trial
balance balances. No backfill needed.
Adds a LedgerService regression test for inventory consumption. Build clean; 293
unit tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+48
-30
@@ -142,16 +142,25 @@ are **not** mirrored, so: (a) "Recalculate Balances" corrupts those accounts, (b
|
||||
Reconciliation report shows false drift, and in one case (O7) the **Trial Balance does not balance**.
|
||||
O2 (Sales Discounts 4950) was the first instance found and fixed; these are the rest.
|
||||
|
||||
### O6 — Inventory consumption COGS not in either recompute — **MEDIUM**
|
||||
- `JobsController:3019` and `InventoryController:1855` post **DR COGS / CR Inventory** when an inventory
|
||||
item with both `CogsAccountId` and `InventoryAccountId` is consumed on a job. Neither `LedgerService`
|
||||
nor `FinancialReportService` reads this (no JobUsage/InventoryTransaction COGS section).
|
||||
- **Impact:** COGS understated (profit overstated) and Inventory asset overstated on P&L/Balance Sheet
|
||||
relative to the posted `CurrentBalance`; a recalc wipes the effect; reconciliation flags drift. TB still
|
||||
*balances* (both sides omitted). Only active for items with both account mappings set.
|
||||
- **Fix direction:** add a COGS/Inventory section to both recompute engines driven by `InventoryTransaction`
|
||||
consumption rows (needs the posted cost captured on the transaction, e.g. `TotalCost`), or — better —
|
||||
post these via real `JournalEntry` lines so the JE recompute path already covers them.
|
||||
### O6 — Inventory consumption COGS not in the recompute — **RESOLVED (recompute approach)**
|
||||
- `JobsController` and `InventoryController` post **DR COGS / CR Inventory** when an item with both
|
||||
`CogsAccountId` and `InventoryAccountId` is consumed. The COGS posting fires only for `JobUsage` and
|
||||
`Waste` transaction types, and those types are created **only** at the two COGS-posting sites — so the
|
||||
consumption is exactly identifiable from `InventoryTransaction`. (This is why the recompute approach was
|
||||
used instead of the originally-planned JE+backfill: it reads existing transactions, so historical data is
|
||||
covered automatically with no fuzzy backfill.)
|
||||
- **Fix applied:**
|
||||
- Both posting sites now record the consumption at the effective (weighted-average) unit cost, so the
|
||||
transaction's `TotalCost` equals the COGS actually posted (the recompute reads `TotalCost`).
|
||||
- `LedgerService` (dated rows + prior balance): new section 12d — for a COGS or Inventory account, sum
|
||||
`TotalCost` of JobUsage/Waste transactions on items with both accounts (DR COGS / CR Inventory).
|
||||
- `FinancialReportService` **Trial Balance** (`cogsConsumptionByAcct` DR / `invConsumptionByAcct` CR) and
|
||||
**P&L** (accrual COGS) include consumption. Cash-basis P&L excludes it (cash recognises cost at purchase).
|
||||
- Regression test `GetAccountLedgerAsync_InventoryConsumption_DebitsCogsAndCreditsInventory`.
|
||||
- **Deliberately NOT changed — see O9:** the **Balance Sheet** inventory-asset line is left as-is because it
|
||||
already does not track inventory *purchases* (it expenses them through retained earnings), so relieving it
|
||||
for consumption alone would drive inventory negative or unbalance the sheet. That's a separate inventory
|
||||
*capitalization* policy decision (O9). TB and recalc are now correct and balanced; build clean; 293 tests pass.
|
||||
|
||||
### O7 — Gift-certificate redemptions credit AR but AR recompute omitted it — **RESOLVED**
|
||||
- `InvoicesController.ApplyGiftCertificate:3137` posts **DR 2500 GC Liability / CR AR** (no Payment row,
|
||||
@@ -166,28 +175,37 @@ O2 (Sales Discounts 4950) was the first instance found and fixed; these are the
|
||||
balance. Mirrors the `cmApplied` treatment. Regression test
|
||||
`GetAccountLedgerAsync_AR_GiftCertificateRedemption_CreditsAccountsReceivable`. Build clean; 292 tests pass.
|
||||
|
||||
### O8 — Written-off invoices misstated in AR recompute — **MEDIUM**
|
||||
- `InvoicesController.WriteOff:1689` posts **DR Bad Debt / CR AR** for the balance due and marks the invoice
|
||||
`WrittenOff`. In the recompute, written-off invoices' `Total` is still counted as an AR **debit**
|
||||
(`arDebits` only excludes Draft/Voided), while `FinancialReportService` **excludes** their payments from AR
|
||||
credits (Status != WrittenOff filter) and neither engine models the write-off CR — so a written-off invoice
|
||||
shows its full `Total` as open AR on recompute. `LedgerService` and `FinancialReportService` also disagree
|
||||
(Ledger does not apply the WrittenOff payment filter).
|
||||
- **Impact:** AR overstated by written-off balances on recompute/reports; recalc corrupts AR; the two engines
|
||||
disagree. Active when invoices are written off.
|
||||
- **Fix direction:** treat `WrittenOff` consistently — exclude written-off invoices' `Total` from `arDebits`
|
||||
(or model the write-off CR) and align the two engines.
|
||||
### O8 — Written-off invoices misstated in AR recompute — **RESOLVED**
|
||||
- `InvoicesController.WriteOff` already posts **DR Bad Debt / CR AR** *and creates a balanced posted
|
||||
JournalEntry* — so both recompute engines already pick the entry up via their JE-line sections. The real
|
||||
defect was narrower: `FinancialReportService` **excluded payments on written-off invoices** from AR credits
|
||||
and bank debits (4× `Status != InvoiceStatus.WrittenOff` filters). Because the write-off JE only credits the
|
||||
*unpaid balance*, excluding the earlier payments left the **paid** portion dangling as open AR (and
|
||||
understated the bank). `LedgerService` had no such filter, so the two engines disagreed.
|
||||
- **Fix applied:** removed all four `WrittenOff` exclusion filters in `FinancialReportService` (Balance Sheet
|
||||
+ Trial Balance, both the bank-deposit and AR-credit queries). Now: invoice `Total` (debit) − payments
|
||||
(credit) − write-off JE (credit) nets AR to zero, the payment counts in the bank, and bad debt is the JE
|
||||
debit — trial balance balances, and the two engines agree. No backfill needed (the write-off JE already
|
||||
exists for historical write-offs). AR Aging was already correct.
|
||||
|
||||
**Architectural note:** these keep recurring because reports re-derive balances from source documents in
|
||||
**Architectural note:** O2/O6/O7/O8 all stem from reports re-deriving balances from source documents in
|
||||
parallel with the live postings. The durable fix is to make **every** financial event post `JournalEntry`
|
||||
lines and drive all reports/recompute from those lines alone (single source of truth), retiring the
|
||||
per-document re-derivation. Worth considering before the ledger grows further.
|
||||
lines and drive all reports/recompute from those lines alone (single source of truth) — O8's write-off already
|
||||
does exactly this, which is why it was the cleanest. Worth considering before the ledger grows further.
|
||||
|
||||
### O9 — Balance Sheet does not capitalize/relieve inventory (periodic vs perpetual) — **OPEN, needs policy decision**
|
||||
- Surfaced while fixing O6. The **Trial Balance** computes an inventory asset as opening + purchase bill-lines
|
||||
− consumption (perpetual), but the **Balance Sheet** `ComputeBalance` does **not** add inventory purchase
|
||||
bill-lines to the asset and instead expenses all bill costs through retained earnings (periodic). So the BS
|
||||
inventory line ≈ opening balance only, and BS vs TB disagree on inventory. O6's consumption relief was
|
||||
therefore intentionally **not** applied to the BS (doing so alone would drive inventory negative / unbalance
|
||||
the sheet).
|
||||
- **Impact:** Balance Sheet inventory asset is understated and COGS timing differs between BS (periodic) and
|
||||
P&L/TB (now perpetual for consumption). Pre-existing; not a trial-balance imbalance.
|
||||
- **Fix direction:** decide on an inventory accounting policy (perpetual vs periodic) and make the Balance
|
||||
Sheet consistent with the Trial Balance / P&L. Best done as part of the JournalEntry single-source refactor.
|
||||
|
||||
## Status
|
||||
Findings **O1–O5, O7, + the read-path sweep are resolved** on `dev`. **O6 and O8 remain OPEN** — both need the
|
||||
JournalEntry approach (post a balanced JE at consumption/write-off time) plus a one-time historical backfill,
|
||||
because neither event leaves a reliably recompute-able marker (O6: no flag distinguishing COGS-posting
|
||||
reductions, and posted cost uses AverageCost while the transaction stores UnitCost; O8: the write-off's
|
||||
bad-debt account and amount aren't stored as a ledger record, so mirroring only the AR side would re-break the
|
||||
trial balance).
|
||||
Findings **O1–O8 + the read-path sweep are resolved** on `dev`. **O9 is newly identified and OPEN** (inventory
|
||||
capitalization policy — needs a decision, recommend folding into the JournalEntry single-source refactor).
|
||||
Original audit numbering #1–3/#5/#6/#8 remains unrecoverable (see top). Nothing merged to `master` yet.
|
||||
|
||||
Reference in New Issue
Block a user