perf: Add tool_id index for 77x faster error queries

[evansenter]

Summary

• Add migration v7 with index on tool_id column
• Reduces get_error_details() query time from 25s to 0.3s (77x faster)

Problem

The query_error_details() function self-joins the events table:

FROM events e1
JOIN events e2 ON e1.tool_id = e2.tool_id AND e2.entry_type = 'tool_use'
WHERE e1.is_error = 1

Without an index on tool_id, SQLite was scanning 160K rows. The query plan showed:

• SCAN e2 USING INDEX idx_events_tool
• AUTOMATIC PARTIAL COVERING INDEX

Solution

Migration v7 adds idx_events_tool_id:

CREATE INDEX IF NOT EXISTS idx_events_tool_id ON events(tool_id)

Test plan

• All 300 tests pass
• Migration runs on make restart
• Query time verified: 25s → 0.3s

Related: #63 (comprehensive API benchmarking)

🤖 Generated with Claude Code

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR adds a migration (v7) to create an index on tool_id for 77x faster error detail queries. The migration is well-documented and the performance improvement is significant.

Issues Found

Critical

None

Important

• storage.py:382-390 - Schema consistency: The idx_events_tool_id index is added in migrate_v7() but NOT in _init_db(). Per the codebase comment at lines 175-180: "Schema elements are defined in BOTH migrations AND _init_db(). This is intentional: _init_db() defines the complete current schema for fresh installs - Migrations incrementally upgrade existing databases to the current schema. Both paths must result in identical schemas." Fresh database installs will NOT have this index, only migrated databases will.

  Fix: Add conn.execute("CREATE INDEX IF NOT EXISTS idx_events_tool_id ON events(tool_id)") in _init_db() alongside the other event indexes (around line 549).

Suggestions

• tests/test_storage.py - Missing test: There are existing tests test_index_on_parent_uuid and test_index_on_agent_id (lines 974-984) that verify indexes exist. Consider adding a similar test_index_on_tool_id test for consistency and to prevent regression.

Verdict

REQUEST_CHANGES - The index must be added to _init_db() to ensure schema consistency between fresh installs and migrated databases.

---

Automated review by Claude Code

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR adds migration v7 to create an index on tool_id for 77x faster error detail queries (25s → 0.3s). The previous review's critical feedback about adding the index to _init_db() has been addressed.

Issues Found

Critical

None

Important

None

Suggestions

• tests/test_storage.py - Missing test: The previous review suggested adding a test_index_on_tool_id test for consistency with existing tests (test_index_on_parent_uuid, test_index_on_agent_id). This would prevent regression and follow established patterns:

def test_index_on_tool_id(self, storage):
    """Verify that idx_events_tool_id index exists."""
    rows = storage.execute_query("PRAGMA index_list(events)")
    indexes = {row[1] for row in rows}
    assert "idx_events_tool_id" in indexes

Verdict

REQUEST_CHANGES - Minor suggestion for test coverage consistency.

---

Automated review by Claude Code

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR adds migration v7 to create an index on tool_id for 77x faster error detail queries (25s → 0.3s). All feedback from previous reviews has been addressed.

Issues Found

Critical

None

Important

None

Suggestions

None

Verdict

APPROVE - Code looks good, no issues found. The migration is well-documented, the index is correctly added to both migrate_v7() and _init_db() for schema consistency, and a regression test has been added following the established pattern.

---

Automated review by Claude Code