PowderCoatingLogix/docs/ACCOUNTING_AUDIT.md

Accounting System Audit

Living record of the accounting/GL audit and its remediation status. Keep this file updated whenever an accounting finding is opened or closed — the original audit was lost once because it was never written down. Don't let that happen again.

Last reconciled against code: 2026-06-19 (branch dev, commits through 9532812). Verification at that point: dotnet build clean (0 errors); dotnet test tests/PowderCoating.UnitTests → 290 passed, 0 failed.

---

How the GL is modeled (context for the findings)

Balances are computed two different ways, and they must agree:

1. At posting time — controllers call AccountBalanceService.DebitAsync/CreditAsync, which mutate the denormalized Account.CurrentBalance.
2. At report time — FinancialReportService (Trial Balance, Balance Sheet, P&L, AR/AP aging, statements) and LedgerService (per-account ledger + RecalculateBalances) recompute balances from source documents.

LedgerService also backs the Balance Reconciliation report (the detective control added in c2cd19e), which compares stored CurrentBalance vs the ledger-recomputed balance. Any account whose recompute path is incomplete will (a) be silently corrupted by a "Recalculate Balances" run and (b) produce misleading output in the reconciliation report.

---

Resolved findings (this audit batch)

#	Finding	Fix	Commit	Verified
#4	Sales Tax Payable (2200) was credited on every invoice but never relieved — the liability grew forever (no remittance flow).	JournalEntries/SalesTaxPayment GET+POST: DR 2200 / CR bank, balanced, in a transaction, honors the period lock and validates amount + bank-account tenancy.	0c921ba	Posting reviewed JournalEntriesController:235-296. Reporting needs no change (posted JE lines already flow through TB/BS/ledger).
#7	Most report queries relied on the global tenant filter, which is bypassed for SuperAdmin → cross-company leakage on P&L / Balance Sheet / Trial Balance / aging / statements.	Explicit CompanyId == companyId predicate added to every query in FinancialReportService.	9532812	Confirmed: every query in FinancialReportService.cs carries an explicit CompanyId predicate.
#9a	A cash refund posted DR AR (control up) while the customer subledger went down — opposite directions → guaranteed AR drift.	Refund now reverses the sale: DR Sales Returns (4960) + DR Sales Tax Payable (tax portion) / CR bank; AR untouched. Split centralized in Core/Accounting/RefundAllocation.Split.	2a82a1d	Posting (InvoicesController.IssueRefund/CancelRefund) and both recompute paths (LedgerService §5b, FinancialReportService TB/BS/P&L) all use RefundAllocation.Split — they agree by construction.
#9b	Store-credit refunds & credit memos posted nothing to the GL on issue (only a CreditMemo + Customer.CreditBalance) → outstanding store credit invisible on the balance sheet.	New 2350 Customer Credits liability. Issue: DR 4950 / CR 2350. Apply: DR 2350 / CR AR. Void remainder: DR 2350 / CR 4950. Updated in all 8 posting sites + TB/BS/P&L + ledger §12b.	9ce3612	Posting sites confirmed (CreditMemosController 180-185/262-270/314-320; InvoicesController store-credit path). Reporting cmContraRevenue/cmIssuedNonVoided math reviewed.
—	No detective control to catch denormalized-balance drift.	Balance Reconciliation report (Reports/Reconciliation): per-account stored vs recomputed, plus AR/AP control-vs-subledger.	c2cd19e	Reviewed FinancialReportService.GetBalanceReconciliationAsync. NOTE: its usefulness is limited by O2 below.

Original numbering gap

The commits reference findings #4, #7, #9a, #9b — implying a list of at least #1–#9. The original audit document was never persisted and is unrecoverable. Findings #1, #2, #3, #5, #6, #8 cannot be matched to commits and their status is unknown. If the original list resurfaces, reconcile it here.

---

Resolved findings (2026-06-19 remediation)

O1 — Account 2300 mislabeled "Payroll Liabilities" while used as Customer Deposits — RESOLVED

• Root cause refined: there is no payroll feature posting to 2300; the deposit GL code has always used 2300 (resolved by number) as the Customer Deposits liability. Migration AccountingDepositsGL already renamed it to "Customer Deposits" for tenants that lacked a 2300, but its NOT EXISTS guard skipped pre-existing tenants, leaving them mislabeled. The two seed files still created new tenants with the wrong name.
• Fix (chosen approach: rename 2300, move payroll to a new 2400):
  • Seed files now create 2300 = "Customer Deposits" (IsSystem) and a separate 2400 = "Payroll Liabilities" — SeedDataService.Accounts.cs, SeedData.cs.
  • EnsureSystemAccountsAsync self-heals: renames any 2300 still named "Payroll Liabilities" → "Customer Deposits" (preserving user renames) and ensures 2400 exists.
  • Migration 20260619233108_RenameDepositsAccountAddPayroll does the same for existing tenants (rename only where Name is still the default; insert 2400 where missing). Down is best-effort (soft-deletes only empty 2400s; does not re-introduce the mislabel).
• Account number 2300 is unchanged, so the deposit posting code (DepositsController, InvoicesController) needed no changes — its lookups are by "2300".

O2 — LedgerService never recomputed 4950 Sales Discounts → recalc corrupted it — RESOLVED

• Fix: added a 4950 section to both LedgerService.GetAccountLedgerAsync and ComputePriorBalanceAsync that reproduces the actual postings so a balance recalc matches the stored balance: invoice discounts (DR at invoice date) + credit-memo issuance (DR full amount at issue) − the unapplied remainder of voided memos (CR at void). This mirrors what AccountBalanceService posts at InvoicesController:849/1108/1629 and CreditMemosController, so the Balance Reconciliation report no longer shows false drift on 4950.
• Note / micro-discrepancy: the ledger uses memo.AmountApplied for the voided remainder (matching the true posting), whereas FinancialReportService TB/BS derive the voided portion from applications against non-voided invoices. These differ only in the rare case of a voided memo applied to a since-voided invoice. Left as-is (report-side nuance, not O2's target); documented here so it isn't mistaken for a bug.
• Regression test: LedgerServiceTests.GetAccountLedgerAsync_SalesDiscounts4950_IncludesInvoiceDiscountsAndCreditMemoContraRevenue.

Verification of O1+O2: dotnet build clean; dotnet test tests/PowderCoating.UnitTests → 291 passed; migration applied to the dev database successfully.

---

O3 — Write-path account lookups omitted explicit CompanyId — RESOLVED

• Added CompanyId to every GL posting-path account lookup that determines where money posts:
  • InvoicesController — all account-resolver helpers (GetCheckingAccountIdAsync, GetCustomerDepositsAccountIdAsync, GetSalesReturnsAccountIdAsync, GetCustomerCreditsAccountIdAsync, GetArAccountIdAsync, GetBadDebtAccountIdAsync, ResolveSalesTaxAccountIdAsync, GetSalesDiscountAccountIdAsync, GetGcLiabilityAccountIdAsync) plus the bank-account and write-off expense dropdowns (scoped via _tenantContext).
  • CreditMemosController — Create/Apply/Void GL lookups (scoped via the in-scope customer/invoice/memo).
  • GiftCertificatesController — Create, BulkCreate, Void GL lookups + GetGcLiabilityAccountIdAsync.
  • BillsController — AP/expense account resolution that pre-fills APAccountId (Create + CreateFromPO).
• DepositsController and JournalEntriesController.SalesTaxPayment already scoped correctly.

O4 — Sales-tax remittance could over-remit (drive 2200 negative) — RESOLVED

• JournalEntriesController.SalesTaxPayment (POST) now rejects any amount exceeding taxAcct.CurrentBalance (0.005 rounding tolerance), so a typo can no longer push Sales Tax Payable into an abnormal debit balance.

Verification of O3+O4: dotnet build clean; dotnet test tests/PowderCoating.UnitTests → 291 passed.

O5 — Gift Certificate Liability 2500 missing for new tenants / mislabeled on default company — RESOLVED

• Root cause (same shape as O1): 2500 is resolved by number as the GC liability (GiftCertificatesController). The AccountingGapsPhase2 migration seeded it for tenants existing at deploy, but (a) the per-tenant seeder SeedDataService.Accounts.cs never created a 2500, so tenants onboarded afterward had no GC liability account and GC GL postings silently no-op'd; and (b) the default-company seeder SeedData.cs created 2500 as "Long-Term Loan", so that company's GC obligations were mislabeled (and the migration's NOT EXISTS guard skipped it).
• Fix:
  • SeedDataService.Accounts.cs now seeds 2500 "Gift Certificate Liability" (IsSystem).
  • SeedData.cs now seeds 2500 as GC liability and moves the long-term loan to 2900.
  • EnsureSystemAccountsAsync self-heals: renames any 2500 still named "Long-Term Loan" → "Gift Certificate Liability" (preserving user renames) and ensures a 2500 exists.
  • Migration 20260620002950_FixGiftCertificateLiabilityAccount: moves long-term loan to 2900 where a 2500="Long-Term Loan" exists and no 2900 is present; relabels the mislabeled 2500; safety-net inserts a 2500 for any company lacking one. Non-destructive (no Id/number/balance changes); Down is best-effort.
• Verified on the dev DB: existing 2500 GC-liability rows preserved; no spurious accounts added; build clean; migration applied; 291 unit tests pass.

---

Read-path account lookup sweep (Tier 1–3) — RESOLVED

Completed the app-wide defense-in-depth pass: every Accounts lookup in a controller now carries an explicit CompanyId predicate, matching the standing rule in CLAUDE.md. ~19 lookups across 12 controllers:

• Tier 1 (write-path validation): AccountsController duplicate account-number check on Create/Edit.
• Tier 2 (dropdowns/lists): AccountsController (Index/year-end/parent dropdown), BankReconciliations, Bills (bank list + receipt-scan + suggest), Budgets, CatalogItems, Expenses, FixedAssets, Inventory, JournalEntries (chart dropdown), Vendors.
• Tier 3 (accountIds.Contains display maps): JournalEntries Details, Reports budget vs actual, VendorCredits Details — scoped via the in-scope entity's CompanyId for uniformity. companyId source per controller: _tenantContext.GetCurrentCompanyId() where available, else the in-scope entity's CompanyId, else _userManager current user. Build clean; 291 unit tests pass.

---

Whole-system re-audit (2026-06-19) — NEW findings, OPEN

Root cause shared by all three: the GL is kept two ways — direct postings via AccountBalanceService.Debit/CreditAsync (authoritative Account.CurrentBalance) and recomputes via LedgerService (drives RecalculateAllAsync) and FinancialReportService (drives the reports). Every posting type must be mirrored in both recompute engines or balances diverge. Several postings are not mirrored, so: (a) "Recalculate Balances" corrupts those accounts, (b) the Balance 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 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, no AmountPaid change — only invoice.GiftCertificateRedeemed). The 2500 debit IS recomputed (GC redemptions), but the AR credit is NOT: AR recompute = sum(Total) − Payments − CreditMemoApplications in both FinancialReportService (BS line 358 / TB line 1129) and LedgerService (AR section).
• Impact: because only one side of the entry is recomputed, the Trial Balance is out of balance by the total GC redeemed; Balance Sheet AR is overstated; recalc corrupts AR. (AR Aging is correct — it uses BalanceDue, which includes GiftCertificateRedeemed.) Active for any company that redeems GCs on invoices.
• Fix applied: GC redemptions now subtracted from AR in both recompute engines — FinancialReportService Balance Sheet (gcRedeemedBs) and Trial Balance (gcRedeemedTb), and LedgerService AR section + prior balance. Mirrors the cmApplied treatment. Regression test GetAccountLedgerAsync_AR_GiftCertificateRedemption_CreditsAccountsReceivable. Build clean; 292 tests pass.

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: 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) — 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–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.