> ## Documentation Index
> Fetch the complete documentation index at: https://285e39fd5e337e58f16290.sightscreen.app/llms.txt
> Use this file to discover all available pages before exploring further.

# Learnings

> Hard-won lessons from production development — patterns to follow and traps to avoid.

These learnings were extracted from \~40 PRs over the last week of development. Each one cost real debugging time. They are organized by category so you can scan for the area you are working in.

## Distributed systems

<Accordion title="Lock TTL must exceed worst-case tick duration">
  **Rule:** Use in-loop heartbeats to extend lock TTL, not just a single lock-before / unlock-after pattern.

  **Why it matters:** If a tick takes longer than the lock TTL (e.g. a slow APNs batch), the lock expires mid-tick and a second instance picks it up — causing duplicate work or data corruption.

  ```ts theme={null}
  // Bad — lock expires if tick is slow
  await lock.acquire(ttl: 30_000);
  await runTick(); // might take 45s
  await lock.release();

  // Good — heartbeat keeps the lock alive
  await lock.acquire(ttl: 30_000);
  await runTick({ onProgress: () => lock.extend(30_000) });
  await lock.release();
  ```

  *PRs #318, #285, #307*
</Accordion>

<Accordion title="DynamoDB conditional writes for multi-instance safety">
  **Rule:** Always use `attribute_not_exists` (or similar conditions) on writes that must be idempotent across instances.

  **Why it matters:** Without conditional writes, a second instance can silently overwrite the first instance's data. DynamoDB does not error on duplicate puts by default.

  ```ts theme={null}
  // Bad — silent overwrite
  await ddb.put({ TableName, Item });

  // Good — fails if item already exists
  await ddb.put({
    TableName,
    Item,
    ConditionExpression: 'attribute_not_exists(pk)',
  });
  ```

  *PR #318*
</Accordion>

<Accordion title="Promise.allSettled over Promise.all for batch operations">
  **Rule:** Use `Promise.allSettled()` instead of `Promise.all()` when sending to multiple targets (e.g. APNs push tokens).

  **Why it matters:** `Promise.all()` rejects on the first failure, dropping the entire batch. One expired push token should not prevent delivery to every other device.

  ```ts theme={null}
  // Bad — one failure kills the batch
  await Promise.all(tokens.map((t) => sendPush(t, payload)));

  // Good — collect individual results
  const results = await Promise.allSettled(tokens.map((t) => sendPush(t, payload)));
  const failures = results.filter((r) => r.status === 'rejected');
  ```

  *PR #318*
</Accordion>

<Accordion title="Fire-and-forget async handlers swallow errors">
  **Rule:** Every async handler needs an explicit error boundary. Never use `void this.fn()` without a `.catch()`.

  **Why it matters:** Unhandled promise rejections from fire-and-forget calls are invisible — no logs, no alerts, no retries. The operation silently fails.

  ```ts theme={null}
  // Bad — error vanishes
  void this.processUpdate(update);

  // Good — error is captured
  void this.processUpdate(update).catch((err) =>
    this.logger.error('processUpdate failed', { err }),
  );
  ```

  *PR #318*
</Accordion>

## Server-driven UI

<Accordion title="Ship API display fields BEFORE building iOS cards">
  **Rule:** Add all display fields to the API response first. Only then build the iOS card that consumes them.

  **Why it matters:** If the client ships first, developers fill gaps with client-side business logic (formatting, conditional rendering). That logic then needs to be ripped out and replaced when the API catches up, doubling the work.

  *PRs #300, #332*
</Accordion>

<Accordion title="Never re-process data the backend already formats">
  **Rule:** If the backend sends a formatted value, bind it directly. Do not apply additional transformations on the client.

  **Why it matters:** Client-side transforms can break edge cases the backend already handles. For example, applying `prefix(3)` to team codes truncated four-letter codes like `PBKS` to `PBK`.

  ```swift theme={null}
  // Bad — client re-processes
  Text(team.code.prefix(3))

  // Good — backend sends the right value
  Text(team.abbreviation) // "PBKS" from API
  ```

  *PR #333*
</Accordion>

<Accordion title="Bump schema version when MatchItemSchema changes">
  **Rule:** Every change to `MatchItemSchema` must include a schema version bump.

  **Why it matters:** iOS uses a disk cache keyed by schema version. Without a bump, devices running the old version will deserialize stale data and never see the update until the cache expires naturally.

  *PR #328*
</Accordion>

## Environment & config

<Accordion title="!!process.env.FLAG treats 'false' as truthy">
  **Rule:** Always compare boolean environment variables with `=== 'true'`. Never rely on `!!` coercion.

  **Why it matters:** `process.env.FLAG = 'false'` is a non-empty string, so `!!process.env.FLAG` evaluates to `true`. This silently enables features you intended to disable.

  ```ts theme={null}
  // Bad
  const dryRun = !!process.env.DRY_RUN;

  // Good
  const dryRun = process.env.DRY_RUN === 'true';
  ```

  *PR #286*
</Accordion>

<Accordion title="Centralized requireEnv() creates all-or-nothing coupling">
  **Rule:** Validate environment variables at the point of use, not all at startup in a single config file.

  **Why it matters:** A top-level `requireEnv()` for every variable means one missing var (even for an unused feature) blocks the entire server from starting. This makes local development and partial deployments unnecessarily painful.

  *PR #286*
</Accordion>

## iOS / Swift 6

<Accordion title="Swift 6 synthesized Codable conflicts with Sendable">
  **Rule:** Add explicit `nonisolated init(from:)` and `encode(to:)` to structs that need both `Codable` and `Sendable`.

  **Why it matters:** Swift 6 synthesizes Codable conformance as main-actor-isolated on structs, which conflicts with `Sendable` generic constraints. This causes compile errors in concurrent contexts.

  ```swift theme={null}
  struct Match: Codable, Sendable {
    let id: String

    // Required for Swift 6 + Sendable
    nonisolated init(from decoder: Decoder) throws { ... }
    nonisolated func encode(to encoder: Encoder) throws { ... }
  }
  ```

  *PR #316*
</Accordion>

<Accordion title="ISO8601DateFormatter with .withFractionalSeconds returns nil for non-fractional timestamps">
  **Rule:** Use two formatters or a fallback when parsing ISO 8601 dates — one with fractional seconds and one without.

  **Why it matters:** `ISO8601DateFormatter` configured with `.withFractionalSeconds` returns `nil` for timestamps like `2024-01-15T10:30:00Z` (no fractional part). This silently drops valid dates.

  ```swift theme={null}
  // Good — fallback chain
  func parseISO8601(_ string: String) -> Date? {
    fractionalFormatter.date(from: string)
      ?? standardFormatter.date(from: string)
  }
  ```

  *PR #332*
</Accordion>

<Accordion title="Never gate follow-persistence on Live Activity checks">
  **Rule:** Do not guard the follow/unfollow flow behind `areActivitiesEnabled()` or similar LA-specific checks.

  **Why it matters:** The `areActivitiesEnabled` guard blocked the entire follow flow, meaning users who had Live Activities disabled could not follow matches at all. Follow persistence is independent of LA state.

  *PR #338*
</Accordion>

<Accordion title="Store observation Tasks as instance properties on singletons">
  **Rule:** Hold strong references to `Task` objects that observe published properties — assign them to instance-level properties on the owning singleton.

  **Why it matters:** If the `Task` is only stored as a local variable, ARC can deallocate it, silently stopping observation. This manifests as frozen token updates or missed state changes that are extremely hard to diagnose.

  *LA platform knowledge*
</Accordion>

## Privacy & telemetry

<Accordion title="sendDefaultPii: true requires a beforeSend scrubber">
  **Rule:** If you enable `sendDefaultPii` in Sentry, you must also configure a `beforeSend` hook that strips sensitive fields.

  **Why it matters:** Without a scrubber, Sentry events will contain user emails, IP addresses, and auth headers — a compliance and privacy violation.

  *PR #335*
</Accordion>

<Accordion title="Audit every log site when bridging logger to telemetry">
  **Rule:** When connecting your application logger to an external telemetry service, audit every `logger.info/warn/error` call for PII.

  **Why it matters:** Logs that were safe when written to local stdout become a liability when forwarded to a third-party service. Emails, URLs with tokens, and device IDs all end up in external dashboards.

  *PR #325*
</Accordion>

<Accordion title="Sanitize log payloads with an allowlist">
  **Rule:** Use an allowlist of safe keys when forwarding structured log payloads to external services. Never forward the raw payload.

  **Why it matters:** A blocklist approach is fragile — new fields added by developers are forwarded by default. An allowlist ensures only explicitly approved fields leave the system.

  *PR #335*
</Accordion>

## Caching

<Accordion title="Never have two caches for the same data">
  **Rule:** Establish a single source of truth for cached data early. If two code paths produce the same data, they should read from and write to the same cache.

  **Why it matters:** Dual caches inevitably drift, causing inconsistent behavior depending on which path served the request. Debugging becomes a coin flip.

  *PR #320*
</Accordion>

<Accordion title="Cache-miss compute is essential with multiple producers">
  **Rule:** When multiple code paths produce the same data, ensure every read path includes a cache-miss fallback that can recompute the value.

  **Why it matters:** If path A writes to cache but path B does not, requests served by path B will see a cache miss with no fallback — returning stale or empty data.

  *PR #342*
</Accordion>

## Replay system

<Accordion title="Mock services should return ALL data">
  **Rule:** Mock/stub services must return the full dataset, not a filtered subset based on `include` parameters.

  **Why it matters:** When the backend adds a new `include`, the mock breaks because it only returns the fields it knew about at the time it was written. Return everything and let the caller filter.

  *PR #322*
</Accordion>

<Accordion title="Replay fixtures must use current timestamps">
  **Rule:** Generate replay fixture timestamps relative to `now`, not hardcoded historical dates.

  **Why it matters:** Date-range queries (e.g. "matches today") will never return fixtures with hardcoded past dates, making them invisible during testing.

  *PR #319*
</Accordion>

<Accordion title="Document timing contracts between services explicitly">
  **Rule:** Write down the expected timing and ordering between cooperating services (e.g. "service A publishes within 2s of event, service B polls every 5s").

  **Why it matters:** Without explicit contracts, teams fix timing bugs by adjusting their own service, only for the other service to compensate in the opposite direction — creating ping-pong fix cycles.

  *PRs #319, #321*
</Accordion>

## Process

<Accordion title="Pre-push hooks: read remote refs from stdin">
  **Rule:** Pre-push hooks that check branch names must read the remote ref from stdin, not from `git rev-parse`. Users can bypass branch-name checks with `git push origin HEAD:main`.

  **Why it matters:** The hook receives the remote ref being pushed to via stdin. If it only checks the local branch name, direct pushes to protected branches slip through.

  *PR #342*
</Accordion>

<Accordion title="Delete old code in the same PR that replaces it">
  **Rule:** When replacing an architecture or subsystem, remove the old implementation in the same PR that introduces the new one.

  **Why it matters:** "I'll clean it up later" PRs rarely happen. Zombie code confuses new contributors, gets accidentally imported, and adds maintenance burden.

  *PR #282*
</Accordion>

<Accordion title="ADRs for simplification are as valuable as ADRs for complexity">
  **Rule:** Write an ADR when you remove or simplify a system, not just when you add complexity.

  **Why it matters:** Without a record of why something was simplified, future developers may re-introduce the complexity, not understanding that it was intentionally removed.

  *PR #313*
</Accordion>
