Skip to Content

← All archived runs

Run: add-milestones-tenant-index

run.md

Run: add-milestones-tenant-index

  • branch: claude/tender-ritchie-v2xkfw
  • pr: #497

00_intake/stub.md

Stub: add tenant-leading index to milestones

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

Problem

The nightly db:audit run flagged [missing-tenant-index] milestones: 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 milestones is tenant-scoped. The collection's live indexes lead with expert, proposal and invoice, not tenantId (packages/services/src/db/models/milestone.ts:91-93), so tenant-scoped milestone reads can't seek on tenantId and degrade toward collection scans as data grows. milestones sits on the go-live / invoicing path (blocksGoLive, invoice / proposal references), so the additive, reversible tenant-leading index is a low-risk, high-value fix.

Proposed change

Add tenant-leading index coverage to milestones, shipped as a reviewed forward/back migration (pnpm --filter @sustentus/services db:migrate create add-milestones-tenant-index) — never ad hoc. Prefer folding tenantId into the existing compound indexes following ESR (Equality, Sort, Range) rather than a bare { tenantId: 1 }, so the real query shapes — by proposal, by invoice, by expert — are covered:

  • { tenantId: 1, proposal: 1, order: 1 }
  • { tenantId: 1, invoice: 1 }
  • { tenantId: 1, expert: 1 }

Note the existing { proposal: 1, order: 1 } is unique — folding tenantId in scopes that uniqueness per tenant, which is more correct, but Define/Build must confirm the intended uniqueness guarantee before changing it. They also confirm the exact set against actual query usage and whether the now-redundant non-tenant-leading compounds should be dropped in the same migration.

Acceptance criteria (rough)

  • A reviewed migration with up/down adds tenant-leading index(es) to milestones and drops any index it makes redundant; down cleanly reverses it.
  • milestone.ts schema index(...) declarations match the migration (schema and live DB agree).
  • The uniqueness guarantee on proposal/order is preserved or deliberately re-scoped per tenant, with the decision recorded.
  • A re-run of db:audit no longer reports missing-tenant-index for milestones.
  • Index choices follow ESR and are justified against real milestone query shapes.

Out of scope (this feature)

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

Notes for Define

  • Decide the uniqueness intent on { proposal, order } first — it changes the index definition.
  • The audit reads the live index set; verify the live indexes against the schema before building.
  • Read first, drop second: if the migration removes the old non-tenant-leading compounds, confirm nothing relies on them (db-auditor + mongodb-query-optimizer skills, ESR guidance).
  • touches: packages/services/src/db/models/milestone.ts, a new migration under packages/services.

01_define/output/spec.md

Spec: add tenant-leading index to milestones

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

Problem

The nightly db:audit run flags [missing-tenant-index] milestones: 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 on milestones, so every read is tenant-scoped. But the plugin only adds its own { tenantId: 1 } index when the schema doesn't already declare tenantId — and milestone.ts:50 declares it explicitly, so that fallback never fires. The collection's three live indexes lead with expert, proposal, and invoice (milestone.ts:91–93); none lead with tenantId. Tenant-scoped milestone reads therefore can't seek on tenantId and degrade toward collection scans as data grows. milestones sits on the go-live / invoicing path (blocksGoLive, invoice / proposal references), so this is a high-value, low-risk fix that advances the database leanness & query efficiency initiative (objective: keep tenant-scoped queries index-backed).

Proposed change

Add tenant-leading index coverage to milestones, shipped as a reviewed forward/back migration created with pnpm --filter @sustentus/services db:migrate create add-milestones-tenant-index — never ad hoc. Following ESR (Equality, Sort, Range), fold tenantId into the existing compound indexes as the leading equality field rather than adding a bare { tenantId: 1 }, so the real tenant-scoped query shapes (by expert, by proposal, by invoice) are covered:

  • { tenantId: 1, expert: 1, proposal: 1 }
  • { tenantId: 1, proposal: 1, order: 1 }unique (see uniqueness note below)
  • { tenantId: 1, invoice: 1, expert: 1 }

Uniqueness on { proposal, order } — decided. The live { proposal: 1, order: 1 } index is unique. Folding tenantId in makes uniqueness per-tenant. Because a proposal is tenant-owned (each proposal document carries exactly one tenantId, and a milestone references one proposal), every (proposal, order) pair already belongs to a single tenant — so { tenantId, proposal, order } unique preserves the existing "one milestone per (proposal, order)" guarantee with no behavioural change, while being more correct for a tenant-scoped collection. This run therefore re-scopes the uniqueness per tenant; it does not loosen or drop it.

Build confirms the exact set against actual milestone query usage in the codebase before writing the migration, and decides — read first, drop second — whether the now-redundant non-tenant-leading compounds ({ expert, proposal }, { proposal, order }, { invoice, expert }) should be dropped in the same migration, verifying nothing relies on them. The milestone.ts schema index(...) declarations are updated to match the migration so schema and live DB agree.

Acceptance criteria

  • A reviewed migration with up/down adds the tenant-leading index(es) to milestones and drops any index it makes redundant; down cleanly reverses it (re-creating dropped indexes, removing added ones).
  • The { proposal, order } uniqueness guarantee is preserved, re-scoped per tenant as { tenantId, proposal, order } unique — not dropped or loosened.
  • The milestone.ts schema index(...) declarations match the migration exactly, so the schema and the live DB agree.
  • Re-running db:audit no longer reports missing-tenant-index for milestones.
  • Each chosen index follows ESR and is justified in the migration/PR against a real milestone query shape (no speculative over-indexing).

Out of scope

  • The other two missing-tenant-index collections (csats, statushistories) — separate stubs.
  • Any unused-index, redundant-index, or orphaned-reference finding on this collection (including the 2/52 orphaned milestone docs) — separate stubs / decomposition.
  • Backfilling, reshaping, or migrating milestone data — this run only touches index definitions.

Open questions

  • none. (The uniqueness intent on { proposal, order } is settled above — preserved and re-scoped per tenant. The precise final index set and whether to drop the now-redundant non-tenant-leading compounds are settled in direction here — lead with tenantId, ESR, drop only what is provably redundant — and confirmed by Build against the code; neither blocks an acceptance criterion.)

02_build/output/notes.md

Build notes: add-milestones-tenant-index

  • commits: feat: add-milestones-tenant-index — tenant-leading milestone indexes + migration

What changed

  • packages/services/src/db/migrations/1782400000000-add-milestones-tenant-index.ts (new): forward/back migration. up creates two tenant-leading indexes and drops the three non-tenant-leading compounds; down recreates the three originals (incl. the global-unique { proposal, order }) and drops the two added. Drops are tolerant of an absent index so a partial re-run converges. Mirrors the shipped add-invoices-tenant-index migration pattern.
  • packages/services/src/db/models/milestone.ts: replaced the three old index(...) declarations ({ expert, proposal }, { proposal, order } unique, { invoice, expert }) with the two tenant-leading ones so the schema and the live DB agree.

Index decision (grounded in real query shapes)

Swept every milestones query across the services. The real, tenant-scoped shapes are:

  • { tenantId, proposal } + sort order — dominant (milestone service findByProposalOrdered*, findDelivery*, findIdsByProposal, archiveByProposal; invoice service isFinalMilestoneForProposal with sort({ order: -1 }); leads service updateMany({ tenantId, proposal: { $in } })). → { tenantId: 1, proposal: 1, order: 1 } unique (E,E,S). Re-scopes the old global-unique { proposal, order } per tenant — safe because a proposal is tenant-owned, so the guarantee is preserved with no behavioural change.
  • { tenantId, expert } — sdm service distinct("proposal", { tenantId, expert }) and the expert-scoped aggregate $match. → { tenantId: 1, expert: 1 }.
  • { tenantId, _id, [expert] }advanceStatus updateOne and the by-id findOnes; served by the default _id index, no compound needed.

No query filters milestones by invoice (the only invoice touch is findByIdAndUpdate(_id, { $set: { invoice } }), keyed on _id). So the old { invoice, expert } index is dropped without replacement — a { tenantId, invoice, … } index would be speculative, which the spec forbids. Read-first/drop-second confirmed: nothing relies on any of the three dropped compounds once every query is tenant-scoped.

Acceptance criteria status

  • Reviewed migration with up/down adds tenant-leading index(es) and drops redundant ones; down cleanly reverses it — symmetric up/down in the new migration.
  • { proposal, order } uniqueness preserved, re-scoped per tenant as { tenantId, proposal, order } unique — not dropped or loosened.
  • milestone.ts schema index(...) declarations match the migration's end state exactly.
  • Re-running db:audit no longer reports missing-tenant-index for milestones — both new indexes lead with tenantId (verified by construction; the audit reads the live index set after the migration applies on merge).
  • Each chosen index follows ESR and is justified against a real milestone query shape; the unjustified invoice index was dropped rather than folded (no speculative over-indexing).

Verify result

  • Mechanical checks (format · lint · typecheck · build) run in CI + the Vercel preview, not here. The migration reuses the exact types/structure of the shipped invoices migration; no type errors expected. The migration itself applies on merge to main via db-migrate.yaml.

Notes for review

  • The deviation from the spec's tentative index list (it floated { tenantId, invoice, expert }) is deliberate and spec-sanctioned: the spec delegated confirming the exact set against real query usage to Build and forbade speculative over-indexing. No milestone query reads by invoice, so that index is dropped, not folded.
  • { tenantId, proposal, order } is unique; creating it cannot fail on existing data because the per-tenant constraint is implied by the prior global-unique { proposal, order }.

03_release/output/investor-update.md

Milestone tracking stays fast as delivery volume grows

Who it's for: Every persona that tracks delivery milestones (platform-wide) What shipped: Milestone lookups are now backed by tenant-leading database indexes instead of scanning the collection. Why it matters: Keeps the go-live and delivery-tracking path reliable at volume — advancing Scale the Bridge and our Q2 goal to validate technical infrastructure.

Dig deeper: <merged-PR URL>

03_release/output/release.md

Release: add-milestones-tenant-index

  • pr: #497 (https://github.com/sustentus/sustentus/pull/497) · merged: pending squash-merge (Ready-to-merge ticked, CI green)
  • CI: green on the re-run after the shared preview DB was cleaned (Format · Lint · Typecheck · Audit database · Migrate preview database all ✅; Migrate production database skipped on PR). The earlier Migrate preview database failures were preview-DB orphan pollution, fixed by PR #499 (prune orphaned migration records), not by this change.
  • technical docs: no technical docs impact — adds a migration + index declarations; the DB-lifecycle doc (migrate/seed/audit) is unchanged and no page enumerates per-collection indexes
  • business docs: no business docs impact — milestone queries return the same results, just index-backed; no persona-facing behaviour change
  • release notes: investor-only — no end-user note (internal performance/infra change). Investor draft in this PR; no changelog entry
  • deploy: pending merge
  • sent: none — investor send intentionally skipped/batched (user decision). This is the 2nd of 3 near-identical DB-index follow-ups; a single combined investor update for the db-leanness initiative can go out once the index work (incl. csats) is done. Investor draft retained in this PR for that batch.

Review summary

  • /code-review (high effort) on the feature diff (migration + milestone.ts) found no correctness bugs. Confirmed: up creates the two tenant-leading indexes before dropping the three old compounds (no read gap); per-tenant { tenantId, proposal, order } unique is implied by the prior global { proposal, order } unique (a proposal is tenant-owned), so creation can't fail on existing data; dropIfExists swallows only IndexNotFound and rethrows otherwise; down is an exact symmetric reversal; schema index(...) matches the migration end-state; CONVENTIONS satisfied (arrow fns, type, async/await, named imports).
  • Nit fixed on branch: the milestone.ts index comment cited the pre-rename migration timestamp 1782300000000; corrected to 1782400000000.

Acceptance check (vs spec)

  • Reviewed migration with up/down adds tenant-leading index(es) and drops the redundant compounds; down reverses it — 1782400000000-add-milestones-tenant-index.ts
  • { proposal, order } uniqueness preserved, re-scoped per tenant as { tenantId, proposal, order } unique
  • milestone.ts index(...) declarations match the migration exactly
  • db:audit no longer reports missing-tenant-index for milestones — both new indexes lead with tenantId; the Audit database CI job is green
  • Index choices follow ESR and are justified against real milestone query shapes (build notes); the unjustified invoice index was dropped, not folded (no speculative over-indexing)