Bryce
4a1817711d
The report's figures were taken with one client record of each shared
pair deactivated and a live Square import run afterwards. That is no
longer how this deploys — both records stay live and the re-key is what
separates them — so the numbers described a configuration that will not
exist.
Re-run from scratch: fresh restore of backup point 209608347 (verified
back to 16,545,495 charges and legacy keys before starting), the new
month-wise migration, then two full ninety-day recomputes.
before 1,451 days out of balance / $81,023.96
after 542 days out of balance / $20,239.00
915 into balance, 6 out, 17,916 summaries untouched
Two corrections to claims that no longer hold:
- "zero days knocked out of balance" is now six. All six are the same
shape — the Tip line falls by a round amount and the day breaks by
exactly that — and all six are on shared-location records. Each is a
tip reversed on one record whose refund went to its twin: the fix makes
a hidden mis-attribution visible rather than causing one.
- the migration takes ~38 minutes, not ~13. The month-wise walk adds
per-month index overhead, and the earlier figure predated it.
What did NOT move is the part that should not. For the 190 clients that
do not share a Square location the residue is 119 days and $1,730.61 in
both runs, with the same five restaurants accounting for it (NG4S, NGMV,
NGEB, NGPS, N-30012). The arithmetic fixes behave identically whatever is
done to the duplicates, which is a stronger check than either run alone.
$18,508.39 of the $20,239.00 — 91% — sits on the twenty shared-location
records. The report now states plainly what retiring the duplicates would
be worth (~$12,000 of variance across ~260 client-days per ninety days),
while noting the comparison is not perfectly isolated because the earlier
run also included a live import.
Migration re-measured: 16,236,839 re-keyed, 500,438 cloned, plan reports
{:total 17045933 :to-migrate 0 :already-scoped 17045933 :unscopable 0},
and the multi-parent gate reads 0 across all 5,158,470 orders of the last
year. Step 10's refunds re-measured too: 660 worth $15,237.02 dated
before their own record's first order, 140 of the 171 no-sales days
falling before it.
31 tests, 76 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
363 lines
17 KiB
Markdown
363 lines
17 KiB
Markdown
# Sales-summary balancing — rollout plan
|
||
|
||
Steps to execute, in order. Every step is either reversible or verifiable before the next one
|
||
begins. The one behaviour change that alters a client's books is behind a per-client feature flag
|
||
that is **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, with the duplicate client records left active exactly as they will be in
|
||
production: **1,451 client-days out of balance / $81,023.96 → 542 days / $20,239.00**. 915 days
|
||
came into balance and 6 broke — all six being tips reversed on one record whose refund sits on its
|
||
twin, which the fix makes visible rather than causes.
|
||
|
||
**Where the remaining $20,239.00 sits is the thing to read.** $18,508.39 of it — 91% — is on the
|
||
twenty records that share a Square location. For the other 190 clients the residue is 119 days and
|
||
$1,730.61, which reproduces an earlier run on a differently prepared database to the penny.
|
||
|
||
Of the total, 171 days are not balancing faults at all — they are days where a client's sales were
|
||
never imported while its refunds were. Step 10 is about those.
|
||
|
||
---
|
||
|
||
## Before you start
|
||
|
||
| | |
|
||
|---|---|
|
||
| Flag introduced | `summary-service-charges` — off by default |
|
||
| Migration to run once | `auto-ap.jobs.rekey-square-external-ids/migrate-all!` |
|
||
| Expected migration runtime | ~38 minutes for 19M orders on a warm cache |
|
||
| Nothing here touches | invoices, payments, the ledger, or any client without the flag set |
|
||
|
||
**Client configuration is left exactly as it is.** Ten Square locations are configured against two
|
||
client records each, and both stay active. The re-key is what resolves them: once every record
|
||
carries its owner in its key, each client's import resolves only its own records and the two
|
||
records keep independent, stable histories. No "which record survives" decision is needed, and
|
||
nothing is deactivated.
|
||
|
||
The consequence to be aware of: each Square payment, refund, payout and shift at a shared location
|
||
becomes **two entities, one per client record** — by design. That is the stable end state, not a
|
||
duplicate to clean up. If any report or export aggregates across client records, one restaurant's
|
||
takings would be counted twice at that layer. Nothing in this work changes that either way.
|
||
|
||
**The only window of risk is between deploying and finishing the migration**, while legacy keys
|
||
still exist for a client to resolve. Steps 2–5 exist to make that window effectively zero.
|
||
|
||
---
|
||
|
||
## Step 1 — 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 4 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 4 verifies clean, 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 2 — Pause the Square importer
|
||
|
||
**This is what makes the deploy safe, and it is easy to skip.** Steps 2 through 5 should be one
|
||
maintenance action, not separate days' work.
|
||
|
||
While legacy keys exist, `square.core3/existing-id` falls back to them — and at a shared location
|
||
that is the one code path that can reach across client records. Running the migration with imports
|
||
paused means no client is resolving keys while the keys are being rewritten, so the window closes
|
||
entirely rather than merely narrowing.
|
||
|
||
The migration itself takes about **38 minutes** for all 19M orders, so the pause is short — and
|
||
if you need it shorter, see step 5: you can resume imports before it finishes.
|
||
|
||
---
|
||
|
||
## Step 3 — Deploy the code
|
||
|
||
Deploy the branch. The flag is absent from every client, so:
|
||
|
||
- tips are calculated exactly as they are today,
|
||
- no `Service Charges` line is written.
|
||
|
||
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 reads both key schemes**, so the deploy does not depend on the migration having
|
||
finished. Two protections cover the interval before it does: imports are paused (step 2), and
|
||
`existing-id` refuses to resolve a record that already belongs to a different client. Do not remove
|
||
the legacy lookup yet — see step 9.
|
||
|
||
---
|
||
|
||
## Step 4 — Run the migration
|
||
|
||
Run it immediately after the deploy, while imports are still paused.
|
||
|
||
```clojure
|
||
(require '[auto-ap.jobs.rekey-square-external-ids :as rk])
|
||
|
||
;; read-only first — no two entities may want the same key. `plan` does NOT return a
|
||
;; :collisions key; you have to hand its :new-keys to `collisions` yourself.
|
||
(rk/collisions (:new-keys (rk/plan (d/db conn) :charge/external-id rk/charge-prefix)))
|
||
;; => [] (anything else: stop, do not migrate)
|
||
|
||
;; then the whole thing
|
||
(rk/migrate-all! 2000)
|
||
```
|
||
|
||
`migrate-all!` runs this same check itself, on every attribute including charges, and throws
|
||
rather than transacting if it finds one. Running it by hand first just means finding out before
|
||
the 38-minute walk rather than partway through it.
|
||
|
||
Runs in about thirty-eight 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.
|
||
|
||
**It is also ordered so that stopping early is survivable.** Refunds, payouts and cash-drawer
|
||
shifts go first — a quarter of a million records, seconds of work — so an interruption cannot catch
|
||
them half done. The long part then walks orders **a month at a time, from the current month
|
||
backwards**, logging `::month-complete` as each finishes:
|
||
|
||
```
|
||
::month-complete :month "2026-08" :rekeyed 118203 :cloned 2244
|
||
::month-complete :month "2026-07" :rekeyed 241887 :cloned 4611
|
||
...
|
||
```
|
||
|
||
That ordering is the recovery plan. If it dies, everything from the last logged month forward is
|
||
fully scoped — and that recent window is what the importer actually reads — so **you can resume
|
||
imports against a partially migrated database** and finish the older tail later. Walking oldest
|
||
first would have spent the first several hours on 2019 data no import will touch, leaving exactly
|
||
the wrong end done.
|
||
|
||
If you do resume imports mid-migration, the ownership guard in `existing-id` is what keeps the
|
||
unmigrated tail safe: a client cannot resolve onto another client's legacy-keyed record.
|
||
|
||
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.** Two checks, doing two different jobs — run both.
|
||
|
||
**(a) Completeness, across everything.** `plan` must report nothing left to do, for all four
|
||
attributes:
|
||
|
||
```clojure
|
||
(dissoc (rk/plan (d/db conn) :charge/external-id rk/charge-prefix) :new-keys)
|
||
;; => {:total 17045933 :to-migrate 0 :already-scoped 17045933 :unscopable 0}
|
||
```
|
||
|
||
Read `:to-migrate 0` **and** `:unscopable 0`. This is the authoritative signal, and it covers all
|
||
17M charges.
|
||
|
||
`unscoped-report` is useful colour but is not the gate: its `:no-owner` column never reaches zero
|
||
for charges, because ~283k payout stubs carry no `:charge/client` of their own and it classifies
|
||
by attribute rather than by resolving ownership. Judge completeness by `plan`.
|
||
|
||
**(b) The safety gate for the cascade** — no payment may answer to two orders, or re-enabling
|
||
`remove-voided-orders` in step 8 can delete a payment another order still needs. Check **every**
|
||
order in the last year, with no sampling:
|
||
|
||
```clojure
|
||
(let [db (d/db conn)
|
||
cs (map first (d/q '[:find ?c :where [?c :client/code _]] db))
|
||
year (java.util.Date. (- (.getTime (java.util.Date.)) (long (* 365 86400000))))]
|
||
(rk/charges-with-multiple-parents
|
||
db (map first (iol-ion.query/scan-sales-orders db cs year nil))))
|
||
;; => 0
|
||
```
|
||
|
||
On the restored copy that is 5,158,470 orders — 27% of the table — via the
|
||
`:sales-order/client+date` index. A year is chosen deliberately: `remove-voided-orders` only ever
|
||
deletes orders Square reports as voided, which are recent, so that is where the destructive risk
|
||
lives. Completeness across all of history is check (a)'s job, not this one.
|
||
|
||
> Do **not** sample this with `(take n (rk/all-order-ids db))`. `all-order-ids` streams `:aevt`,
|
||
> which is ascending entity id, so a `take` returns the *oldest* orders — on the restored copy the
|
||
> first 400,000 are all from 2019–2021, before any of the contention this gate looks for. It would
|
||
> report a confident zero having inspected none of the relevant data.
|
||
|
||
---
|
||
|
||
## Step 5 — Resume the Square importer
|
||
|
||
Normally: once step 4's two checks read clean. The maintenance window ends here.
|
||
|
||
**If the migration did not finish**, you do not have to wait for it. Resume imports once the
|
||
`::month-complete` log covers the window your importer reads — the last 75 days for payouts and
|
||
cash-drawer shifts, and whatever range the order import is configured for. Then re-run
|
||
`migrate-all!` afterwards to walk the remaining older months; it will skip everything already done.
|
||
Run the step 4 checks again once it does finish.
|
||
|
||
The first cycle after resuming is the one to watch. Compare these against the same counts taken
|
||
immediately before the deploy — growth should be ordinary daily volume:
|
||
|
||
```clojure
|
||
(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))
|
||
(count (d/datoms (d/db conn) :aevt :charge/external-id))
|
||
```
|
||
|
||
A near-doubling of any of them means records are being created rather than matched — **stop and
|
||
roll back the deploy.** Charges are included deliberately: they are the one that doubles a client's
|
||
takings rather than merely duplicating a row.
|
||
|
||
---
|
||
|
||
## Step 6 — 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 7, 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 7 — Turn the flag on, a few restaurants at a time
|
||
|
||
Needs accounting sign-off first: `summary-service-charges` posts to **49000 Service Income**, chosen
|
||
so the work could be measured. It affects reporting, never whether a day balances.
|
||
|
||
```clojure
|
||
@(d/transact conn [{:db/id [:client/code "NGxx"]
|
||
:client/feature-flags ["summary-service-charges"]}])
|
||
(ss/refresh-sales-summaries 90)
|
||
```
|
||
|
||
Start with two or three restaurants, confirm, then widen.
|
||
|
||
**Verify** against the capture from step 6:
|
||
|
||
```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 flag for the affected clients and re-run step 6. The flag is the
|
||
rollback: removing it restores today's behaviour exactly.
|
||
|
||
---
|
||
|
||
## Step 8 — Re-enable `remove-voided-orders`
|
||
|
||
Safe once step 4's gate reads zero. Keep the detach-rather-than-delete guard from step 1.
|
||
|
||
---
|
||
|
||
## Step 9 — Remove the legacy key lookup
|
||
|
||
**Schedule this; do not leave it open-ended.** Both client records at a shared location stay active
|
||
permanently, so the legacy fallback in `square.core3/existing-id` is the one code path that can ever
|
||
reach across them. Deleting it is what turns the guarantee from conventional into structural.
|
||
|
||
Once `plan` reports `:to-migrate 0` and has stayed there through several import cycles, drop the
|
||
legacy branch of `existing-id` — and with it `owned-by-other-client?`, which exists only to make
|
||
that branch safe while it lives. After this, two clients on one location are structurally incapable
|
||
of resolving onto each other's records, and no ordering discipline is required to keep it that way.
|
||
|
||
Until it is done, the protection is the guard plus the maintenance window, both of which depend on
|
||
people doing the right thing. That is the reason not to let this drift.
|
||
|
||
---
|
||
|
||
## Step 10 — Deal with the refunds that have no sales behind them
|
||
|
||
**The most important item in this document, and the only one that is not just execution.**
|
||
|
||
171 of the 542 remaining days are a record carrying refunds on a day it recorded no sales at all.
|
||
140 of those fall *before that client's first ever order*. Nine records are affected, holding
|
||
**660 refunds worth $15,237.02 dated before their own first order**:
|
||
|
||
| Client | First order | Refunds before it | Value |
|
||
|---|---|---:|---:|
|
||
| NGDL | 2026-07-29 | 96 | $2,751.50 |
|
||
| NGWN | 2026-07-30 | 95 | $2,218.61 |
|
||
| NGDU | 2026-08-02 | 94 | $2,188.44 |
|
||
| NG4S | 2026-05-29 | 79 | $2,180.08 |
|
||
| NGPS | 2026-05-26 | 81 | $2,167.60 |
|
||
| NGCT | 2026-08-01 | 98 | $1,789.49 |
|
||
| NGVC | 2026-08-02 | 95 | $1,594.17 |
|
||
| NGDS | 2026-01-14 | 22 | $347.13 |
|
||
|
||
They did not make those refunds. The database's own ownership history shows a $35.35 refund dated
|
||
26 February belonging to `NGDG` that day and taken over by `NGDU` on 12 August, with others flipping
|
||
between the two several times a day across 12–15 August. Unscoped refund keys meant whichever import
|
||
ran last took ownership — fault 1, seen from the other end. Steps 1 and 4 stop it recurring; they do
|
||
not undo it.
|
||
|
||
Note `NG4S` and `NGPS` are **not** among the ten shared locations, so this is wider than the
|
||
duplicate records.
|
||
|
||
Two ways to close it, and the business has to pick:
|
||
|
||
1. **Import the missing sales.** Correct if these records are meant to have their own books. Their
|
||
refunds are real and the matching orders exist in Square.
|
||
2. **Move the refunds to the record that has the sales.** Correct if the record is a duplicate that
|
||
should never have had books of its own.
|
||
|
||
Before either, check whether the surviving record already holds the same Square refund — within the
|
||
ninety-day window all 192 orphans did, but **outside it payouts and shifts have no counterpart at
|
||
all**, so a blanket delete would destroy the only copy of some records.
|
||
|
||
```clojure
|
||
;; per client: refunds dated before that client's own first order
|
||
(let [first-order (->> (d/q '[:find [?d ...] :in $ ?c
|
||
:where [?o :sales-order/client ?c] [?o :sales-order/date ?d]]
|
||
(d/db conn) [:client/code "NGDU"])
|
||
(reduce (fn [a b] (if (.before a b) a b))))]
|
||
(->> (d/q '[:find [(pull ?r [:sales-refund/date :sales-refund/total]) ...] :in $ ?c
|
||
:where [?r :sales-refund/client ?c]]
|
||
(d/db conn) [:client/code "NGDU"])
|
||
(filter #(.before (:sales-refund/date %) first-order))
|
||
count))
|
||
```
|
||
|
||
**Until this is resolved those days stay out of balance, on purpose.** A summary change to close
|
||
them was written and measured — it works, closes 171 days and $5,795.18, and breaks nothing — and it
|
||
was removed, because an unbalanced day is the only visible signal that a restaurant's sales are not
|
||
being imported. A test asserts the day stays unbalanced so nobody closes it without reading this.
|
||
|
||
---
|
||
|
||
## What this will not fix
|
||
|
||
The 371 real trading days still out of balance, $14,443.82 — of which $13,940.86 is on the twenty
|
||
shared-location records, where the two books disagree about who owns what. For every other client
|
||
it is 103 days and $502.96. Named causes: the NGBR/NGBK tender gap ($6,358.99 of
|
||
tender against $6,059.57 of order totals on one day), the ezCater fee question, and unexplained
|
||
clusters on NGMV and NGEB.
|
||
|
||
Plus 16 days / $1,227.65 where a processing fee lands on a day with no trading — the same shape as
|
||
step 10 but from the payout side, so it needs the payout modelled rather than a rule in the summary.
|
||
|
||
---
|
||
|
||
## 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.
|