# 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 — Inventory accounting policy — **RESOLVED (policy decision: expense at purchase / periodic)** - **Decision (owner, 2026-06-19):** materials (powder, consumables) are **expensed at purchase**, not capitalized as inventory-for-resale — this is a service business that *uses* materials to deliver a service, it does not sell inventory. So the **periodic** model is correct. - **Implication — no code change needed:** - The Balance Sheet correctly does **not** capitalize inventory (cost hits the P&L at purchase via the bill line categorized to a COGS/expense account). The earlier decision to leave the BS untouched stands. - Purchase receiving creates a `Purchase`-type `InventoryTransaction` that posts **no** GL — correct. - The **perpetual** consumption-COGS path (O6: `DR COGS / CR Inventory` on JobUsage/Waste) is **opt-in**: it only fires when an item has *both* `CogsAccountId` and `InventoryAccountId`, which are set **only via CSV import** (never by normal item creation). Under this policy those mappings should be left empty, so the path stays dormant. O6's recompute fix remains correct and harmless when unused. - **⚠ Footgun to avoid:** do **not** both expense powder at purchase (bill → COGS account) *and* map an item's `CogsAccountId` + `InventoryAccountId` — that would record the cost twice (once at purchase, once at consumption). Keep item COGS/Inventory account mappings empty under the expense-at-purchase policy. #### O9 update (2026-06-20) — default GL accounts feature widened this surface - The original O9 note assumed item `CogsAccountId`/`InventoryAccountId` are "set only via CSV import, never by normal item creation." **That assumption no longer holds.** A new **Default Accounts** feature (Chart of Accounts → "Set Defaults", stored on `CompanyPreferences.Default{Revenue,Cogs,Inventory}AccountId`) pre-fills these fields on **normal** Inventory/Catalog item creation. A company that sets *both* a default COGS and a default Inventory account will have new items post `DR COGS / CR Inventory` on consumption — i.e. it opts the shop into the perpetual path. - **Why it's still safe:** the defaults are **null by default**, so nothing changes until a company deliberately sets them. The footgun (double-counting under expense-at-purchase) is surfaced with an inline warning on the Default Accounts card and in the Settings help article. Posting and balance-recompute both read the item's *stored* accounts, so there is **no recompute drift** — verified during the 2026-06-20 audit. - **Revenue default fallback:** invoice lines now fall back to `DefaultRevenueAccountId` (active only), then to account 4000 (`InvoicesController.Create`). Resolved at invoice-create time and stored on the `InvoiceItem`, so recompute stays consistent. ### 2026-06-20 audit — dropdown sub-type→type broadening + deposit account picker Reviewed after broadening account dropdowns from sub-type to parent `AccountType` and adding a user-selectable deposit account. **No ledger-drift bugs found.** Notes: - **Deposit account picker ↔ recompute:** consistent. Live posting debits the chosen `DepositAccountId`, and `LedgerService` reproduces the debit by `DepositAccountId == accountId` (lines ~78/724). Picking a non-default deposit account recomputes correctly. - **Bank / "pay-from" / bank-rec pickers — server-side guard added (2026-06-20):** the submitted account is now validated via `AccountGuard.IsValidMoneyAccountAsync` (active, company-owned, AccountType Asset or Liability) before any posting, at bill `RecordPayment` / `Create(payNow)` / `EditPayment`, `BankReconciliation.Create`, and deposit `Record`. Defense in depth against tampered/stale POSTs. Per the "trust the operator" decision this still allows e.g. A/R (an Asset) as a source — it only rejects non-money types (Revenue/Expense/Equity/COGS). - **Latent deposit imbalance — RESOLVED (2026-06-20):** a deposit saved with a null `DepositAccountId` posted `CR 2300` with no offsetting debit → unbalanced. `DepositsController.Record` now blocks recording when the `2300` Customer Deposits account exists but no deposit/bank account resolves (user must pick one). When `2300` doesn't exist (company not using accounting), no GL posts at all, so the deposit is still allowed through. - **Type/sub-type mismatch risk — RESOLVED (2026-06-20):** account `AccountType` is now **derived** from the chosen `AccountSubType` on create/edit via `AccountClassification.TypeForSubType` (single source of truth, also used by the Create pre-select), so the two can never disagree and the sub-type-based sign convention is always consistent with the displayed type. A read-only sweep of the dev DB (109 accounts) found **0** existing mismatches, so no repair tool was needed. ## Status **All findings O1–O9 + the read-path sweep are resolved** on `dev` (O9 by policy decision — expense at purchase — needing no code change). The optional structural follow-up is the JournalEntry single-source refactor (`docs/ACCOUNTING_LEDGER_REFACTOR.md`), which would prevent the O2/O6/O7/O8 bug class from recurring. Original audit numbering #1–3/#5/#6/#8 remains unrecoverable (see top). Nothing merged to `master` yet.