Skip to content

perf(server): stop maintaining redundant projection indexes - #12847

Merged
juliusmarminge merged 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
extoci:perf/remove-redundant-projection-indexes
Sep 21, 2026
Merged

juliusmarminge merged 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
extoci:perf/remove-redundant-projection-indexes

Conversation

@extoci

@extoci extoci commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Four projection indexes are strict prefixes of later ordering indexes. SQLite can use the longer indexes for the same lookups, but every projection write still maintained both B-trees.

Drop the redundant indexes in migration 55, after Orchestrator v2 migration 54, and keep the covering indexes that serve the existing read plans. The migration is safe to rerun and a focused test verifies both sides of that contract.

The benchmark used a 1.25 GB snapshot of a real T3 database and representative activity rows with approximately 3.9 KB payloads.

Benchmark Before After Change
Insert 10,000 activity rows 580.8 ms 520.6 ms 10.4% faster
Read 1,000 activity rows 14.3 ms 13.8 ms No regression
Database storage — 5.48 MB reclaimed Smaller

The representative read kept the same covering-index query plan after the migration.

Validation: the focused migration test runs through Orchestrator v2 migration 54 and the cleanup migration 55. The v2 server typecheck passes, and targeted lint and formatting pass. The copied-database benchmark exercised the same index drops before retargeting.

Model: GPT-5.6 Sol. Harness: Codex in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 21, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 21, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 5737d93

Macroscope's review found this PR approvable — This adds a one-time migration that removes four redundant SQLite indexes while retaining composite indexes with the same lookup prefixes. The change is narrowly scoped, preserves query semantics, and includes coverage for normal and preview upgrade paths.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 9a40ed18-22d7-4467-b156-e9123dabe41d

📥 Commits

Reviewing files that changed from the base of the PR and between 61b25b8 and 695a963.

📒 Files selected for processing (3)
  • apps/server/src/persistence/Migrations.ts
  • apps/server/src/persistence/Migrations/054_RemoveRedundantProjectionIndexes.test.ts
  • apps/server/src/persistence/Migrations/054_RemoveRedundantProjectionIndexes.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Migration 054 removes four redundant projection indexes. The migration registry includes it for execution and manifest generation. An in-memory SQLite test verifies that the redundant indexes are absent and covering indexes remain.

Changes

Projection Index Cleanup

Layer / File(s) Summary
Projection index removal
apps/server/src/persistence/Migrations/054_RemoveRedundantProjectionIndexes.ts
Migration 054 uses SqlClient to execute four DROP INDEX IF EXISTS statements.
Migration registration and validation
apps/server/src/persistence/Migrations.ts, apps/server/src/persistence/Migrations/054_RemoveRedundantProjectionIndexes.test.ts
The migration is imported and registered with ID 54. The SQLite test verifies the removed and retained indexes.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: stopping maintenance of redundant projection indexes.
Description check ✅ Passed The description explains what changed, why it changed, the performance impact, migration safety, preserved indexes, and validation results. It does not use the template headings or include the checkli…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge juliusmarminge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retarget against v2 pls. Would prefer not introducing more migrations on main cause numbers conflicts

@extoci
extoci force-pushed the perf/remove-redundant-projection-indexes branch from 695a963 to 5aa2c9a Compare September 21, 2026 10:11
@macroscopeapp
macroscopeapp Bot dismissed their stale review September 21, 2026 10:12

Dismissing prior approval to re-evaluate 5aa2c9a

@extoci
extoci changed the base branch from main to t3code/codex-turn-mapping September 21, 2026 10:12
@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Sep 21, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Macroscope has since reviewed this pull request. An earlier review was skipped by a cost limit; a review has now completed, so that notice no longer applies.

@extoci
extoci force-pushed the perf/remove-redundant-projection-indexes branch from 5aa2c9a to 9834cc9 Compare September 21, 2026 10:17
@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:XXL 1,000+ changed lines (additions + deletions). labels Sep 21, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 21, 2026
@extoci
extoci force-pushed the perf/remove-redundant-projection-indexes branch from 9834cc9 to 5737d93 Compare September 21, 2026 12:13
@macroscopeapp
macroscopeapp Bot dismissed their stale review September 21, 2026 12:13

Dismissing prior approval to re-evaluate 5737d93

@juliusmarminge
juliusmarminge merged commit 32490b9 into pingdotgg:t3code/codex-turn-mapping Sep 21, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants