super-productivity/ARCHITECTURE-DECISIONS.md
Johannes Millan 2d9988dd73
perf(sync): SuperSync server speed + correctness hardening (#7621)
* docs(sync): add super sync server perf plan

* perf(sync): implement supersync server perf phases

* fix(sync): bracket auth cache invalidation

* fix(sync): avoid empty replay state stringify

* fix(sync): harden supersync batch uploads

* fix super sync review findings

* fix(sync): guard payload bytes backfill rollout

* perf(sync): speed up payload_bytes backfill and index its scan

Raise the backfill batch size (DEFAULT 5->500, MAX 25->1000) so a
100M-row operations table backfills in minutes rather than tens of
hours. Add a CONCURRENTLY partial index on (user_id, id) WHERE
payload_bytes = 0: it drains to empty post-backfill so the boot-time
backfill self-check and the BOOL_OR quota probe stop doing a full
sequential scan to prove absence, and it makes the backfill's per-user
keyset paging a true index seek. Wire the new concurrent-index
migration into both deploy scripts' P3018 recovery path. Add
migration-SQL guard tests for the ADD COLUMN (metadata-only fast path)
and the new partial index.

* fix(sync): bound auth cache invalidation map and bracket every delete

The auth verification cache's invalidationVersions map grew one entry
per lifetime-invalidated user with no eviction (unbounded heap on a
long-lived single replica). Cap it at the same 10k LRU bound as the
entries map, re-inserting the just-invalidated user at the MRU tail so
the CAS race protection still holds for the only window that matters
(one DB round trip). Bracket the passkey/magic-link registration
cleanup deletes with pre+post invalidate to match the documented
convention, and invalidate on verifyEmail so a freshly-verified user
isn't denied for up to the cache TTL.

* perf(sync): skip the redundant exact replay-state measurement

The delta accounting is a proven over-estimate of the serialized state
size, so when the running bound stays within the cap the true size is
too and the final exact JSON.stringify is provably redundant. Skip it
in that case (still measure-and-throw whenever the bound does not prove
safety). This collapses the common small/incremental replay back to
zero expensive full stringifications, matching the old per-op loop
instead of regressing it. Name the entity-key JSON overhead constant
and document that assertReplayStateSize's return value is load-bearing.

* refactor(sync): split processOperationBatch into pipeline stages

Extract the 297-line batch upload method into a thin orchestrator plus
six named single-responsibility stage helpers (validate+clamp, intra-
batch dedupe, classify existing duplicates, conflict-detect, reserve
seq + insert, full-state clock). Behavior-preserving: every stage
writes terminal rejections into the shared results array by index and
the two empty-set guards short-circuit exactly as before. Also share
the timestamp clamp, the duplicate-op SELECT, and the merged
full-state clock persistence between the batch and legacy paths so
they cannot silently diverge.

* test(sync): pin batch error-code divergence and aggregate-once

Strengthen the intra-batch duplicate test to assert same-id /
different-content yields DUPLICATE_OPERATION (deliberate divergence
from the legacy INVALID_OP_ID), and document the divergence in the
plan. Replace the single-full-state aggregate test with two
full-state ops + a spy asserting _aggregatePriorVectorClock runs
exactly once and last-write-wins — the old test could not catch a
per-op-aggregate regression. Add a makeOp fixture factory. Correct
the plan's overstated replay-stringification numbers.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: johannesjo <1456265+johannesjo@users.noreply.github.com>
2026-05-15 17:24:16 +02:00

8.7 KiB

Architecture Decision Records

This document tracks significant architectural decisions and patterns in the Super Productivity codebase. When making changes that affect these patterns, reference this document and update it if needed.

Active Patterns & Decisions

1. dueDay/dueWithTime Mutual Exclusivity Pattern

Status: Active (since commit 400ca8c1, 2026-01-29)

Decision: The task.dueDay and task.dueWithTime fields are mutually exclusive in new data. When setting dueWithTime, dueDay must be cleared (set to undefined). When reading, dueWithTime takes priority over dueDay.

Rationale:

  • Prevents state inconsistency bugs where both fields had conflicting values
  • Single source of truth for task scheduling
  • Simpler state management

Implementation:

  • Writing: Clear dueDay when setting dueWithTime (in meta-reducers)
  • Reading: Check dueWithTime first; only check dueDay if dueWithTime is not set (in selectors)
  • Legacy Data: Old data with both fields works via priority pattern (no migration needed)

Key Files:

When to Update This Pattern:

  • Adding new date/time scheduling fields
  • Modifying task scheduling logic
  • Working with task selectors that check due dates

2. TODAY_TAG Virtual Tag Pattern

Status: Active (established pattern)

Decision: TODAY_TAG (ID: 'TODAY') is a virtual tag whose membership is determined by task.dueWithTime or task.dueDay, not by task.tagIds. The tag's taskIds field stores only the ordering of tasks, not membership.

Key Invariant: TODAY_TAG.id must NEVER be added to task.tagIds

Rationale:

  • Uniform move operations across all tags (virtual and regular)
  • Single source of truth for "today" membership (date fields, not tagIds)
  • Self-healing ordering (stale entries automatically filtered)
  • Natural integration with planner (which uses date fields)

Related: Uses the dueDay/dueWithTime mutual exclusivity pattern (Decision #1)

Key Files:

When to Update This Pattern:

  • Adding new virtual tags
  • Modifying tag membership logic
  • Working with today's task list

3. Sync Package Boundary Direction

Status: Active (since May 2026)

Decision: Operation-log sync code is split by dependency direction: src/app composes host-specific wiring, @sp/sync-providers owns bundled provider implementations, and @sp/sync-core owns framework-agnostic reusable sync primitives.

Rationale:

  • Keeps reusable sync algorithms independent of Angular, NgRx, app models, and provider implementations
  • Prevents provider IDs, app action/entity enums, validation schemas, UI, OAuth, and platform bridges from leaking into the core engine package
  • Gives boundary lint a clear rule: packages never import app code, and providers consume only public sync-core exports

Implementation:

  • ESLint rejects Angular, NgRx, app, shared-schema, sync-core deep imports, and dynamic imports inside package sources
  • @sp/sync-core has no runtime dependencies and owns vector-clock algorithms used by client/server compatibility paths
  • packages/shared-schema compatibility-re-exports generic vector-clock algorithms from @sp/sync-core; @sp/sync-core must not import @sp/shared-schema
  • @sp/sync-providers depends on public @sp/sync-core plus provider runtime helpers, while app factories inject credentials, platform bridges, validators, OAuth routing, and config

Documentation: docs/sync-and-op-log/package-boundaries.md

Key Files:

When to Update This Pattern:

  • Moving sync code between app and packages
  • Adding a package export or dependency
  • Adding a provider implementation or plugin-facing provider contract
  • Changing vector-clock ownership or shared-schema compatibility

4. Batch Uploads Under RepeatableRead

Status: Active (since May 2026)

Decision: SuperSync batch uploads derive conflict-safety from the shared user_sync_state.lastSeq row write that reserves server sequence numbers, not from PostgreSQL RepeatableRead snapshot isolation alone.

Rationale:

  • PostgreSQL RepeatableRead does not provide full serializable snapshot isolation
  • Two concurrent upload transactions can both pass conflict prefetch checks when they read the same pre-insert snapshot
  • Reserving sequence numbers through one user_sync_state.lastSeq row forces accepted writers for the same user to serialize on that row lock
  • If two batches race, the later writer blocks on the row and the transaction retry path handles the serialization failure rather than silently accepting conflicting operations

Implementation:

  • Batch upload conflict detection runs in memory against prefetched latest entity rows and updates that map as operations are accepted
  • Accepted operations reserve one contiguous sequence range with INSERT ... ON CONFLICT ... DO UPDATE SET last_seq = last_seq + delta
  • The batch insert does not use skipDuplicates; an unexpected unique conflict aborts the transaction and lets the request retry
  • Removing or sharding the lastSeq write requires replacing this safety mechanism with an equivalent per-user serialization primitive

Documentation: docs/sync-and-op-log/diagrams/02-server-sync.md

Key Files:

When to Update This Pattern:

  • Changing upload conflict detection
  • Changing server sequence assignment
  • Changing transaction isolation for upload operations
  • Introducing multi-writer or multi-region upload processing

How to Use This Document

When Making Architectural Changes

  1. Before implementing: Check if your change affects any active pattern
  2. During implementation: Follow the documented patterns
  3. After implementation: Update this document if you've:
    • Changed an existing pattern
    • Added a new architectural pattern
    • Made a decision that affects future development

When to Add a New Decision

Add a new decision record when:

  • The decision affects multiple files/modules
  • Future developers need to understand "why" not just "what"
  • The pattern needs to be followed consistently across the codebase
  • The decision prevents a specific class of bugs

Decision Record Template

### N. [Pattern/Decision Name]

**Status**: ✅ Active | 🚧 Draft | ⚠️ Deprecated | ❌ Superseded

**Decision**: [One-sentence summary of the decision]

**Rationale**:

- [Why was this decision made?]
- [What problems does it solve?]

**Implementation**:

- [How is it implemented?]
- [Key techniques or patterns used]

**Documentation**: [Link to detailed docs]

**Key Files**: [List of primary files implementing this pattern]

**When to Update This Pattern**: [Scenarios when someone should review/update this]


Commit Reference

When committing changes related to these patterns, reference this document and the specific decision:

feat(tasks): implement feature X

Uses dueDay/dueWithTime mutual exclusivity pattern (ARCHITECTURE-DECISIONS.md #1)