Skip to content

fix(ci): freshen all e2e-fixture observation timestamps (unblocks #1630 reach e2e)#1747

Open
efiten wants to merge 1 commit into
Kpa-clawbot:masterfrom
efiten:fix/e2e-fixture-observations-freshen
Open

fix(ci): freshen all e2e-fixture observation timestamps (unblocks #1630 reach e2e)#1747
efiten wants to merge 1 commit into
Kpa-clawbot:masterfrom
efiten:fix/e2e-fixture-observations-freshen

Conversation

@efiten

@efiten efiten commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Problem

test-issue-1630-reach-mobile-e2e.js has been failing on master since ~2026-06-16, in its precondition pickRepeaterWithReach ("no repeater with reach links found in fixture") — not in any of its actual assertions. The same SHA passed on 06-15 and failed on 06-16 with no code change, i.e. it tracks the wall clock, not the tree.

Root cause

tools/freshen-fixture.sh shifts nodes, transmissions, observers and neighbor_edges timestamps to ~now, but for observations.timestamp it only rewrote rows where timestamp = 0 OR timestamp IS NULL. Real-timestamped observations stayed frozen at fixture-capture time.

Per-node reach (/api/nodes/{pk}/reach?days=NscanReachRows, cmd/server/node_reach.go) windows on observations.timestamp >= sinceEpoch. ~30 days after the fixture was captured, the newest observation aged out of the 30-day window, so reach returned no links and the test could find no repeater with reach.

Fix

Shift all non-zero observations.timestamp forward by the same offset (preserving relative order), mirroring the other tables in the script. The offset subquery is uncorrelated, so SQLite evaluates MAX once on the pre-update state (same idiom the existing blocks rely on).

Verification

Ran the updated script against test-fixtures/e2e-fixture.db:

BEFORE: newest observation 31 days old  → outside the 30-day reach window
AFTER : newest observation  0 days old, all 500 observations within 30 days

CI Playwright on this PR is the end-to-end confirmation. Scope is the CI fixture helper only — no application code, schema, or runtime behaviour changes.

🤖 Generated with Claude Code

…zeroed ones

freshen-fixture.sh shifted nodes/transmissions/observers/neighbor_edges
timestamps to ~now, but for observations.timestamp it only rewrote rows where
timestamp = 0 OR IS NULL. Real-timestamped observations stayed frozen at capture
time, so once the committed fixture aged past a query's window they fell out of
it.

Per-node reach (/api/nodes/{pk}/reach?days=N, node_reach.go scanReachRows) filters
`observations.timestamp >= sinceEpoch`. ~30 days after the fixture was captured the
newest observation (verified: 31 days old) dropped out of the 30-day window, so
reach returned no links and test-issue-1630-reach-mobile-e2e.js failed in
pickRepeaterWithReach ("no repeater with reach links found") — a CI failure
unrelated to any code change, tracking only the wall clock.

Shift all non-zero observation timestamps forward by the same offset (preserving
order), mirroring the other tables; the uncorrelated MAX subquery is evaluated
once on the pre-update state. Verified on test-fixtures/e2e-fixture.db: newest
observation goes 31d → 0d old, all 500 land within the 30-day window.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Kpa-clawbot

Copy link
Copy Markdown
Owner

🤖 Hourly PR watch v2 — #1747

Verdict: Merge-ready (with caveat).

Surface: Tooling — tools/freshen-fixture.sh only. 12 additions, 1 deletion.

Parallel review summary:

  • carmack/munger (backend/SQL): Correct. Uncorrelated subquery evaluates MAX once on the pre-update state, then row-shift applies uniformly to all timestamp > 0. Preserves relative ordering. The subsequent timestamp = 0 OR IS NULL branch is unaffected by the prior UPDATE (those rows don't match WHERE timestamp > 0). No overflow risk on INTEGER seconds.
  • kent-beck (TDD): No unit test for a shell tool — falls under the tooling-change exemption per AGENTS.md. The integration validation is the CI signal itself: this PR exists to unblock the reach: missing narrow-viewport CSS — table h-scrolls + map dominates on mobile #1630 reach-mobile e2e, and Playwright is now SUCCESS on this branch. That's the assertion.
  • dijkstra (correctness): Comment block accurately documents intent + invariants. Good failure mode story (why observations age out of sinceEpoch window).

3-axis check:

  • mergeable: MERGEABLE
  • mergeStateStatus: CLEAN
  • CI: ✅ Go Build & Test, 🎭 Playwright E2E — all SUCCESS
  • Polish: 0 BLOCKER, 0 MAJOR

Auto-merge skipped because strict gate requires ≥1 test commit and this is a pure tooling fix (no _test.go/__tests__/ changes). Operator can squash-merge directly — the change is low-risk, scoped, and the e2e signal validates it end-to-end.

Suggested: gh pr merge 1747 --squash

@Kpa-clawbot

Copy link
Copy Markdown
Owner

🤖 Hourly PR-watch review

Verdict: APPROVE (operator merge recommended)

What this fixes

Reach-mobile e2e (#1630) precondition has been failing on master since ~06-16: pickRepeaterWithReach returns nothing because tools/freshen-fixture.sh shifts every other table forward but only rewrote observations.timestamp rows where the value was 0/NULL. Real-timestamped observations stayed pinned to fixture-capture time and aged out of the 30d reach window.

Diff review

  • One-table fix, mirrors the existing transmissions/nodes/observers/neighbor_edges shift idiom.
  • Uses uncorrelated MAX(timestamp) subquery → SQLite evaluates once on pre-update state ✅ (same trick the surrounding blocks already rely on; correctness preserved).
  • WHERE timestamp > 0 guards against shifting the 0/sentinel rows that the next statement then rewrites from transmissions.first_seen — order of statements is correct.
  • No risk of double-shift: the second UPDATE runs on a fresh SET value computed from transmissions, not stacked on the first.

Cross-PR impact

This PR is the unblocker for ALL currently-open PRs whose Playwright check is failing (#1746, #1737, #1736, #1734, #1728). Once merged, rebasing those should flip them to CLEAN.

Why not auto-merge

  • Hourly gate requires ≥1 added test file; this is a shell-script fix to test infra, so heuristic doesn't apply but I'm leaving the merge button to the operator.
  • Direct evidence the fix works: this PR's own Playwright check is green (and was failing on master immediately prior).

Findings: 0 BLOCKER / 0 MAJOR / 0 MINOR

Reviewed by: carmack + kent-beck personas (backend + test-infra surface).

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.

2 participants