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

notid · 2026-08-15T19:20:52-07:00

notid commented

2026-08-15 19:20:52 -07:00

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), running the real nightly job twice: once with the fixes off, once with them on.

	Days out of balance	Clean	Variance
Today's calculation, ninety days re-run	1,258	93.34%	$69,560.10
+ refunded tips	971	94.86%	$66,414.39
+ service charges	279	98.52%	$7,790.54
+ returns on refund-only days	123	99.35%	$2,970.35

1,135 days repaired, none knocked out of balance, and not one already-balanced day altered — verified line by line (category, debit/credit side, amount to the cent, account), not just on each day's bottom line. Of the 123 days remaining, only 33 are above ten cents.

📊 Full report: https://claude.ai/code/artifact/58910b7e-45e0-40c9-956d-a6932d548dea
📋 Rollout plan: docs/2026-08-15-sales-summary-rollout-plan.md — steps to execute, in order

Nothing changes on deploy

Both behaviour changes sit behind per-client feature flags that are off by default:

summary-service-charges
summary-refund-only-returns

Merging and deploying changes no client's books until one is opted in. Removing a flag is the rollback and restores today's behaviour exactly.

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. See docs/2026-08-15-remove-voided-orders-risk.md.

Three fixes: client-scope the four key schemes; look the record up under both schemes so the change can deploy before the migration finishes; and a migration giving every order its own payment.

Run over the whole database: 19,040,785 orders walked, 9,100,314 payments re-keyed, 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 — which has no tender to join through — contributed nothing while its reversal sat unread on :sales-order/tip. The fix is additive rather than substitutive on purpose: 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 at all. Now credited for Square orders only, both signs.

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. When they fall on different days the credit has no debit anywhere.

This last one is the answer to why the duplicated restaurants looked so much worse than everyone else: 155 of the 158 days still failing on them had no sales orders at all — the summary was nothing but orphaned refunds and their fees. The refunds were claimed by whichever record imported them during the shared period; the orders went to the other. Nothing was double-counted and no money was lost; the two halves of each refund simply ended up on two sets of books.

It is not a duplicate-only problem, which is why it is fixed properly rather than papered over — the largest non-duplicate cluster, NG4S, has the identical shape.

get-returns now falls back to the day's refunded total, but only where the client recorded no sales orders whatsoever. That guard is the whole design: with no orders there is no order-derived return to double-count and no trading day can be moved. Days that did trade and still carry an unmatched refund are left alone deliberately — apportioning a return across a day that also sold is an accounting question, not a mechanical one.

Supporting changes

Change	Why
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	an out-of-balance day was only visible by opening the screen
Bound the dirty-summary scan to one client	1,321 ms → 5.6 ms per client
`compare-sales-summaries` moved to `test/clj` as `auto-ap.tools.*`	a verification harness, not part of the running application — nothing in production should be able to depend on it

One correction worth reading

d/as-of cannot be used to compare summary amounts. :ledger-mapped/amount, ledger-side and account are all :db/noHistory, so superseded values are discarded — a recomputed summary reads back through as-of with its categories intact and its amounts absent, which is indistinguishable from a legitimate all-zero balanced day. Every figure in the report comes from live captures taken straight after each pass. The tool's docstring now says so where someone will actually hit it.

Needs a decision before rollout

Which client record survives at each shared location (business). The newer record usually has no history from before the split, so keeping it loses years of that location's books.
Which account service charges post to (accounting). Currently 49000 Service Income, chosen so the work could be measured; it affects reporting, never whether a day balances.
Recognising a return on a refund-only day (accounting). Posts to 41300 Returns, the account already used for returns, but it moves the recognition date — a refund settling after month end lands in the later period.

What this does not fix

123 client-days over ninety days, $2,970.35:

108 days / $1,995.36 — real trading days with genuine discrepancies: 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.
15 days / $974.99 — a processing fee landing on a day with no trading. Same shape as the refund-only day, but the offsetting entry belongs to the payout rather than to sales, so it needs the payout side modelled rather than another rule in the summary.

Testing

lein test auto-ap.jobs.sales-summaries-test auto-ap.square.core3-test auto-ap.jobs.rekey-square-external-ids-test — 28 tests, 65 assertions, 0 failures.

Beyond unit tests, the whole analysis was rebuilt from a fresh restore of the production backup and reproduced the previous run to the cent at the point the two are comparable — 279 days and $7,790.54, with not one client-day differing by half a penny.

🤖 Generated with Claude Code

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), running the real nightly job twice: once with the fixes off, once with them on. | | Days out of balance | Clean | Variance | |---|---:|---:|---:| | Today's calculation, ninety days re-run | 1,258 | 93.34% | $69,560.10 | | + refunded tips | 971 | 94.86% | $66,414.39 | | + service charges | 279 | 98.52% | $7,790.54 | | **+ returns on refund-only days** | **123** | **99.35%** | **$2,970.35** | **1,135 days repaired, none knocked out of balance, and not one already-balanced day altered** — verified line by line (category, debit/credit side, amount to the cent, account), not just on each day's bottom line. Of the 123 days remaining, only 33 are above ten cents. 📊 **Full report:** https://claude.ai/code/artifact/58910b7e-45e0-40c9-956d-a6932d548dea 📋 **Rollout plan:** `docs/2026-08-15-sales-summary-rollout-plan.md` — steps to execute, in order --- ## Nothing changes on deploy Both behaviour changes sit behind per-client feature flags that are **off by default**: - `summary-service-charges` - `summary-refund-only-returns` Merging and deploying changes no client's books until one is opted in. Removing a flag is the rollback and restores today's behaviour exactly. ## 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. See `docs/2026-08-15-remove-voided-orders-risk.md`. Three fixes: client-scope the four key schemes; look the record up under **both** schemes so the change can deploy before the migration finishes; and a migration giving every order its own payment. Run over the whole database: **19,040,785 orders walked, 9,100,314 payments re-keyed, 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 — which has no tender to join through — contributed nothing while its reversal sat unread on `:sales-order/tip`. The fix is additive rather than substitutive on purpose: 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` at all. Now credited for Square orders only, both signs. **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. When they fall on different days the credit has no debit anywhere. This last one is the answer to why the duplicated restaurants looked so much worse than everyone else: **155 of the 158 days still failing on them had no sales orders at all** — the summary was nothing but orphaned refunds and their fees. The refunds were claimed by whichever record imported them during the shared period; the orders went to the other. Nothing was double-counted and no money was lost; the two halves of each refund simply ended up on two sets of books. It is not a duplicate-only problem, which is why it is fixed properly rather than papered over — the largest non-duplicate cluster, `NG4S`, has the identical shape. `get-returns` now falls back to the day's refunded total, **but only where the client recorded no sales orders whatsoever**. That guard is the whole design: with no orders there is no order-derived return to double-count and no trading day can be moved. Days that *did* trade and still carry an unmatched refund are left alone deliberately — apportioning a return across a day that also sold is an accounting question, not a mechanical one. ## Supporting changes | Change | Why | |---|---| | 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 | an out-of-balance day was only visible by opening the screen | | Bound the dirty-summary scan to one client | 1,321 ms → 5.6 ms per client | | `compare-sales-summaries` moved to `test/clj` as `auto-ap.tools.*` | a verification harness, not part of the running application — nothing in production should be able to depend on it | ## One correction worth reading `d/as-of` **cannot** be used to compare summary amounts. `:ledger-mapped/amount`, `ledger-side` and `account` are all `:db/noHistory`, so superseded values are discarded — a recomputed summary reads back through `as-of` with its categories intact and its **amounts absent**, which is indistinguishable from a legitimate all-zero balanced day. Every figure in the report comes from live captures taken straight after each pass. The tool's docstring now says so where someone will actually hit it. ## Needs a decision before rollout - **Which client record survives at each shared location** (business). The newer record usually has no history from before the split, so keeping it loses years of that location's books. - **Which account service charges post to** (accounting). Currently 49000 Service Income, chosen so the work could be measured; it affects reporting, never whether a day balances. - **Recognising a return on a refund-only day** (accounting). Posts to 41300 Returns, the account already used for returns, but it moves the recognition *date* — a refund settling after month end lands in the later period. ## What this does not fix 123 client-days over ninety days, $2,970.35: - **108 days / $1,995.36** — real trading days with genuine discrepancies: 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. - **15 days / $974.99** — a processing fee landing on a day with no trading. Same shape as the refund-only day, but the offsetting entry belongs to the payout rather than to sales, so it needs the payout side modelled rather than another rule in the summary. ## Testing `lein test auto-ap.jobs.sales-summaries-test auto-ap.square.core3-test auto-ap.jobs.rekey-square-external-ids-test` — **28 tests, 65 assertions, 0 failures**. Beyond unit tests, the whole analysis was rebuilt from a fresh restore of the production backup and reproduced the previous run **to the cent** at the point the two are comparable — 279 days and $7,790.54, with not one client-day differing by half a penny. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

notid added 4 commits 2026-08-15 19:20:52 -07:00

feat(sales-summaries): refresh on a schedule, skipping accepted days 95d0537c69

sales-summaries-v2 recomputes every dirty summary, but nothing set the dirty
flag on a schedule: mark-all-dirty was only ever called by hand from the
comment block, and the job was registered in neither server.clj's
INTEGREAT_JOB dispatch nor terraform/deploy.tf, so -main was dead code that
could never run in production. Summaries were only recalculated when someone
remembered to do it in the REPL, and POS data keeps arriving after a business
day closes, so a summary computed once on the day was routinely wrong and
stayed wrong.

Add a daily job that marks the trailing 7 days dirty and recomputes them,
leaving finished work alone. "Finished" is the condition the app already calls
Balanced -- debits equal credits and every line is mapped to an account. Since
that is derived rather than stored, a summary that later falls out of balance
is picked up again on the next run.

Extract the Balanced predicate into auto-ap.datomic.sales-summaries so the
grid's pill and the job share one definition, rather than a background job
requiring an SSR namespace. total-debits/total-credits resolve the ledger side
from either a plain keyword or the {:db/ident ...} map a pull returns, and
accepted? requires every item to declare a side: un-normalized pulled items
otherwise sum to 0.0 on both sides, read as balanced, and get skipped
silently and permanently.

Also fix sales-summaries-v2 destroying user-entered line items. It filtered
for :sales-summary-item/manual? to preserve them, but dirty-sales-summaries'
index-pull selector never fetched :sales-summary/items, so manual-items was
always empty. Because items is a component attribute upserted via
[:reset-rels ...], every recompute deleted the hand-entered lines -- often the
very lines that make a summary balance. Harmless while nothing ran on a
schedule; destructive the moment this does.

Register the job in the admin Background Jobs dropdown too, with a days
field: schedules are prod-only, so the admin page is the only way to run it
on staging.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Merge branch 'feat/scheduled-sales-summary-refresh' 8af9bfdee2

update 943bc18842

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

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>

notid closed this pull request

2026-08-15 20:18:55 -07:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: notid/integreat#17