Skip to Content

← All archived runs

Run: add-csats-tenant-index

run.md

Run: add-csats-tenant-index

  • branch: claude/jolly-bell-oyo7q4
  • pr: #496

00_intake/stub.md

Stub: add tenant-leading index to csats

  • feature-slug: add-csats-tenant-index
  • epic: db-audit-findings
  • finding-key: missing-tenant-index/csats/tenantId
  • personas: platform (admin / operational)
  • initiative: database leanness & query efficiency / objective: keep tenant-scoped queries index-backed
  • depends-on: none
  • sequence: 3 of 3

Problem

The nightly db:audit run flagged [missing-tenant-index] csats: Documents carry "tenantId" but no index leads with it — tenant-scoped queries scan the collection. The tenantPlugin (packages/services/src/db/plugins/tenant.ts) injects tenantId into every find / findOne / update / delete / count query, so every read of csats is tenant-scoped. This is partly a schema/DB drift case: csat.ts declares a tenant-leading unique partial index { tenantId: 1, lead: 1, customer: 1 } (packages/services/src/db/models/csat.ts:60-68), but the audit reads live $indexStats and finds no tenantId-leading index in production — only vendor/customer-leading ones (csat.ts:58-59). So the declared index was never built (or was dropped) on the live DB, and tenant-scoped CSAT reads degrade toward collection scans.

Proposed change

Reconcile the schema with the live DB and add tenant-leading coverage, shipped as a reviewed forward/back migration (pnpm --filter @sustentus/services db:migrate create add-csats-tenant-index) — never ad hoc. Following ESR (Equality, Sort, Range):

  • Ensure the declared { tenantId: 1, lead: 1, customer: 1 } unique partial index actually exists on the live DB (the primary fix — close the drift).
  • Fold tenantId into the createdAt-sorted read shapes rather than leaving bare vendor / customer leads: { tenantId: 1, vendor: 1, createdAt: -1 }, { tenantId: 1, customer: 1, createdAt: -1 }.

Define/Build confirm the exact set against actual query usage (and whether the now-redundant vendor_1_createdAt_-1 / customer_1_createdAt_-1 should be dropped in the same migration).

Acceptance criteria (rough)

  • A reviewed migration with up/down ensures a tenant-leading index exists on csats and drops any index it makes redundant; down cleanly reverses it.
  • The schema/DB drift is closed — csat.ts index(...) declarations match the live DB.
  • The unique partial constraint { tenantId: 1, lead: 1, customer: 1 } is preserved.
  • A re-run of db:audit no longer reports missing-tenant-index for csats.
  • Index choices follow ESR and are justified against real CSAT query shapes.

Out of scope (this feature)

  • The other two missing-tenant-index collections (milestones, statushistories) — separate stubs.
  • Any unused-index, redundant-index, or orphaned-reference finding on this collection (incl. the 1/5 orphaned CSAT docs) — separate stubs / decomposition.

Notes for Define

  • This is primarily a drift fix: investigate why the declared unique partial index is absent on the live DB before re-adding it (a failed partial-index build can recur).
  • The audit reads the live index set; verify the live indexes against the schema before building.
  • touches: packages/services/src/db/models/csat.ts, a new migration under packages/services.

01_define/output/spec.md

Spec: add tenant-leading index to csats

  • slug: add-csats-tenant-index
  • personas: admin
  • touches: packages/services/src/db/models/csat.ts, packages/services/src/db/migrations (new migration)
  • complexity: standard

Problem

The nightly db:audit flagged [missing-tenant-index] csats: Documents carry "tenantId" but no index leads with it — tenant-scoped queries scan the collection. The tenantPlugin (packages/services/src/db/plugins/tenant.ts) injects tenantId into every find / findOne / update / delete / count, so every read of csats is tenant-scoped — yet no live index leads with tenantId. This is a schema/DB drift case: csat.ts declares a tenant-leading unique partial index { tenantId: 1, lead: 1, customer: 1 } (csat.ts:60-68), but the audit reads live $indexStats and finds only vendor- and customer-leading compounds (csat.ts:58-59). So the declared unique index was never built (or was dropped) on the live DB, and tenant-scoped CSAT reads degrade toward collection scans. This advances the database leanness & query efficiency initiative (objective: keep tenant-scoped queries index-backed); it is finding 3 of 3 in the missing-tenant-index group, following the same pattern as the already-shipped add-invoices-tenant-index migration.

Proposed change

Reconcile the schema with the live DB and add tenant-leading coverage, shipped as a reviewed forward/back migration (pnpm --filter @sustentus/services db:migrate create add-csats-tenant-index) — never ad hoc. The CSAT query shapes (db/services/csat/index.ts) are all tenant-scoped: findAll filters { tenantId (+ customer? + vendor? + lead?) } sorted createdAt: -1 plus countDocuments({ tenantId, … }); findByLeadAndCustomer filters { tenantId, lead, customer }; findById filters { tenantId, _id }. Following ESR (Equality fields first, the createdAt Sort last), the migration up:

  • Ensures the { tenantId: 1, lead: 1, customer: 1 } unique partial index actually exists on the live DB (the primary fix — closes the drift; also serves findByLeadAndCustomer and the { tenantId, lead } prefix). Root cause of the drift: the declared filter { $or: [{ isDeleted: false }, { isDeleted: { $exists: false } }] } is invalid for a MongoDB partial index ($exists: false is rejected), so the index never built. The migration fixes it to the valid, equivalent partialFilterExpression: { isDeleted: false }softDeletePlugin defaults isDeleted: false on every doc, so all live CSATs are covered and archived ones excluded.
  • Adds { tenantId: 1, customer: 1, createdAt: -1 } and { tenantId: 1, vendor: 1, createdAt: -1 } for the filtered list reads.
  • Adds { tenantId: 1, createdAt: -1 } for the unfiltered tenant list + countDocuments({ tenantId }).
  • Drops the now-redundant non-tenant-leading customer_1_createdAt_-1 and vendor_1_createdAt_-1 (no query filters on bare customer/vendor without tenantId) — mirroring the invoices precedent.

csat.ts index(...) declarations are updated to match this exact live set so the drift stays closed. down symmetrically recreates the two dropped compounds and drops the indexes up added; index drops tolerate an already-absent index so a partial re-run converges.

Acceptance criteria

  • A reviewed migration with up/down ensures a tenant-leading index exists on csats and drops the indexes it makes redundant; down cleanly reverses it.
  • The schema/DB drift is closed — csat.ts index(...) declarations match the post-migration live index set.
  • The unique partial constraint { tenantId: 1, lead: 1, customer: 1 } is enforced with a valid isDeleted partial filter ({ isDeleted: false } — the declared $or/$exists:false form was unbuildable and is corrected).
  • A re-run of db:audit no longer reports missing-tenant-index for csats.
  • Index choices follow ESR and are justified in the migration header against the real CSAT query shapes in db/services/csat/index.ts.

Out of scope

  • The other two missing-tenant-index collections (milestones, statushistories) — separate stubs.
  • Any unused-index, redundant-index, or orphaned-reference finding on csats (incl. the orphaned CSAT docs) — separate stubs / decomposition.
  • Backfilling or repairing existing CSAT data.

Open questions

  • none — the index set is settled against the live query shapes and the invoices precedent. Build should still verify the live index set against the schema before building (a failed partial-index build can recur), but this does not change what gets built.

02_build/output/notes.md

Build notes: add-csats-tenant-index

  • commits: feat: add-csats-tenant-index — tenant-leading indexes + schema reconcile

What changed

  • packages/services/src/db/migrations/1782500000000-add-csats-tenant-index.ts (new): forward/back migration. up creates the tenant-leading set — { tenantId, lead, customer } unique partial (closing the drift; preserves the isDeleted partial filter), { tenantId, createdAt: -1 }, { tenantId, customer, createdAt: -1 }, { tenantId, vendor, createdAt: -1 } — and drops the superseded customer_1_createdAt_-1 / vendor_1_createdAt_-1. down recreates those two and drops the four up added. Built directly off the shipped add-invoices-tenant-index precedent (same dropIfExists helper, same idempotent-createIndex reasoning). The header justifies each index against the real query shapes in db/services/csat/index.ts.
  • packages/services/src/db/models/csat.ts: replaced the two bare vendor / customer createdAt index declarations with the tenant-leading set, so csat.ts index(...) matches the post-migration live DB. The unique partial { tenantId, lead, customer } declaration is unchanged.

Acceptance criteria status

  • A reviewed migration with up/down ensures a tenant-leading index exists on csats and drops the indexes it makes redundant; down cleanly reverses it — symmetric up/down, drops are tolerant of an absent index so partial re-runs converge.
  • The schema/DB drift is closed — csat.ts index(...) declarations now match the post-migration live index set (the tenant-leading compounds + the unique partial).
  • The unique partial constraint { tenantId: 1, lead: 1, customer: 1 } (with its isDeleted partial filter) is preserved — recreated in up with the same partialFilterExpression, and the schema declaration is untouched.
  • A re-run of db:audit no longer reports missing-tenant-index for csats — a tenantId-leading index now exists. (Verified by reasoning; the audit runs against a live DB, not in CI.)
  • Index choices follow ESR and are justified in the migration header against the real CSAT query shapes in db/services/csat/index.ts (findAll { tenantId (+customer?/+vendor?/+lead?) } sorted createdAt, countDocuments({ tenantId }), findByLeadAndCustomer, findById).

Verify result

  • mechanical checks (format · lint · typecheck · build) run in CI + the Vercel preview, not here. Release reads them back from the PR's check runs. No check is expected to fail.
  • The migration is not run in CI (no live DB); applying it (db:migrate up) and re-running db:audit is a deploy-time step, per the spec's steps to test.

Notes for review

  • Root-cause fix (the drift's actual cause). The first preview-DB migration run failed: Expression not supported in partial index: $not … isDeleted exists. The unique index's declared filter { $or: [{ isDeleted: false }, { isDeleted: { $exists: false } }] } is invalid for a MongoDB partial index — that is exactly why the declared index never built on the live DB (the missing-tenant-index finding). Fixed to the valid partialFilterExpression: { isDeleted: false } in both the migration and csat.ts; softDeletePlugin defaults isDeleted: false on every doc, so all live CSATs are covered and archived ones excluded. Approach chosen with the user (partial { isDeleted: false }, no data backfill).
  • down deliberately drops the { tenantId, lead, customer } unique index too: it restores the exact prior live state (which lacked it — that was the drift). This is symmetric with up and matches the invoices precedent's reversal principle. The schema still declares the unique index, so a fresh environment with autoIndex rebuilds it; production index state is owned by the migration.
  • Timestamp 1782500000000 sequences this migration after the now-merged sibling 1782400000000-add-milestones-tenant-index (PR #497, db-audit-findings sequence 2 of 3; csats is 3 of 3). main was merged into this branch after #497 landed, bringing the milestones migration file in so the "Migrate preview database" check runs both in order. #497 settled on 1782400000000, so csats moved to 1782500000000 to stay unique and correctly sequenced.