perf: optimize backend and real-time hot paths

[AussieScorcher]

Summary

Optimizes Core's backend and real-time hot paths while preserving API, WebSocket, storage, and database behavior. The work was benchmarked before implementation, compared against multiple candidate implementations, and validated with committed behavior/regression suites.

Changes Made

• Reduced D1 round trips with batched reads/writes, RETURNING mutations, narrower queries, contribution-action context reuse, and workload-aligned indexes.
• Streamlined real-time handling with typed Durable Object RPC, maintained socket indexes, shared serialization, ordered per-socket queues, coalesced status checks, and batched analytics.
• Improved endpoint, cache, and storage paths with request coalescing, cheaper cache keys/hits, nested R2 routing, native byte ranges, streaming uploads, and less middleware/request reconstruction.
• Accelerated XML sanitization, geometry traversal, polygon generation, and vatSys profile generation after benchmarking multiple candidate algorithms.
• Updated the Cloudflare Worker runtime configuration/tooling and added reusable performance plus behavior suites under benchmarks/.

Additional Information

Root causes addressed

The audit found repeated D1 reads, sequential write loops, broad payload queries, per-message socket scans and allocations, duplicate cache/network work, avoidable request reconstruction, full-object R2 reads for byte ranges, and compute pipelines that rescanned or rebuilt data unnecessarily.

Representative benchmark results

• XML sanitizer: 14.95x median improvement on the benchmark fixture with exact output equality.
• Real-time participant snapshot: 105.99 ms -> 6.29 ms by maintaining indexes instead of scanning every socket.
• Controller-targeted broadcast: 40.58 ms -> 4.23 ms using the controller index.
• Cache API calls: 2,000 -> 1,001 for unique misses and 200 -> 101 for a cold burst.
• Contribution actions after route authorization: 2-4 D1 calls -> 1, depending on action.
• Latest-map D1 payload: 88,325 -> 54 bytes in the synthetic fixture by selecting the required descriptor only.
• R2 one-megabyte range of a 100 MB object: 100 MB -> 1 MB read.

These are local microbenchmarks intended to compare implementations, not production latency claims.

Validation

• bunx tsc --noEmit
• bunx eslint .
• bun run test:backend
• bun run test:realtime
• bun run benchmark
• bun run openapi
• bun run build
• bunx wrangler types --check
• Local Wrangler smoke tests and Miniflare R2 nested-path/range probes

No production deployment or remote database migration was performed.

Author Information

Discord Username:
VATSIM CID:

---

Checklist:

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

[greptile-apps]

Greptile Summary

A broad performance pass across the backend and real-time layers, touching 38 files. The changes are well-scoped: each optimisation has a clear before/after and the benchmark suite provides concrete measurements.

• Real-time (connection.ts): DO upgraded to the v2 DurableObject<Env> base class with a typed getState RPC; controller/pilot presence now maintained as in-memory indexes instead of full socket-map scans; status checks coalesced per controller; serialized heartbeat and batched analytics cut per-message allocations.
• Database (auth, contributions, downloads, vatsys-profile-generator, schema.sql): Sequential reads replaced with executeReadBatch/executeBatch reducing D1 round trips; RETURNING mutations eliminate follow-up reads; getContributionActionContext collapses 2–4 calls into one; schema replaces single-column indexes with workload-aligned composites and adds idempotent DROP INDEX stmts.
• Storage/cache/XML (storage.ts, cache.ts, xml-sanitizer.ts): R2 reads now honour byte-range headers natively; namespace version hints include a freshness TTL and module-level dedup; control-char strip converted from an array iteration to a pre-compiled regex.

Confidence Score: 3/5

Safe to merge after addressing the null-guard gap in fetchLoginState; all other changes are behaviorally correct.

The fetchLoginState rewrite in auth.ts skips a null check before destructuring result.results[0] — if D1 returns 0 rows despite the FROM (SELECT 1) anchor, the OAuth callback throws an unhandled TypeError for every affected login attempt. The rest of the PR is solid: role-hierarchy simplifications are mathematically correct, UPSERT RETURNING patterns produce the expected behavior in SQLite/D1, the socket-index cleanup is reliable in the CF DO event model, and the removed dead-code branch for VATSIM Radar was unreachable.

src/services/auth.ts (null-guard before destructuring), src/network/connection.ts (forceOffline response shape change), src/services/contributions.ts (submitted_xml over-fetch).

Important Files Changed

Filename	Overview
src/network/connection.ts	Major real-time hot-path refactor: typed DO RPC replaces internal HTTP, controller/pilot socket indexes replace full-socket scans, coalesced status checks, batched analytics, serialized heartbeat. One behavioral change noted — forceOffline=true now omits controllers/pilots from the response.
src/services/auth.ts	Login path collapsed from two parallel D1 reads into one combined anchor query; new getConnectionPrincipalByApiKey merges user lookup and ban check. Missing null guard on result.results[0] before destructuring could crash the OAuth callback.
src/services/contributions.ts	getContributionActionContext collapses 2–4 D1 reads into one; processDecision approval uses executeBatch for atomic outdate+approve. Shared context query always selects submitted_xml even in paths that don't need it.
src/services/downloads.ts	Collapses recordDownload into two executeBatch calls; ON CONFLICT DO UPDATE ... WHERE pattern correctly uses SQLite RETURNING behavior. Return type now includes versions directly, eliminating the follow-up getStats call.
src/services/cache.ts	Namespace version hints carry a checkedAt timestamp expiring after 5s; module-level dedup map coalesces concurrent version fetches. Added getResponse for streaming cache hits.
src/services/storage.ts	R2 get now passes incoming Range headers directly so only the requested byte range is transferred. 206 Partial Content and Content-Range headers built correctly. etag corrected to httpEtag.
schema.sql	Replaces single-column indexes with workload-aligned composite indexes; adds DROP INDEX IF EXISTS stmts and PRAGMA optimize for migration idempotency.
src/index.ts	BARS DO now extends DurableObject with typed getState RPC; /connect forwards raw request; multiple regex and inline-object allocations hoisted to module-level constants. Removed VATSIM Radar branch was unreachable dead code.
src/services/vatsys-profile-generator.ts	Four sequential D1 reads collapsed into one executeReadBatch; airportBounds propagated as a parameter. The bounds query runs for all airports including legacy format airports that never consume the value.
src/services/xml-sanitizer.ts	Control-character strip replaced from Array.from + filter + join with a pre-compiled module-level regex — same correctness, significantly faster per benchmark.
src/services/contact.ts	INSERT and UPDATE converted to RETURNING patterns eliminating follow-up reads; updateStatus logic merged into a single CASE expression; hasRecentSubmissionFromIp changed from COUNT(*) to EXISTS.
src/services/divisions.ts	DISTINCT subquery for has_objects replaced with correlated EXISTS; addMember pre-check removed in favor of catching UNIQUE constraint (409 preserved via string-match in catch block).

Comments Outside Diff (1)

1. src/services/vatsys-profile-generator.ts, line 1294-1333 (link)

   airportBounds is fetched for every airport regardless of format

   The boundsResult (airport bounding box) is always fetched in getGenerationInputs, but it is only consumed by the intas code path via validateAirportBounds. For airports using the legacy format the query is executed and the result silently discarded. Because all four queries are batched in a single round-trip the extra statement costs very little, but the D1 work (index lookup + row transfer) still happens. Omitting the bounds query when the format can be determined ahead of time — or keeping it but noting the intent — would make the data flow explicit.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/services/vatsys-profile-generator.ts
   Line: 1294-1333
   
   Comment:
   **`airportBounds` is fetched for every airport regardless of format**
   
   The `boundsResult` (airport bounding box) is always fetched in `getGenerationInputs`, but it is only consumed by the `intas` code path via `validateAirportBounds`. For airports using the `legacy` format the query is executed and the result silently discarded. Because all four queries are batched in a single round-trip the extra statement costs very little, but the D1 work (index lookup + row transfer) still happens. Omitting the bounds query when the format can be determined ahead of time — or keeping it but noting the intent — would make the data flow explicit.
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
src/services/auth.ts:522-524
**Unchecked destructuring on `result.results[0]`**

`FROM (SELECT 1) anchor` is intended to guarantee at least one result row, but if D1 returns an empty array for any reason (D1 runtime quirk, statement-level error swallowed by the session, etc.), `row` is `undefined` and the destructuring on line 523 throws a `TypeError` that propagates unhandled through `handleCallback`, crashing the VATSIM OAuth flow for the affected user. A quick guard — or replacing the `FROM (SELECT 1)` anchor with `executeReadBatch` for the two independent reads — would eliminate the risk entirely.

### Issue 2 of 4
src/network/connection.ts:1733-1746
**`forceOffline=true` silently drops `controllers` and `pilots` from the response**

Previously, the `?offline=true` query parameter was passed into the DO fetch URL but never actually read by the handler, so it was a no-op — the response always included `controllers` and `pilots`. Now `forceOffline=true` returns `{ airport, objects, offline: true }` with those two fields absent. Any client that ever sent `?offline=true` expecting a shape consistent with a normal state response will now receive a structurally different object. The `ConnectionStateSnapshot` type marks them as optional, which is architecturally correct, but the silent behavioral change is undocumented and not reflected in the OpenAPI spec.

### Issue 3 of 4
src/services/contributions.ts:135-148
**`getContributionActionContext` always selects `submitted_xml` even for delete/permission checks**

`c.submitted_xml` can be up to 5 MB per contribution. `deleteContribution` and the permission gate in `regenerateContribution` only need existence and ownership fields — neither uses the XML. Omitting `submitted_xml` from this shared context query and fetching it lazily inside `regenerateContribution` (where it's actually needed) would avoid transferring up to 5 MB of data on every delete or permission check.

```suggestion
		const result = await this.dbSession.executeLatest<Omit<Contribution, 'submittedXml'> & { actorIsProductManager: number }>(
			`SELECT
				c.id, c.user_id AS userId, submitter.display_name AS userDisplayName,
				c.airport_icao AS airportIcao, c.package_name AS packageName,
				c.notes, c.simulator,
				c.submission_date AS submissionDate, c.status,
				c.rejection_reason AS rejectionReason, c.decision_date AS decisionDate,
				CASE WHEN staff.role IN ('LEAD_DEVELOPER', 'PRODUCT_MANAGER') THEN 1 ELSE 0 END AS actorIsProductManager
			 FROM users actor
			 LEFT JOIN staff ON staff.user_id = actor.id
			 LEFT JOIN contributions c ON c.id = ?
			 LEFT JOIN users submitter ON submitter.vatsim_id = c.user_id
			 WHERE actor.vatsim_id = ?
			 LIMIT 1`,
```

### Issue 4 of 4
src/services/vatsys-profile-generator.ts:1294-1333
**`airportBounds` is fetched for every airport regardless of format**

The `boundsResult` (airport bounding box) is always fetched in `getGenerationInputs`, but it is only consumed by the `intas` code path via `validateAirportBounds`. For airports using the `legacy` format the query is executed and the result silently discarded. Because all four queries are batched in a single round-trip the extra statement costs very little, but the D1 work (index lookup + row transfer) still happens. Omitting the bounds query when the format can be determined ahead of time — or keeping it but noting the intent — would make the data flow explicit.

_{Reviews (1): Last reviewed commit: "test(perf): add benchmark regression sui..." | Re-trigger Greptile}