fix(server): stop maintaining the unused event correlation index #23171 · pingdotgg/t3code

Added 3 commits4d ago

Merge origin/main into fix/issue-205344faeac5
test(server): reproduce unused correlation index (issue #20534)5dedf45
fix(server): drop the unused orchestration event correlation indexa623c6b

MmacroscopeappBotapproved these changes3d ago

Approvability

Verdict: Approved at 4faeac5

Macroscope's review found this PR approvable — This focused server migration removes an unused SQLite index while preserving the correlation column, stored values, and other indexes. The accompanying tests verify the migration history and schema invariants, with no product-default or user-facing behavior changes.

^{You can add or adjust custom eligibility rules. Learn more.}

Ggithub-actionsBot3d ago

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	20.9 KiB	20.9 KiB	0 B (0.0%)	29.3 KiB	✅
Codex	Live turn messages	2	2	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	+7 B (+0.1%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	+7 B (+0.6%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.2 KiB	21.2 KiB	0 B (0.0%)	29.3 KiB	✅
Claude	Live turn messages	2	2	0 (0.0%)	8	✅

Baseline: 89fe509 · PR result: 4faeac5 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

Codex decoded thread snapshot: 108.5 KiB
Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

CcoderabbitaiBot3d ago

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.config.ts
Review profile: CHILL
Plan: Team
Run ID: 3ffe79b2-cc8f-4f62-9f7c-88edd361c164

📥 Commits

Reviewing files that changed from the base of the PR and between 89fe509232e787b51d0cf695861afe73316ce85b and 4faeac514204f78c537380ba5a8b7ae2401ce83c.

📒 Files selected for processing: 4

apps/server/src/persistence/Migrations.test.ts
apps/server/src/persistence/Migrations.ts
apps/server/src/persistence/Migrations/068_DropOrchestrationEventCorrelationIndex.ts
apps/server/src/persistence/reconcileV2PreviewMigration.test.ts

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

Migration 68 drops idx_orch_events_correlation_id without changing the correlation_id column or its values. The migration is registered, and migration tests verify index removal, retained indexes, correlation ID preservation, and upgrade migration lists.

Changes: Correlation index migration

Layer / File(s)	Summary
Implement and register migration `apps/server/src/persistence/Migrations/068_DropOrchestrationEventCorrelationIndex.ts`, `apps/server/src/persistence/Migrations.ts`	Migration 68 drops `idx_orch_events_correlation_id` if it exists and is added to the migration entries.
Verify migration behavior and upgrade lists `apps/server/src/persistence/Migrations.test.ts`, `apps/server/src/persistence/reconcileV2PreviewMigration.test.ts`	Tests verify that the correlation ID and specified indexes remain, the correlation index is removed, and preview-upgrade expectations include migration 68.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: shivamhwp

Merge Risk: ⚪ Minimal · up to 4faea

The index can be removed without a supported impact on event data or the inspected query paths. No actionable issue remains before merge.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title clearly and concisely describes the main change: stopping maintenance of the unused event correlation index.
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.
Description check	Passed	The pull request description includes all required sections. It clearly states the problem, change, scope and approval, verification results, limitations, runtime validation, and the agent and harness…

Full details: Description check

Explanation

The pull request description includes all required sections. It clearly states the problem, change, scope and approval, verification results, limitations, runtime validation, and the agent and harness used.

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

fix(server): stop maintaining the unused event correlation index#23171

Problem

Change

Scope and approval

Verification

Checks

Approvability

Thread transfer impact