Four faults were leaving restaurant days out of balance — one in the data, three in the arithmetic. Measured over ninety days on a restored copy of production (210 clients, 18,900 client-days): 1,258 days out of balance and $69,560.10 becomes 123 days and $2,970.35, of which only 33 are above ten cents. 1,135 days repaired, none knocked out of balance, and not one already-balanced day altered — verified line by line (category, side, amount to the cent, account), not just on each day's bottom line. THE DATA FAULT Ten Square locations were configured against two client records each. Sales orders scoped their identifier by client; refunds, card payments, payouts and cash-drawer shifts used the bare Square id. Those attributes are :db.unique/identity, so both clients' imports resolved to a single entity and the last writer won — 3,387 refunds, 4,069 payouts and 2,628 cash-drawer shifts changed hands over time, across 19 client pairs of which only 10 are visible in today's configuration. Worse, one payment could belong to two orders. :sales-order/charges is :db/isComponent, so removing a voided order cascaded into payments the other client still needed. Fixes: client-scope the four key schemes; look the record up under both schemes so the change deploys before the migration finishes; and a migration that gives every order its own payment. Run over the whole database that is 19,040,785 orders walked, 9,100,314 payments re-keyed and 200,027 copied, ending with 17,047,142 payments scoped, none left to rename, none unscopable, and no payment owned by more than one order. Idempotent and resumable; about thirteen minutes. THE ARITHMETIC FAULTS - Refunded tips stayed on the books. get-tip summed tips by joining through :sales-order/charges, so a return-only order — no tender to join through — contributed nothing while its reversal sat unread on :sales-order/tip. Additive, not substitutive: where an order does have a tender the tender is the correct source. - Service charges were collected but never earned. Nothing read :sales-order/service-charge. Now credited for Square orders only, both signs, behind summary-service-charges. - A refund on a day with no sales had nothing to offset it. Refunds are credited on the day the money goes back; the return that offsets them is read from that day's orders. get-returns now falls back to the day's refunded total, but only where the client recorded no sales orders at all — with no orders there is no order-derived return to double-count and no trading day can be moved. Behind summary-refund-only-returns. Both flags are off by default, so deploying this changes nothing until a client is opted in. docs/2026-08-15-sales-summary-rollout-plan.md has the steps. SUPPORTING - Install schema attributes before the tuples that compose them. A tuple in schema.edn is built from an attribute in cloud-migration-schema.edn, so every test fixture died in setup — very likely why sales summaries had no tests before this. - Log each day's imbalance and its suspect lines. - Bound the dirty-summary scan to one client: 1,321 ms to 5.6 ms. - compare-sales-summaries lives in test/clj as auto-ap.tools.* — it is a verification harness, not part of the running application. Its docstring now warns that d/as-of cannot be used to compare summary amounts: :ledger-mapped/amount, ledger-side and account are :db/noHistory, so a recomputed summary reads back with its amounts absent and looks like a legitimate balanced day. 28 tests, 65 assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
218 lines
8.8 KiB
Markdown
218 lines
8.8 KiB
Markdown
# Sales-summary balancing — rollout plan
|
|
|
|
Steps to execute, in order. Every step is either reversible or verifiable before the next one
|
|
begins. Both behaviour changes are behind per-client feature flags that are **off by default**, so
|
|
merging and deploying this branch changes nothing on its own.
|
|
|
|
Measured on a restored copy of production (backup point `209608347`), 210 clients over
|
|
2026-05-10 → 2026-08-07: **1,258 client-days out of balance / $69,560.10 → 123 days / $2,970.35**,
|
|
with zero days knocked out of balance and zero already-balanced days altered.
|
|
|
|
---
|
|
|
|
## Before you start
|
|
|
|
| | |
|
|
|---|---|
|
|
| Branch | `worktree-sales-summary-balance` |
|
|
| Flags introduced | `summary-service-charges`, `summary-refund-only-returns` — both off by default |
|
|
| Migration to run once | `auto-ap.jobs.rekey-square-external-ids/migrate-all!` |
|
|
| Expected migration runtime | ~13 minutes for 19M orders on a warm cache |
|
|
| Nothing here touches | invoices, payments, the ledger, or any client without the flags set |
|
|
|
|
**One prerequisite that is not code.** Ten Square locations are configured against two client records
|
|
each. Someone in the business has to decide which record survives at each. The newer record usually
|
|
has no history from before the split, so keeping it loses years of that location's books. Do this
|
|
before step 3.
|
|
|
|
---
|
|
|
|
## Step 1 — Deploy the code
|
|
|
|
Deploy the branch as normal. Both flags are absent from every client, so:
|
|
|
|
- tips are calculated exactly as they are today,
|
|
- no `Service Charges` line is written,
|
|
- no `Returns` line is written on refund-only days.
|
|
|
|
The only changes that take effect immediately are the safe ones: imbalance logging, the
|
|
dirty-summary scan bounded to one client (1,321 ms → 5.6 ms per client), the schema-ordering fix,
|
|
and the importer's new client-scoped keys.
|
|
|
|
**The importer starts writing client-scoped keys straight away, and reads both schemes.** That is
|
|
deliberate and is what makes the deploy independent of the migration. Do not remove the legacy
|
|
lookup in `square.core3/existing-id` yet — see step 8.
|
|
|
|
**Verify before moving on.** After one nightly import cycle:
|
|
|
|
```clojure
|
|
;; refunds, payouts and shifts must not have doubled
|
|
(count (d/datoms (d/db conn) :aevt :sales-refund/external-id))
|
|
(count (d/datoms (d/db conn) :aevt :expected-deposit/external-id))
|
|
(count (d/datoms (d/db conn) :aevt :cash-drawer-shift/external-id))
|
|
```
|
|
|
|
Compare against the same counts taken immediately before deploy. Growth should be ordinary daily
|
|
volume. A near-doubling means the legacy fallback is not working — **stop and roll back the deploy**.
|
|
|
|
---
|
|
|
|
## Step 2 — Guard `remove-voided-orders`
|
|
|
|
Do this before the migration, not after. `:sales-order/charges` is `:db/isComponent true`, so
|
|
retracting an order cascades into its payments. Until step 3 finishes there are still payments with
|
|
two parent orders, and deleting one client's voided order can take the other client's payment with
|
|
it.
|
|
|
|
Either leave `remove-voided-orders` switched off until step 3 completes, or change it to detach a
|
|
payment that has more than one parent rather than delete it. Detaching is worth doing regardless —
|
|
it makes the safety a property of the deletion rather than of the migration having been run first.
|
|
|
|
See `docs/2026-08-15-remove-voided-orders-risk.md`.
|
|
|
|
---
|
|
|
|
## Step 3 — Retire the duplicate client records
|
|
|
|
Business decision from the top of this document. Deactivate the losing record's Square location so
|
|
the importer stops fetching for it. The record itself stays; its history is untouched.
|
|
|
|
**Verify:** no Square location is configured against two active client records.
|
|
|
|
---
|
|
|
|
## Step 4 — Run the migration
|
|
|
|
```clojure
|
|
(require '[auto-ap.jobs.rekey-square-external-ids :as rk])
|
|
|
|
;; read-only first — check :collisions is empty for every attribute
|
|
(dissoc (rk/plan (d/db conn) :charge/external-id rk/charge-prefix) :new-keys)
|
|
|
|
;; then the whole thing
|
|
(rk/migrate-all! 2000)
|
|
```
|
|
|
|
Runs in about thirteen minutes over 19M orders. It is **idempotent and resumable** — a record that
|
|
already carries the right name is skipped, so it can be stopped and re-run without consequence.
|
|
|
|
If it appears to crawl, the cause is almost certainly garbage collection in the process driving it,
|
|
not the transactor. That misdiagnosis cost two days of projected runtime during this work. Free
|
|
retained memory in the REPL and re-measure before changing anything about the database.
|
|
|
|
**Verify — all four must read zero to migrate and zero unscopable:**
|
|
|
|
```clojure
|
|
(rk/unscoped-report (d/db conn))
|
|
(dissoc (rk/plan (d/db conn) :charge/external-id rk/charge-prefix) :new-keys)
|
|
;; => {:total 17047142 :to-migrate 0 :already-scoped 17047142 :unscopable 0}
|
|
|
|
;; and the gate that this work exists for
|
|
(rk/charges-with-multiple-parents (d/db conn) (take 400000 (rk/all-order-ids (d/db conn))))
|
|
;; => 0
|
|
```
|
|
|
|
Note `unscoped-report`'s `:no-owner` column is not a gap: ~283k payout-stub payments carry no
|
|
`:charge/client` attribute of their own, so it cannot verify them by attribute. `plan` resolves
|
|
ownership through whatever refers to them and is the figure to trust.
|
|
|
|
---
|
|
|
|
## Step 5 — Recompute summaries, flags still off
|
|
|
|
```clojure
|
|
(require '[auto-ap.jobs.sales-summaries :as ss])
|
|
(ss/refresh-sales-summaries 90)
|
|
```
|
|
|
|
This is the pass that banks the deduplication. **Capture the result before going further** — you
|
|
will need it as the baseline for step 6, and it cannot be reconstructed afterwards:
|
|
|
|
```clojure
|
|
(require '[auto-ap.tools.compare-sales-summaries :as cmp]) ; test/dev classpath
|
|
(def before (cmp/summaries-in (d/db conn) start end))
|
|
(spit "before.edn" (pr-str before))
|
|
```
|
|
|
|
> **Do not use `d/as-of` to compare summary amounts.** `:ledger-mapped/amount`, `ledger-side` and
|
|
> `account` are `:db/noHistory`, so past values are discarded. A summary that has since been
|
|
> recomputed reads back through `as-of` with its amounts *absent*, which looks like a legitimate
|
|
> balanced day. Capture live, before and after, and diff the captures.
|
|
|
|
---
|
|
|
|
## Step 6 — Turn the flags on, a few restaurants at a time
|
|
|
|
Needs accounting sign-off first, on two points:
|
|
|
|
- `summary-service-charges` posts to **49000 Service Income**. Chosen so the work could be measured.
|
|
It affects reporting, never whether a day balances.
|
|
- `summary-refund-only-returns` posts to **41300 Returns**, the account already used for returns. It
|
|
moves the *recognition date* of a return onto the day the refund settled, so a refund settling
|
|
after month end lands in the later period.
|
|
|
|
```clojure
|
|
@(d/transact conn [{:db/id [:client/code "NGxx"]
|
|
:client/feature-flags ["summary-service-charges"
|
|
"summary-refund-only-returns"]}])
|
|
(ss/refresh-sales-summaries 90)
|
|
```
|
|
|
|
Start with two or three restaurants, confirm, then widen.
|
|
|
|
**Verify** against the capture from step 5:
|
|
|
|
```clojure
|
|
(def after (cmp/summaries-in (d/db conn) start end))
|
|
(cmp/compare-window ...) ; both arguments live database values, never as-of
|
|
```
|
|
|
|
The two numbers that matter — both were zero across all 18,900 client-days in testing:
|
|
|
|
- `:balanced->unbalanced` must be **0**
|
|
- previously-balanced days whose lines changed must be **0**
|
|
|
|
If either is non-zero, retract the flags for the affected clients and re-run step 5. The flags are
|
|
the rollback: removing them restores today's behaviour exactly.
|
|
|
|
---
|
|
|
|
## Step 7 — Re-enable `remove-voided-orders`
|
|
|
|
Safe once step 4's gate reads zero. Keep the detach-rather-than-delete guard from step 2.
|
|
|
|
---
|
|
|
|
## Step 8 — Remove the legacy key lookup
|
|
|
|
Only once `plan` reports `:to-migrate 0` and has stayed there through several import cycles. Drop
|
|
the second branch of `square.core3/existing-id`. At that point two clients sharing a location
|
|
becomes structurally incapable of producing a shared record, rather than prevented by a convention a
|
|
future import could break.
|
|
|
|
This is the last step and there is no hurry.
|
|
|
|
---
|
|
|
|
## What this will not fix
|
|
|
|
123 client-days over ninety days, $2,970.35, of which only 33 are above ten cents.
|
|
|
|
| | Days | Variance | |
|
|
|---|---:|---:|---|
|
|
| Real trading days with genuine discrepancies | 108 | $1,995.36 | the NGBR/NGBK tender gap, the ezCater fee question, unexplained clusters on NGMV and NGEB |
|
|
| Processing fee on a day with no trading | 15 | $974.99 | same shape as the refund-only day, but the offsetting entry belongs to the payout, not to sales |
|
|
|
|
Refunds landing on days that *did* trade are deliberately left alone. Apportioning a return across a
|
|
day that also sold is a judgement call, and guessing at it is how correct books get quietly
|
|
rewritten.
|
|
|
|
---
|
|
|
|
## Two operational findings, unrelated to the summaries
|
|
|
|
- **The production backup had not written a restore point since 2025-03-10** — about seventeen
|
|
months — although data files were still uploading daily. Worth an alert on restore-point age.
|
|
- **The database server is sized for a much smaller dataset**: a 2 GB cache against 27 GB of data.
|
|
Worth checking what production is set to.
|