Skip to content

fix(api): account for every reengagement candidate - #296

Merged
GSTJ merged 4 commits into
mainfrom
fix/reengagement-accounting
Sep 7, 2026
Merged

fix(api): account for every reengagement candidate#296
GSTJ merged 4 commits into
mainfrom
fix/reengagement-accounting

Conversation

@GSTJ

@GSTJ GSTJ commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Summary

Every candidate the re-engagement cron looks at now leaves the run either sent or suppressed with a reason, instead of six branches that ended a person's turn counting nothing.
The daily readout gains a per run table so the same arithmetic can be checked from the outside.

Details

  • Six branches used to end without a send and without a reason: a user deleted between the selector and the decision, a run that hit its push ceiling, a token Expo refuses, a queue claimed by a racing run, a queue claimed before the run even started, and a send that threw.
  • The last two are the ones that mattered most. A queue already claimed when the run began was filtered out before the grouping, so the person never reached the decision loop and appeared in no count but a row level tally. On an hourly cron that is most of the people the job looks at.
  • The send that threw was the other one. There was no per user catch, so one failure escaped the whole run, took every person behind it, and took the heartbeat with it. That hour reported nothing at all, which reads exactly like a cron that stopped being scheduled. The heartbeat now goes out from a finally, so a failure in the selectors or the cadence read still reports the hour and still surfaces as an error rather than as a quiet evening.
  • Three of the six branches are decisions about the person and now carry a reason, under two names: dead_token for a token Expo will not take, and a new already_sent for a queue with nothing left to claim, whether the claim came from an earlier run or from one racing this one.
  • The cadence is decided before the queue as well as before the clock. A claimed key is a log row, and that same row is what puts the person inside the weekly floor, so naming the key first would have put already_sent on most of the base and buried every reason anybody can act on. It is left for the narrow case it describes: due, inside their own hour, and nothing left to say.
  • A run that throws before it finishes deciding now says so on its heartbeat and in the readout. Every count on a failed run is whatever it had reached, which for a failure in the selection is zero across the board, and that is the row a quiet evening produces too.
  • The other three are the absence of a decision, so they get their own held count rather than a reason that would read as one. It reads zero in a healthy run. At the push ceiling a non zero held is a backlog waiting for the next hour rather than anybody lost.
  • The run summary and the hourly heartbeat gain people, the distinct users behind the candidate rows. candidates counts rows and one person can hold several, so sent and suppressed were never meant to add up to it. They add up to people, together with held, and the tests assert that on every path.
  • Readout: a new table of the last 24 runs with the hour, candidates, people, sent, suppressed, held and the reasons that were not zero. The single row heartbeat cannot tell an hour that sent nothing from a day that sent nothing.
  • The selectors move to their own module. The service was over its line budget and the two halves answer different questions, who is due and who gets it. Every name callers already imported is re-exported from the old path, so no call site moves.
  • No change to who gets a push or when. The cadence, the caps and the evening window are untouched.
  • Worth recording for whoever reads the readout next: there is no timezone column on the user, so a user with coordinates and a user without both resolve a local hour the same way, longitude when there is one and America/Sao_Paulo when there is not. Neither is where the missing candidates went.
  • The metric this serves is retention, and the open rate of these pushes is unreadable while the denominator leaks. The number to watch is held on the hourly heartbeat, and the new table is where a run that lost somebody becomes visible within the hour rather than after a day of digging.

Testing steps

  1. Open the daily metrics issue after the next run and find the re-engagement section.
  2. Read the new table of the last 24 hours. Each row is one run of the job.
  3. Check that the people column equals sent plus suppressed plus held on every row. That is the whole point of the change.
  4. Check that the held column reads zero. The one case where it should not is an hour that reached the send ceiling, and that reads as a backlog for the next hour rather than anybody lost.
  5. Check that a row with nobody sent names the rule that held them in the reasons column, rather than leaving it blank.

Screenshots

No user facing surface changes. The visible output is the new readout table:

| Hour (UTC) | Candidates | People | Sent | Suppressed | Held | Reasons |
| --- | ---: | ---: | ---: | ---: | ---: | --- |
| 2026-09-07 21:00 UTC | 7 | 5 | 3 | 2 | 0 | cooldown 1, window 1 |
| 2026-09-07 20:00 UTC | 5 | 3 | 0 | 3 | 0 | window 3 |
| 2026-09-07 19:00 UTC | 5 | 3 | 0 | 3 | 0 | cooldown 3 |
| 2026-09-07 18:00 UTC | 0 | 0 | 0 | 0 | 0 | run failed |

GSTJ added 4 commits September 7, 2026 06:56
The hourly heartbeat could report five candidates, zero sent and three
suppressed, and the missing two were not a rounding error. Several branches
ended a person's turn with no send and no reason: a deleted user, a full run,
a token Expo refuses, a queue another run had claimed, and a send that threw.

The last of those also took the heartbeat with it, so the hour reported
nothing at all and an outage read exactly like a cron that had stopped being
scheduled.

Every candidate now leaves the run either sent or suppressed with a reason,
and the row carries `people` so the counts can be checked against each other:
`candidates` counts rows and one person can hold several.
A user whose every nudge had its dedupe key claimed before the run started
was filtered out before the grouping, so they reached no decision and landed
in no count except a row level tally. On an hourly cron that is most of the
people the job looks at. They are grouped like everybody else now, with an
empty queue and the reason that fits.

The heartbeat also moves into a `finally`. The selectors, the claimed key
lookup and the cadence read all happen before any per user handling can catch
anything, so a database blip in one of them still ended the hour with no row
at all, and a missing hour is documented as a dead cron.

The selectors move to their own module. The service was over its line budget
and the two halves answer different questions: who is due, and who gets it.
Every name callers already imported is re-exported, so no call site moves.
The cadence is now decided before the queue as well as before the clock. A
claimed key is a NotificationLog row, and that row is also what puts the
person inside the weekly floor, so reporting the key first would have put
already_sent on most of the base and buried every reason anybody can act on.
It is left for the narrow case it names: due, inside their own hour, and
nothing left to say.

The suppression event also goes back to naming the nudge that was actually
queued. Reporting the claimed kind for somebody who still had a second nudge
waiting would have changed what an existing property means.

A run that throws before it finishes deciding now says so on its heartbeat.
Every count on a failed run is whatever it had reached, which for a failure
in the selectors is zero across the board, and that is the row a quiet
evening produces too.
Every heartbeat sent before the flag existed carries no property for it, so
the missing case is the common one on the day this deploys and the comparison
has to read absent as false.
@GSTJ
GSTJ merged commit bad601b into main Sep 7, 2026
16 checks passed
@GSTJ
GSTJ deleted the fix/reengagement-accounting branch September 7, 2026 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant