fix(scheduler): require a strictly later period, not merely a different one

[sturlese]

Found by an autonomous bughunt iteration.

Bug

scheduler.is_due compares calendar-period keys with !=:

return _period_key(cadence, last_started_at) != _period_key(cadence, now)

Inequality is true when now is in a later period than the last run — and equally true when it is in an earlier one. So a run stamped in a future period makes the workflow due on every tick:

>>> is_due("daily", NOW + timedelta(days=1), NOW)
True          # expected False — tomorrow is not an earlier period than today

The line directly above the comparison already states the intended rule:

"due only when now falls in a later calendar period than the last run."

Why it is unbounded, not a one-off

last_run_started_at returns the newest row by started_at:

runs = store.runs(workflow_id=workflow_id)
return runs[-1].started_at if runs else None

A future-dated run therefore stays "the last run" no matter how many runs tick appends after it. Every subsequent tick sees a different period key and fires again — until real time catches up with that stamp.

That is exactly what the module docstring says cannot happen:

"a scheduler that calls tick 300 times in an hour runs a daily digest exactly once that day … The period, not success, is what gets consumed — that is what makes the demo's week-9 runaway impossible."

End-to-end, with a run stamped tomorrow already in the store, three consecutive flightdeck tick invocations each print:

✓ daily-digest: ran (daily) · mock-trainer-us · €0.00 · run 05032aee3108

A cron calling tick every minute would run the workflow every minute, spending real budget against a real provider, for as long as the skew lasts.

A future-dated run is not exotic: clock skew between the scheduler host and the store, an imported store, or a backfill all produce one.

Fix

return _period_key(cadence, now) > _period_key(cadence, last_started_at)

All three cadence keys are ordered tuples — (y, m, d), (isoyear, isoweek), (y, m) — so > is well defined. Past and same-period behaviour is unchanged; the eight existing due-logic assertions pass untouched.

Test

tests/test_scheduler.py:

test	asserts
test_a_future_dated_run_does_not_make_the_period_due_forever	all three cadences return False for a future stamp
test_future_dated_run_stays_the_newest_row_so_the_storm_would_be_unbounded	the future row remains last_run_started_at even after a later run is added — the reason the retry has no bound
test_tick_does_not_storm_on_a_future_dated_run	three consecutive tick invocations add zero runs

All three verified to fail without the fix (git stash on the source alone); the third fails with "daily-digest: ran" on every tick.

Validation

All three CI jobs green locally:

• python -m pytest --cov=flightdeck --cov-fail-under=85 → 261 passed, coverage 94.48%
• ruff check src tests → All checks passed!
• flightdeck demo + flightdeck audit verify → ledger verified, 2,434 entries, chain intact

🤖 Generated with Claude Code

https://claude.ai/code/session_01XjLWb6igee2tVTG7wg93Fx

[sturlese]

Gate verdict — PASS after one amendment round

The review confirmed the arithmetic was airtight and then found something better: the fix as first written traded an unbounded storm for unbounded silent starvation.

The finding

With a run stamped ten years ahead — a condition ADR 002 explicitly designs for, since "timestamps may be supplied for imports and the demo seeder" — the > comparison made the workflow due on no tick at all. Measured: 3,465 consecutive days of is_due(...) == False, printing the byte-identical line a healthy skip prints (cli.py:355 uses the same string on both paths), writing no ledger event, exiting 0.

A storm announces itself — 300 ledger entries, 300 provider charges, an alert. Starvation is invisible in the console, the ledger and the exit code until someone notices a digest stopped arriving.

Worse, the first fix put the code in direct contradiction with docs/governance.md:56, which it did not update:

"a daily workflow is due unless some run already started today … Crucially, any run in the period counts."

With only a tomorrow-stamped row, no run started today, so governance says DUE. The > version said NOT DUE.

Root cause — both failures are the same mistake

is_due was asked about the newest run, while the documented rule is stated over the set. A run stamped in another period, past or future, simply is not a run in this one, and collapsing the history to one row cannot express that:

reading of the newest row	future-stamped run
periods differ (!=, main)	fires on every tick — the storm
now's period is later (>, first commit)	fires on no tick — starvation
any run in this period (this commit)	fires once, then the period is spent

Fix

Take the documented rule literally. is_due now receives the start times and asks whether any falls in now's period — still pure, as the module docstring promises. runs_started_this_period supplies them, bounded in SQL to the period's first instant so a long history costs nothing, then keyed so a stamp beyond the period's last instant is excluded.

Both existing behaviours are preserved: ran earlier today → not due; ran yesterday → due.

Measured, end-to-end through the real CLI

300 tick invocations in one simulated day, with a run stamped 10 years ahead already in the store:

	ticks that ran
main (!=)	300 — the storm
first commit (>)	0 — starvation
this commit	1 — correct

Ruled out by the review

• Weekly ISO tuple ordering is clean — every date 1990→2060 enumerated, 3,705 keys, strictly increasing, zero non-monotonic transitions, including every W53 year (2020, 2026, 2032). isocalendar() returns the ISO year precisely so this holds. Daily and monthly likewise.
• Timezone: _to_utc normalizes both operands before keying; a naive/aware mix cannot invert an ordering.
• != was not deliberate belt-and-braces — the module's headline is "the reason a retry storm is impossible by construction", and the storm was reproduced at 300/300 on main. The pre-existing docstring already said "later calendar period", so the code contradicted itself.
• Blast radius contained: is_due had exactly one caller; no metrics, report or demo path depends on it.

CI green. Left open for human review.