Repository navigation
fix(events): one D1 round trip per upload; stop re-reading never-exported rows - #75
Merged
Merged
Conversation
…rted rows - Fold the tombstone and token checks into the ingest batch; its final statement reports ownership and maps to the same 403. - Write desktop diagnostics and identity observations as posthog_state='excluded' (migration 0008 rebuilds analytics_events to widen the CHECK and backfills existing rows), so the export queue holds only rows it can send. - Export events from the five-minute cron only, up to ten batches per run, instead of after every upload. - Log each upload's status, event count and stage durations without identifiers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…est batch Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deploying scientfactory-website with
|
| Latest commit: |
045aae5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://26dd0507.scientfactory-website.pages.dev |
| Branch Preview URL: | https://claude-collector-ingest-2026.scientfactory-website.pages.dev |
Every observation write now also requires the installation's token and no tombstone, not only its stored event. Found in review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
What this changes
The events collector (
workers/events) does less database work per desktop upload. That shortens the time before Scient gets its acknowledgement, which is part of the G6 analytics send timeouts.database.batch. Every write was already guarded by them in SQL. The batch's last statement returns whether this token owns a live, non-deleted installation, and an empty result is the same 403 as before. Deleted installations and wrong tokens still get 403 with nothing written; the existing real-SQL tests cover both.install.account_observed,install.profile_observedand deviceinstall.owner_observedstayedposthog_state='pending'forever, so every export re-read them. In production that is 21,471 rows, all of the pending rows (read-only query, 2026-11-15). They are now written as'excluded'. The export's SQL and JS filters stay as a backstop.waitUntilafter every upload. Each 5-minute cron run exports up to 10 batches of 100, inside the existing lease. PostHog now lags D1 by up to about 5 minutes, instead of seconds. D1 stays the primary store, and the 202 acknowledgement never waited on PostHog.status,events,error_class,read_ms,rate_limit_ms,d1_ms,total_ms. It has no identifiers, tokens, IPs or payloads. The timings exist so a later change can be based on measured server time.analytics-report.mjs) countsexcludedas first-party-only. Reconciliation (analytics-reconciliation.mjs) leavesexcludedout, where it would otherwise throw "Unknown delivery state".D1 migration
0008_analytics_export_excluded.sqlSQLite cannot widen a CHECK constraint in place, so the migration rebuilds
analytics_events:'pending','sent'and'excluded'.'excluded'(the backfill).analytics_no_deleted_event,analytics_daily_event_totals,analytics_daily_active_totals).The triggers are recreated after the copy, so daily totals are not counted twice. I checked the column, index and trigger list against production's live
sqlite_master(read-only). Production holds about 149k rows (DB size 171 MB), so the copy is small.Deploy order (operator step; not done here)
wrangler d1 migrations apply scientfactory-downloads --remote --config workers/events/wrangler.jsonc. The deployed worker (f7abeca) keeps working after it, because it writes the default'pending'.'excluded', the upload returns 500, and clients retry.'pending'. This idempotent query moves them:/healthshows the newworker_version_tag, the Workers logs showAnalytics ingestionlines, andSELECT posthog_state, count(*) FROM analytics_events GROUP BY 1has no never-exportedpendingrows.This PR is independent of the desktop PR ScientFactory/scient-desktop#931. Neither changes the contract (still revision 11), and the two can land and deploy in either order.
Quality Review
Correctness of the merged checks. All writes in the batch were already guarded by the token and the tombstone:
WHERE NOT EXISTS tombstoneand a token-matchWHERE.A wrong-token or deleted request now runs no-op guarded writes instead of stopping early. The response is the same, and no row changes. The tombstone trigger cannot fire, because deletion erases the identity in the same transaction that writes the tombstone. I updated the comment in
functions/_lib/analyticsPeople.tsto say this.Export throughput. At 1,000 events per 5 minutes, the cron exceeds today's ingest by a wide margin (about 25k stable events a week). The loop stops at the first short batch.
Reports. I checked every query that reads
posthog_statein the worker and scripts: export, report and reconciliation.Not changed: retention scans (not part of the decision), the contract revision, and website-event ingestion (
functions/_lib/events.tsstill writes'pending').Tests
bun run check: format, typecheck, events and mirror typecheck, 185 tests passed (2 skipped, as on main), and both builds. Run on Node 24.19 with the live-app env unset.index.test.ts: onebatchwith 3 statements, no.first(),'pending'bound, and no exportwaitUntil.readiness.test.ts, three real-SQL tests:'excluded'and the session event'pending', with no export task.analytics-reconciliation.test.mjs: the query excludes'excluded'.wrangler d1 migrations apply --localinto a scratch directory, seeded before 0008. Rows were backfilled, all indexes and triggers came back, and totals were not recounted. No remote write, migration or deploy was run.Preview
No user-visible website change; the Cloudflare page preview doesn't apply.
Documentation impact: Updated — README.md (event export states, cron-only export, ingestion logging)
🤖 Generated with Claude Code