perf(api): add composite indexes for SyncController.GetChangesV2 scans

[thomasluizon]

What

SyncController.GetChangesV2 (src/Orbit.Api/Controllers/SyncController.cs:146) is the delta-sync endpoint: for each of seven synced entities it fetches every row changed since a client-supplied timestamp, filtering by the entity's owner key plus UpdatedAtUtc. None of those predicates had a supporting composite index, so each query degraded to a scan filtered on UpdatedAtUtc.

This adds one leading-column btree index per entity, matching each query's exact predicate:

Entity	GetChangesV2 predicate	Index added
Habits	UserId == u && UpdatedAtUtc > since	(UserId, UpdatedAtUtc)
Goals	UserId == u && UpdatedAtUtc > since	(UserId, UpdatedAtUtc)
Tags	UserId == u && UpdatedAtUtc > since	(UserId, UpdatedAtUtc)
Notifications	UserId == u && UpdatedAtUtc > since	(UserId, UpdatedAtUtc)
ChecklistTemplates	UserId == u && UpdatedAtUtc > since	(UserId, UpdatedAtUtc)
HabitLogs	HabitId IN (...) && UpdatedAtUtc > since	(HabitId, UpdatedAtUtc)
GoalProgressLogs	GoalId IN (...) && UpdatedAtUtc > since	(GoalId, UpdatedAtUtc)

HabitLogs and GoalProgressLogs have no UserId column — GetChangesV2 fetches them by the parent HabitId/GoalId set. The correct index keys off those FK columns, not UserId.

ProcessedRequests.UserId — intentionally skipped (no dedicated FK index)

The audit flagged ProcessedRequests.UserId as lacking an FK index. It does not need one: the table already has the composite unique index (UserId, IdempotencyKey, RequestType), whose leading column is UserId, so it fully serves the ON DELETE CASCADE lookup. That is exactly why EF's ForeignKeyIndexConvention did not auto-generate a standalone index there. Adding one would be a redundant write-amplifying duplicate. A test locks this rationale in.

#338-safety

Every index is emitted as CREATE INDEX IF NOT EXISTS (with DROP INDEX IF EXISTS on Down), matching the repo's post-#338 pattern in AdoptPerformanceIndexes. I grepped all of src/Orbit.Infrastructure/Migrations/** — none of these index names or (col, UpdatedAtUtc) column combinations exist in any prior migration, and UpdatedAtUtc appears in zero existing indexes. So the migration cannot hit the 42P07 relation already exists deploy crash.

Tests

• SyncChangesIndexConfigurationTests — a Theory asserting each synced entity declares the composite index in the exact column order (order matters for btree usability), plus a Fact verifying the ProcessedRequest composite index leads with UserId.
• dotnet ef migrations has-pending-model-changes → clean (model ↔ snapshot in sync).
• Full suite green: 4886 passed, 0 failed.

Behavior-preserving; performance only.

Refs thomasluizon/orbit-ui-mobile#243

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #352 — perf(api): add composite indexes for SyncController.GetChangesV2 scans

Recommendation: APPROVE

Summary

Narrowly-scoped, behavior-preserving performance PR: adds 7 composite btree indexes (via both the EF fluent-API in OrbitDbContext.cs and a matching idempotent raw-SQL migration) to support the exact predicates SyncController.GetChangesV2 (src/Orbit.Api/Controllers/SyncController.cs:146-190) uses for delta-sync — no controller code, DTOs, or endpoints changed. Every one of the 7 LINQ predicates in GetChangesV2 was traced against the added indexes and they match column-for-column and in the correct leading-column order; the "no UserId on HabitLog/GoalProgressLog" and "ProcessedRequests already covered by its composite unique index" claims in the PR body both check out against the actual entity/config code.

Findings

Critical

None.

High

None.

Medium

None.

Low / Info

• Migration uses plain CREATE INDEX, not CONCURRENTLY (src/Orbit.Infrastructure/Migrations/20260712190945_AddSyncChangesV2Indexes.cs:14-46) — each of the 7 statements takes a SHARE lock, blocking writes during the build. This exactly matches established repo precedent (20260613001629_AdoptPerformanceIndexes.cs uses the identical idiom) — not a regression, flagged for awareness only.
• Some entities now carry 3 indexes all leading with the same column (e.g. Goal: UserId / (UserId,IsDeleted) / (UserId,UpdatedAtUtc) in OrbitDbContext.cs:855-857) — partially redundant with the standalone single-column index, marginal write-amplification cost. Pre-existing pattern, not introduced by this diff; out of scope here.

Subagents

Agent	Verdict
security-reviewer	PASS — no SQL injection (compile-time literal identifiers), no auth/authz surface touched, idempotency guard is standard practice
contract-aligner	N/A — no DTO/Controller route/packages/shared type changed

What's good

• Exact predicate-to-index mapping traced and verified against the live GetChangesV2 LINQ.
• Correctly identifies HabitLog/GoalProgressLog have no UserId column and indexes the actual FK used (HabitId/GoalId) instead of blindly following the UserId pattern of the other 5 entities.
• ProcessedRequests "intentionally skipped" rationale is correct and locked in by a dedicated test, not just prose.
• Migration follows the established idempotent IF NOT EXISTS/IF EXISTS idiom, with Down() in exact reverse order of Up().
• New SyncChangesIndexConfigurationTests.cs asserts via EF InMemory model inspection that each of the 7 synced entities declares the composite index in the exact expected column order — will catch future drift if query predicates change without the index following.

Validation

Build/Unit Tests/SonarCloud run as separate required CI checks for this PR (skipped here per workflow). PR body reports the full suite green and dotnet ef migrations has-pending-model-changes clean.

Recommendation

Merge as-is. No Critical or High findings. The two Info-level observations are pre-existing repo conventions, not regressions from this diff.