Skip to content

Count no-op ticks so due == sent + no_op in worker logs #101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AndrewAct merged 1 commit into from
Jul 24, 2026
+12 −16
Merged

Count no-op ticks so due == sent + no_op in worker logs #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions backend/TODO.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1 @@
# Backend TODO

## Worker: add a no-op counter to TickResult

`apps/workers/horoscope_email.py` — `TickResult` currently tracks `due`, `sent`, `skipped`,
but `_claim_and_send()` has several "due but nothing to do" branches (already resolved
today, pending-not-yet-stale, failed-max-attempts, claim race lost) that fall through to
`return` at line ~141-144 without incrementing any counter. Result: `due != sent + skipped`,
which reads as unexplained missing sends in the `Tick complete` log even though it's benign.

Fix: add a counter (e.g. `no_op`) to `TickResult`, increment it right before that `return`,
and include it in the `Tick complete` log line so `due == sent + no_op` (skipped stays a
separate, unrelated bucket — see the `_is_due()` vs `_claim_and_send()` distinction).
13 changes: 10 additions & 3 deletions backend/apps/workers/horoscope_email.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class TickResult:
due: int = 0
sent: int = 0
skipped: int = 0
no_op: int = 0


@dataclass
Expand Down Expand Up @@ -139,8 +140,10 @@ async def _claim_and_send(
logger.debug("sub=%s date=%s already resolved (status=%s)", sub_id, local_date, status)

if delivery_id is None:
# Retries exhausted, nothing stale yet, or claimed by another worker between our
# read and write — all fine, nothing to do.
# Retries exhausted, nothing stale yet, already resolved, or claimed by another
# worker between our read and write — all fine, nothing to do. Counted so
# due == sent + no_op (skipped is a disjoint bucket from _is_due, see above).
result.no_op += 1
return

await deps.delivery_service.send_daily_horoscope(subscription, delivery_id, local_date)
Expand Down Expand Up @@ -170,7 +173,11 @@ async def _run_tick_safely(deps: WorkerDeps) -> None:
return
if result.due:
logger.info(
"Tick complete: due=%s sent=%s skipped=%s", result.due, result.sent, result.skipped
"Tick complete: due=%s sent=%s no_op=%s skipped=%s",
result.due,
result.sent,
result.no_op,
result.skipped,
)


Expand Down
3 changes: 2 additions & 1 deletion backend/tests/services/test_worker_horoscope_email.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ async def test_tick_skips_when_already_resolved_today():

result = await tick(deps)

assert result == TickResult(due=1, sent=0, skipped=0)
assert result == TickResult(due=1, sent=0, skipped=0, no_op=1)
deps.delivery_service.send_daily_horoscope.assert_not_awaited()
deps.repository.claim_delivery.assert_not_awaited()
deps.repository.reclaim_failed_delivery.assert_not_awaited()
Expand Down Expand Up @@ -132,6 +132,7 @@ async def test_tick_does_not_reclaim_when_nothing_is_stale():
result = await tick(deps)

assert result.sent == 0
assert result.no_op == 1
deps.delivery_service.send_daily_horoscope.assert_not_awaited()


Expand Down