fix(collect): single daily firing with full-backlog catch-up and traffic self-heal

[mike-wendt]

Summary

Closes #14. Closes #12 (superseded -- see comment on that issue).

collect.yml's retry-window design (#11) had a real bug: it branched on github.event.schedule (which cron pattern matched) instead of wall-clock time, so a delayed firing wrote data under the wrong day -- orphaning 2026-09-02 and mislabeling 2026-09-03 with ~55 minutes of partial data. Working through the fix surfaced a deeper design problem and two explicit data-durability goals for this repo (never lose data; keep each day's snapshot self-contained enough to build on), which this PR is designed around rather than just patching the one bug.

• Single daily firing (0030 UTC), no retry window. collect.yml never collects the current, still-in-progress day -- only ever finalizes fully-elapsed days.
• Full-backlog catch-up, not just the oldest missing day. The Gate step scans the last 7 days for every currently-missing snapshot and collects all of them in one firing. A design that only recovers the single oldest day per firing can never actually close a backlog once behind, since a new day becomes due every day at the same rate as recovery -- this was caught during review before implementation.
• Visible telemetry, restoring what the old design had: ::warning:: when catching up more than one day at once, ::error:: naming any day about to age out of the 7-day window unrecovered (pointing at BOOTSTRAP.md's manual recovery path).
• scripts/refresh_traffic.py (new) patches already-committed snapshot files' traffic fields in place, keeping them in sync with GitHub's rolling 14-day breakdown -- which can lag or later revise a day's numbers -- runs on every firing via if: always() so it's never silently skipped by an unrelated upstream failure. This keeps each snapshot the true, self-contained record for its day instead of requiring a separate file to reconstruct the real value. backfill_traffic.py goes back to being the one-time pre-launch historical seed only, not workflow-invoked.
• Second independent bug fixed in build_site.py: latest_traffic checked views is not None instead of as_of_date is not None, so it always resolved to the newest snapshot regardless of whether that snapshot's traffic was actually valid (views defaults to 0, never None).
• Site disclaimer added: traffic can lag ~1 day, a missing/zero point isn't a real zero.
• Known, documented (not silently implied) limitation: on a multi-day catch-up, repo/labels fields reflect collection time, not each day's real state -- only activity/traffic are genuinely accurate for a backfilled day. This is the same point-in-time limitation the project already had for a single missed day; multi-day catch-up just makes it more visible when it happens.

Review process

This design went through two independent adversarial agent reviews before implementation (one on resilience/correctness, one on alignment with stated goals and scope). Findings and how each was addressed:

1. (Critical) Gate originally only advanced by one day per firing -- a backlog could never close under continued normal operation. Fixed: collects every missing day in the lookback per firing.
2. (Critical) The obvious fix to feat: scaffold sirius-stats collection and site pipeline #1 (loop the fetch over multiple days) would silently stamp today's repo/labels values onto every backfilled day -- addressed via explicit ::warning:: + docs, not silently implied full recovery.
3. (High) 7-day lookback silently dropped data past the boundary with zero signal. Fixed: ::error:: annotation for the day-8 boundary case.
4. (High) Refresh traffic/Commit steps would be silently skipped by GitHub Actions' default if: success() whenever Fetch metrics fails. Fixed: both get if: always().
5. (High) An earlier draft was internally inconsistent about whether there's ever a second commit in one day. Resolved: one Commit step, always last, always covers everything from that firing.
6. (Medium) workflow_dispatch would lose the ability to force a same-day/specific check once caught up. Fixed: added an optional date input as an explicit escape hatch.
7. (Medium) refresh_traffic.py initially only patched null traffic, missing GitHub's later revisions to already-published numbers. Fixed: refreshes any date still within GitHub's current 14-day window, not just previously-null ones.

Test plan

• uv run scripts/build_site.py exits cleanly against current on-disk snapshots
• Gate script logic verified via a Python simulation of the exact algorithm (multi-day catch-up, day-8 boundary detection)
• Bash syntax check of the Gate step (bash -n)
• python3 -m py_compile on all modified/new scripts
• Disclaimer confirmed rendering in the built site/index.html
• Confirm Verify pipeline / verify passes on this PR
• After merge, confirm the next scheduled firing correctly collects 2026-09-03 and any other missing days, and that refresh_traffic.py picks up 2026-09-02's traffic once GitHub publishes it

🤖 Generated with Claude Code

[mike-wendt]

Cursor review

PR Review: #15 — fix(collect): single daily firing with full-backlog catch-up and traffic self-heal

Reviewer: Cursor Cloud Agent
Branch: fix-collect-target-date-logic → main
Verdict: Approve with minor nits — solid fix for a real production incident; design is well-reasoned and documented. Safe to merge after addressing optional follow-ups below.

---

Summary

This PR replaces the hourly retry-window collector (#11) with a simpler model:

1. One daily cron at 0030 UTC, collecting only fully-elapsed days.
2. 7-day catch-up scan that collects all missing days per firing (not just the oldest).
3. refresh_traffic.py to patch traffic fields in existing snapshots on every run.
4. build_site.py bugfix — use as_of_date is not None instead of views is not None as the traffic validity signal.
5. Removes the bad 2026-09-03.json partial snapshot from the scheduler-delay incident.

CI verify passes on head 72b29e6. Local python3 scripts/build_site.py succeeds against current snapshots.

---

What works well

Root-cause fix is correct

The old gate branched on github.event.schedule (which cron pattern fired), not wall-clock semantics. A run delayed past midnight targeted “yesterday” while still conceptually collecting the wrong day — exactly the 2026-09-03 orphan/mislabel incident. Moving to “scan last 7 days for missing files” removes that entire class of bugs.

Full-backlog catch-up is the right call

Collecting only the oldest missing day per firing cannot close a backlog under steady daily load (1 recovered/day, 1 new/day). The multi-day loop in the Gate step fixes that. The ::warning:: when target_count > 1 is good operational telemetry.

Traffic self-heal is well integrated

• Refresh traffic and Commit use if: always() so traffic patching is not silently skipped when Fetch metrics fails.
• refresh_traffic.py re-checks all dates still in GitHub’s 14-day window (not just null traffic), handling both lag and later revisions.
• build_site.py fix correctly treats views as 0 (not None) when as_of_date is null — this was a real bug that made latest_traffic always pick the newest snapshot.

Documentation is thorough

README.md, BOOTSTRAP.md, DATA.md, and CONTRIBUTING.md are updated consistently. The explicit limitation that repo/labels on backfilled days reflect collection time (not that day’s state) is documented rather than hidden.

Data cleanup

Deleting data/snapshots/2026-09-03.json (collected at 2026-09-03T00:55:50Z with partial-day activity) is the right remediation. 2026-09-02.json remains and is a good live test case for refresh_traffic.py (as_of_date: null today).

---

Issues & suggestions

1. Minor — stale docstring in backfill_traffic.py (non-blocking)

The module doc was updated to say “one-time historical seed,” but lines 14–15 still say:

Safe to rerun any time (e.g. to catch up after a missed collect.yml run)

That’s misleading post-launch; missed-day recovery is now fetch_metrics.py --date + refresh_traffic.py, not backfill_traffic.py.

Suggestion: Reword to something like “Safe to rerun when extending pre-launch history” or drop the collect.yml catch-up reference.

---

2. Minor — verify.yml doesn’t exercise refresh_traffic.py (non-blocking)

verify runs fetch_metrics.py + build_site.py but not the new script. A broken refresh_traffic.py would only surface in production collect.yml.

Suggestion (optional): Add a verify step:

- name: Refresh traffic (not committed)
  env:
    SIRIUS_TRAFFIC_TOKEN: ${{ secrets.SIRIUS_TRAFFIC_TOKEN }}
  run: uv run scripts/refresh_traffic.py

Even without a token it should exit cleanly (same pattern as fetch).

---

3. Minor — refresh_traffic.py always rewrites files (non-blocking)

Every date in the 14-day API window with a matching snapshot gets write_text() even when values are unchanged. In practice JSON output should be stable, so git diff --cached --quiet should avoid empty commits — but worth watching on first few production runs.

Suggestion (optional): Compare before write, or only write when as_of_date/counts actually change. Not required for merge.

---

4. Minor — workflow_dispatch date input has no validation (non-blocking)

A typo or future date goes straight to fetch_metrics.py --date. Failure mode is a failed Actions run, not data corruption. Acceptable for an admin escape hatch.

Suggestion (optional): Add a quick date -d / regex check in the Gate step with a clear error message.

---

5. Design tradeoff — single cron vs retry window (acknowledged)

Replacing 3 hourly firings (#11) with one 0030 UTC cron trades scheduler resilience for correctness/simplicity. The 7-day catch-up mitigates a single dropped run, but two+ consecutive dropped runs still need the ::error:: day-8 path + manual recovery.

This is a reasonable tradeoff given the incident, and the PR documents it. No change requested — just flag for the author that #12 is intentionally superseded.

---

6. Nit — catch-up fetch order is newest-first

Gate builds targets as yesterday → 7 days ago. fetch_metrics.py runs in that order. Correctness is unaffected (activity/traffic are per-day), but oldest-first might be slightly nicer for logs and commit messages.

---

Code-specific notes

Area	Assessment
.github/workflows/collect.yml	Gate logic is clear; if: always() on Refresh/Commit is correct; commit message derived from staged files is an improvement
scripts/refresh_traffic.py	Small, focused; mirrors fetch_metrics API patterns; skips gracefully without token
scripts/build_site.py	Critical bugfix; comments explain the as_of_date vs views distinction well
scripts/fetch_metrics.py	Doc updates align with new collect behavior
templates/index.html	Disclaimer (Delayed ~1 day; gaps ≠ zero) is appropriate

---

Test coverage gaps (for author awareness)

PR test plan is manual/simulation-based (no committed unit tests). That’s consistent with the repo (no test suite today). If the author wants hardening later:

• Gate bash logic as a small fixture-based test or documented simulation script in-repo
• refresh_traffic.py with mocked API responses
• Regression test for build_site.py as_of_date is None → skip behavior

Not blockers for this PR.

---

Post-merge checklist (from PR test plan)

• Next scheduled 0030 UTC run collects 2026-09-03 (and any other gaps)
• refresh_traffic.py patches 2026-09-02 once GitHub publishes that day’s traffic
• Confirm no spurious empty commits when traffic values are unchanged
• Watch for ::warning:: / ::error:: annotations in Actions if backlog or day-8 boundary occurs

---

Recommended action

Merge after optional docstring fix in backfill_traffic.py. Everything else can be follow-up if desired.

The PR correctly addresses #14, supersedes #12’s retry-window approach with a better model, and the build_site.py fix is independently valuable.

[mike-wendt]

Addressed in dac2418:

• feat: scaffold sirius-stats collection and site pipeline #1 (stale docstring) — fixed, backfill_traffic.py's "safe to rerun" line no longer references collect.yml catch-up.
• feat: scaffold sirius-stats collection and site pipeline #2 (verify.yml coverage) — added, refresh_traffic.py now runs in verify.yml (exits cleanly without a token, same pattern as Fetch metrics).
• docs: add CONTRIBUTING and CLAUDE files to reduce README #6 (catch-up order) — flipped to oldest-first (7 days ago -> yesterday) for more readable logs/commits.

Left as-is, agreeing with your own "non-blocking"/"acceptable" calls:

• Surface release download counts once sirius cuts a release #3 (always rewrites files) — correct that this happens, but git diff --cached --quiet in the Commit step already prevents an empty commit when nothing actually changed, so there's no real bug here, just an extra write. Not worth the added complexity of a pre-write comparison.
• Add retry/backoff for transient GitHub API failures in fetch_metrics.py #4 (no date input validation) — agreed, acceptable for an admin-only escape hatch; failure mode is a failed Actions run, not data corruption.
• Track issue/PR label distribution and priority flow over time #5 (single cron vs retry window tradeoff) — correctly identified and already the intentional design (see PR description); chore: calibrate collect.yml retry window/cadence once we have real delay data #12 is closed as superseded for this reason.

Test coverage gaps noted -- no unit test suite exists in this repo today, consistent with everything else here; can revisit if it becomes a real pain point.

[mike-wendt]

Reviewed changes and they match the plan