erpnext/specs/accounts_refactor_spec.md

# Accounts / Controller Refactor — Spec

## Motivation
Move ERPNext away from the deep `AccountsController → SellingController/BuyingController → SalesInvoice`
inheritance chain and the monolithic `sales_invoice.py` / god-object `accounts_controller.py`
toward **composition**: per-doctype `services/` plus shared module-level `accounts/services/`.
Goal is testability, readability, and factoring shared domain logic out so Sales/Purchase
voucher logic is not duplicated.

## Target structure
```
erpnext
├── controllers
│   └── transaction_controller.py        # thin lifecycle base, delegates to services
├── accounts
│   ├── general_ledger.py                # the SINK (unchanged): post / merge / round-off / reverse
│   ├── services
│   │   ├── base_gl_composer.py          # BaseGLComposer — shared GL helpers
│   │   ├── gl_validator.py              # list-level validation (functions, stateless)
│   │   ├── advances.py
│   │   ├── taxes.py
│   │   └── budget.py
│   └── doctype
│       └── sales_invoice
│           ├── sales_invoice.py         # thin: delegates to services
│           ├── services
│           │   ├── gl_composer.py       # SalesInvoiceGLComposer(BaseGLComposer)
│           │   ├── pos.py
│           │   ├── loyalty.py
│           │   ├── status.py
│           │   ├── inter_company.py
│           │   ├── fixed_assets.py
│           │   └── timesheet_billing.py
│           ├── mapper.py
│           └── api.py
```

## GL layer — frozen design
Pipeline:
```
SalesInvoiceGLComposer.compose()  →  gl_entries  →  gl_validator.validate(gl_entries)  →  general_ledger.make_gl_entries()
```

| Role | Location | Form | Responsibility |
|---|---|---|---|
| **Composer (base)** | `accounts/services/base_gl_composer.py` → `BaseGLComposer` | class (stateful, holds `self.doc`) | shared row factory + common entries |
| **Composer (doctype)** | `sales_invoice/services/gl_composer.py` → `SalesInvoiceGLComposer(BaseGLComposer)` | class | voucher-specific rows via `.compose()` |
| **Validator** | `accounts/services/gl_validator.py` | module functions (stateless) | assert the finished `gl_entries` list is legal to post |
| **Sink** | `accounts/general_ledger.py` (unchanged) | module functions | merge / round-off / post / reverse |

### Naming decisions (frozen)
- Chose **`compose`** over `make`/`build` — the sink already owns the verb `make` (`make_gl_entries`); `compose` avoids a two-makers collision.
- `base_` prefix on the shared/abstract file; the concrete subclass carries the specific name, no prefix.
- Rejected: `gl_map` (it's a list, not a map — but it's an entrenched public param; rename to `gl_entries` later as its own deprecation pass), `gl_processor` (redundant with `general_ledger.py`), `gl_entries.py` (collides with the `gl_entry` doctype + the ubiquitous local var), `ledger_builder` (clashes with stock/payment ledger), `builder`/`maker` (generic; "maker" collides with `make_gl_entries`).

## Bucketing `accounts_controller.py`
- **Base composer (`BaseGLComposer`):** `get_gl_dict`, `get_value_in_transaction_currency`, `make_discount_gl_entries` (+ `get_amount_and_base_amount`, `get_tax_amounts`), `make_precision_loss_gl_entry`, `make_exchange_gain_loss_journal` (+ `gain_loss_journal_already_booked`), `set_transaction_currency_and_rate_in_gl_map`. Regional hooks `update_gl_dict_with_regional_fields` / `..._app_based_fields` stay free functions called inside `get_gl_dict`.
- **Advances service:** `set_advances`, `get_advance_entries`, `clear_unallocated_advances`, `validate_advance_entries`, `set_advance_gain_or_loss`, `calculate_total_advance_from_ledger`, `set_total_advance_paid`, `set_advance_payment_status`, `delink_advance_entries`, `create_advance_and_reconcile`, `get_advance_payment_doctypes`, `_remove_advance_payment_ledger_entries`, module funcs `get_advance_journal_entries` / `get_advance_payment_entries`.
- **Validator (from `general_ledger.py`):** `validate_disabled_accounts`, `validate_accounting_period`, `validate_cwip_accounts`, `check_freezing_date`, `validate_against_pcv`, `validate_allowed_dimensions`. (Moved in Phase 1.)
  - **Balance trio stays in `general_ledger.py` for now** (revised during Phase 1): `get_debit_credit_difference` / `get_debit_credit_allowance` / `raise_debit_credit_not_equal_error`. `get_debit_credit_difference` *mutates* entries (rounds debit/credit in place) and the trio is interleaved with `process_debit_credit_difference` → `make_round_off_gle` (the round-off *repair* run before and after balancing). It is not a standalone pre-post gate, so it can't move into a pure `validate(gl_entries)` without changing behavior. It travels with round-off when that moves compose-side (see below).
  - **Stays in compose (do NOT move to validator):** `process_debit_credit_difference` / `make_round_off_gle` — these *repair* balance by appending a round-off entry (mutation), not validation.
  - **Stays in composer (not validator):** row-level checks (right account for a row, dimension applicability) — validator only validates the finished list.
- **Leave in controller:** `validate_company_in_accounting_dimension`, `validate_company` (dimension validation, not GL).

## Phases
Each phase is behavior-preserving, one draft PR, gated by the Phase-0 snapshot suite + `bench run-tests --site test-site-ai`.

### Phase 0 — Safety net (first, mandatory)
Characterization tests snapshotting `gl_entries` output for representative transactions (SI/PI with taxes, multi-currency, advances, discounts, round-off, POS). Every later phase passes iff snapshots are byte-identical.

### Phase 1 — Extract `gl_validator.py` (lowest risk) — DONE
Moved the 6 pure list-level validators to `erpnext/accounts/services/gl_validator.py`; `general_ledger.py` imports and calls them at the existing call sites (no behavior change). A consolidated `gl_validator.validate(gl_entries)` facade is deferred — the current checks run at different points (make_gl_entries / save_entries per-entry / make_reverse_gl_entries), so collapsing them into one call would alter ordering. Verified: all 12 Phase-0 snapshots byte-identical.

### Phase 2 — Pilot composer on Sales Invoice only — DONE
Added `BaseGLComposer` (minimal: holds `self.doc`) and `SalesInvoiceGLComposer`. SI's `get_gl_entries` is a thin shim delegating to `SalesInvoiceGLComposer(self).compose()`. All 11 SI-specific row builders (make_customer/tax/item/internal_transfer/pos/loyalty/write_off/rounding GL entries, stock_delivered_but_not_billed, get_gl_entries_for_fixed_asset, get_gle_for_change_amount) moved onto the composer and operate on `self.doc`. The `super().get_gl_entries()` stock-expense call became `super(SalesInvoice, doc).get_gl_entries()` (MRO-faithful). Bucket-A shared helpers (`get_gl_dict`, `make_discount_gl_entries`, `make_precision_loss_gl_entry`, `set_transaction_currency_and_rate_in_gl_map`, `get_tax_amounts`, `get_amount_and_base_amount`) **stay on the controller** — they're still called via `self.doc` and only lift to `BaseGLComposer` once all doctypes use composers (can't move while other doctypes inherit them). Verified: 12 snapshots + 10 existing SI tests (perpetual `super()`, POS change, write-off, returns, fixed-asset disposal/regain, internal transfer, loyalty) all green.

### Phase 3 — Second doctype: Purchase Invoice (base earns its shape) — DONE
Added `PurchaseInvoiceGLComposer` with all 13 PI GL builders migrated (make_supplier_gl_entry, add_supplier_gl_entry, make_item_gl_entries, make_stock_adjustment_entry, get_provisional_accounts, make_provisional_gl_entry, update_net_purchase_amount_for_linked_assets, make_tax_gl_entries, make_internal_transfer_gl_entries, make_gl_entries_for_tax_withholding, make_payment_gl_entries, make_write_off_gl_entry, make_gle_for_rounding_adjustment). PI.get_gl_entries is a thin shim. **Decision after comparing SI and PI: keep `BaseGLComposer` minimal** (`self.doc` + abstract `compose`). The two flows differ too much to share a template — different step order, different builders, per-doctype `make_regional_gl_entries`. Revisit base-lifting only when a 3rd+ doctype reveals a real common shape. Remaining on doc: Bucket-A helpers (`make_precision_loss_gl_entry`, `set_transaction_currency_and_rate_in_gl_map`, `get_gl_dict`, `get_tax_amounts`, `get_amount_and_base_amount`) and inherited `set_gl_entry_for_purchase_expense`. Verified: 12 snapshots + 80/81 existing PI tests green (1 pre-existing failure in `test_purchase_invoice_with_exchange_rate_difference_for_non_stock_item`, unrelated to refactoring).

### Phase 4 — Roll out composer to remaining GL-posting doctypes
Payment Entry, Journal Entry, Delivery Note, Stock Entry, etc. Mechanical now; one PR per doctype (or small batches), each snapshot-gated.

- **Payment Entry — DONE.** Added `payment_entry/services/gl_composer.py` → `PaymentEntryGLComposer(BaseGLComposer)`. `compose()` mirrors the old `build_gl_map` (setup party account field, set txn currency/rate, then party/bank/deductions/tax builders, then `add_regional_gl_entries`). The four row builders (`add_party_gl_entries`, `add_bank_gl_entries`, `add_tax_gl_entries`, `add_deductions_gl_entries`) moved onto the composer and operate on `self.doc`; `build_gl_map` is now a thin shim delegating to the composer. **Advance builders stay on the doc** (`make_advance_gl_entries`, `add_advance_gl_entries`, `get_dr_and_account_for_advances`, `add_advance_gl_for_reference`) — they post in a separate pass inside `make_gl_entries`, not part of `compose()`, and belong to the Phase 5 advances service. Shared helpers (`get_gl_dict`, `calculate_base_allocated_amount_for_reference`, `get_exchange_rate`, `get_party_account_for_taxes`) stay on the doc, called via `self.doc`. Extended the Phase-0 snapshot net with 5 PE scenarios (receive-vs-SI, pay-vs-PI, deductions, taxes, multi-currency). Verified: 17 snapshots byte-identical + 53 existing PE tests green.
- **Journal Entry — DONE.** Added `journal_entry/services/gl_composer.py` → `JournalEntryGLComposer(BaseGLComposer)`. A JE already carries its ledger rows in the `accounts` child table, so `compose()` is a straight projection of those rows into GL dicts via `self.doc.get_gl_dict` (resolving txn currency/rate from the first foreign-currency row, mirroring the former `build_gl_map`). `build_gl_map` is now a thin shim (kept public — JE tests call it directly). Dropped the now-unused `get_advance_payment_doctypes` import from `journal_entry.py`. Extended the snapshot net with 3 JE scenarios (basic two-line, multi-currency, against-SI with party + reference). Verified: 20 snapshots byte-identical + 18 existing JE tests green.

### Phase 5 — Extract `advances.py`
Move the advances cluster. After composers, because advances cross-calls the exchange-gain/loss helper now on `BaseGLComposer`.

### Phase 6 — Extract remaining domain services from `accounts_controller`
`taxes.py`, `budget.py`, etc. Shrink `accounts_controller` to a thin lifecycle base that delegates.

### Phase 7 — Split the rest of the `sales_invoice.py` monolith
Non-GL doctype services: `pos.py`, `loyalty.py`, `status.py`, `inter_company.py`, `fixed_assets.py`, `timesheet_billing.py`. Independent of GL work; can run parallel to 5–6.

### Phase 8 — Collapse the inheritance chain
Flatten `SellingController` / `BuyingController` layers that are now pass-through. Last, because only safe once the delegated-to services exist.

**Dependencies:** 1→2→3 sequential; 4 and 7 can parallelize once 3 lands; 8 always last.

## Cross-cutting rules
- Public signatures stay stable — keep the `gl_map=` param and `make_gl_entries` intact. The `gl_map → gl_entries` rename is its own deprecation pass, deferred to the end (or excluded).
- Composers are classes (stateful, per-document); sink and validator are stateless module functions.
- Every phase: behavior-preserving, snapshot + `bench run-tests --site test-site-ai` green before merge, draft PR.