fix(api): account for every reengagement candidate - #296
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
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.dead_tokenfor a token Expo will not take, and a newalready_sentfor a queue with nothing left to claim, whether the claim came from an earlier run or from one racing this one.already_senton 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.heldcount rather than a reason that would read as one. It reads zero in a healthy run. At the push ceiling a non zeroheldis a backlog waiting for the next hour rather than anybody lost.people, the distinct users behind the candidate rows.candidatescounts rows and one person can hold several, sosentandsuppressedwere never meant to add up to it. They add up topeople, together withheld, and the tests assert that on every path.heldon 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
Screenshots
No user facing surface changes. The visible output is the new readout table: