fix(sales-summaries): stop days falling out of balance #18

Open
notid wants to merge 9 commits from sales-summary-balancing-v2 into master
Showing only changes of commit 79a4b457b0 - Show all commits

View File

@@ -27,6 +27,7 @@ each its own copy of the refunds.
| Flag introduced | `summary-service-charges` — off by default | | Flag introduced | `summary-service-charges` — off by default |
| Migration to run once | `auto-ap.jobs.rekey-square-external-ids/migrate-all!` | | 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 | | Expected migration runtime | ~38 minutes for 19M orders on a warm cache |
| Backfill runtime (step 5) | ~5.9 hours for 90 days across the 20 shared-location records — an overnight job |
| Nothing here touches | invoices, payments, the ledger, or any client without the flag set | | 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 configuration is left exactly as it is.** Ten Square locations are configured against two
@@ -40,6 +41,11 @@ becomes **two entities, one per client record** — by design. That is the stabl
duplicate to clean up. If any report or export aggregates across client records, one restaurant's 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. takings would be counted twice at that layer. Nothing in this work changes that either way.
That holds automatically for everything imported *from now on*, because the keys carry the client.
It does **not** hold for history: refunds, payouts and shifts already in the database exist only
once, on whichever record imported them last, and re-keying freezes that rather than evening it out.
Step 5 is what brings the existing history into the same shape.
**The only window of risk is between deploying and finishing the migration**, while legacy keys **The only window of risk is between deploying and finishing the migration**, while legacy keys
still exist for a client to resolve. Steps 26 exist to make that window effectively zero. still exist for a client to resolve. Steps 26 exist to make that window effectively zero.
@@ -332,56 +338,51 @@ people doing the right thing. That is the reason not to let this drift.
**The most important item in this document, and the only one that is not just execution.** **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. 16 of the 122 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 and all 16 fall before that client's first ever order. Two clients are affected, holding **160
**660 refunds worth $15,237.02 dated before their own first order**: refunds worth $4,347.68 dated before their own first order**:
| Client | First order | Refunds before it | Value | | Client | First order | Refunds before it | Value | Days out of balance |
|---|---|---:|---:| |---|---|---:|---:|---:|
| NGDL | 2026-07-29 | 96 | $2,751.50 | | NG4S | 2026-05-29 | 79 | $2,180.08 | 10 |
| NGWN | 2026-07-30 | 95 | $2,218.61 | | NGPS | 2026-05-26 | 81 | $2,167.60 | 7 |
| 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 **Step 5's backfill already resolved the other seven.** Before it, nine records were in this state
26 February belonging to `NGDG` that day and taken over by `NGDU` on 12 August, with others flipping holding 660 refunds worth $15,237.02 — but seven of them were shared-location twins whose refunds
between the two several times a day across 1215 August. Unscoped refund keys meant whichever import only looked orphaned because their orders had never been imported. Replaying the window gave them
ran last took ownership — fault 1, seen from the other end. Steps 1 and 4 stop it recurring; they do their orders, and the refunds stopped predating them.
not undo it.
Note `NG4S` and `NGPS` are **not** among the ten shared locations, so this is wider than the NG4S and NGPS are different: neither shares a Square location, so there is no twin holding the other
duplicate records. half. Their sales genuinely are not in the system for the period their refunds cover. The database's
own ownership history is the evidence to check — for the twins it showed refunds changing hands
between the two records; for these two there is no second record to have taken them from.
Two ways to close it, and the business has to pick: 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 1. **Import the missing sales.** Correct if these records are meant to have their own books. Try
refunds are real and the matching orders exist in Square. `backfill-history` for them first, with a window reaching back before their first order — that is
2. **Move the refunds to the record that has the sales.** Correct if the record is a duplicate that exactly what fixed the seven, and it is one command.
should never have had books of its own. 2. **Move the refunds to the record that has the sales.** Correct only if the refunds were misfiled
onto a record that should not have books of its own.
Before either, check whether the surviving record already holds the same Square refund — within the Start with (1): it is cheap, reversible in the sense that it only adds what Square reports, and it
ninety-day window all 192 orphans did, but **outside it payouts and shifts have no counterpart at is already proven to work on this exact symptom.
all**, so a blanket delete would destroy the only copy of some records.
```clojure ```clojure
;; per client: refunds dated before that client's own first order ;; per client: refunds dated before that client's own first order
(let [first-order (->> (d/q '[:find [?d ...] :in $ ?c (let [first-order (->> (d/q '[:find [?d ...] :in $ ?c
:where [?o :sales-order/client ?c] [?o :sales-order/date ?d]] :where [?o :sales-order/client ?c] [?o :sales-order/date ?d]]
(d/db conn) [:client/code "NGDU"]) (d/db conn) [:client/code "NG4S"])
(reduce (fn [a b] (if (.before a b) a b))))] (reduce (fn [a b] (if (.before a b) a b))))]
(->> (d/q '[:find [(pull ?r [:sales-refund/date :sales-refund/total]) ...] :in $ ?c (->> (d/q '[:find [(pull ?r [:sales-refund/date :sales-refund/total]) ...] :in $ ?c
:where [?r :sales-refund/client ?c]] :where [?r :sales-refund/client ?c]]
(d/db conn) [:client/code "NGDU"]) (d/db conn) [:client/code "NG4S"])
(filter #(.before (:sales-refund/date %) first-order)) (filter #(.before (:sales-refund/date %) first-order))
count)) count))
``` ```
**Until this is resolved those days stay out of balance, on purpose.** A summary change to close **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 them was written and measured — it works, closes 16 days and $1,227.65, and breaks nothing — and it
was removed, because an unbalanced day is the only visible signal that a restaurant's sales are not 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. being imported. A test asserts the day stays unbalanced so nobody closes it without reading this.
@@ -389,14 +390,24 @@ being imported. A test asserts the day stays unbalanced so nobody closes it with
## What this will not fix ## 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 122 client-days over ninety days, $2,379.45, of which only 32 are above ten cents.
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 | | Days | Variance | |
step 11 but from the payout side, so it needs the payout modelled rather than a rule in the summary. |---|---:|---:|---|
| Real trading days with genuine discrepancies | 106 | $1,151.80 | see below |
| Refunds on a record with no sales imported | 16 | $1,227.65 | step 11 — deliberately visible |
Of the 106 trading days, only **3 are on shared-location records** — $648.84 in total, and all three
are already diagnosed: NGBK and NGBR at $299.42 each on 2026-08-06, where Square recorded $6,358.99
of tender against $6,059.57 of order totals (the gap itself, not a summary fault), and NGDA at
$50.00, an auto-gratuity booked as a service charge.
The other 103 days come to **$502.96 across 190 clients** — a few dollars here and there, mostly
till rounding, plus small undiagnosed clusters on NGMV ($259.38 over 5 days) and NGEB ($199.09 over
4 days, an ezCater fee-treatment question). Those two are worth a look but are not urgent.
That 103-day, $502.96 figure has been identical in every run of this analysis — with the duplicates
deactivated, with them live, and with them backfilled. It is the floor this work reaches.
--- ---