Skip to content

docs: Add schema design documentation#65

Closed
evansenter wants to merge 2 commits into
perf-tool-id-indexfrom
docs-schema
Closed

docs: Add schema design documentation#65
evansenter wants to merge 2 commits into
perf-tool-id-indexfrom
docs-schema

Conversation

@evansenter

Copy link
Copy Markdown
Owner

Summary

  • Add docs/SCHEMA.md with comprehensive database schema documentation
  • Link from CLAUDE.md and README.md

Contents

The schema docs cover:

  • Design principles (don't over-distill, aggregate→drill-down)
  • All 8 tables with column definitions
  • 8 indexes with their purposes
  • FTS5 full-text search setup
  • Migration history (v1-v7)
  • Schema evolution guidelines

Test plan

  • Docs render correctly in GitHub
  • Links work from CLAUDE.md and README.md

🤖 Generated with Claude Code

@claude

claude Bot commented Jan 8, 2026

Copy link
Copy Markdown

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

Code Review

Summary

This PR adds comprehensive database schema documentation to docs/SCHEMA.md and links it from CLAUDE.md and README.md. The documentation covers tables, indexes, FTS setup, and migration history.

Issues Found

Critical

None

Important

  • docs/SCHEMA.md:189-195 - Documentation claims migration 7 (add_tool_id_index) exists, but the actual codebase has SCHEMA_VERSION = 6 and no migration 7. The migration history table is incorrect:

    | 7 | add_tool_id_index | Performance index for self-joins |
    

    This migration doesn't exist in storage.py.

  • docs/SCHEMA.md:160 - Documentation lists idx_events_tool_id index but this index does not exist in the codebase. Grep confirms it's only mentioned in the new docs file, not in storage.py. The actual index list should not include this.

Suggestions

  • docs/SCHEMA.md:54 - The events table documentation shows uuid TEXT UNIQUE NOT NULL but the actual schema uses a composite unique constraint: UNIQUE(session_id, uuid). The uuid alone is not unique - the same event UUID could theoretically appear in different sessions. Consider updating to accurately reflect the constraint.

  • docs/SCHEMA.md:135-160 - The index documentation could include the additional indexes created by migrations but not listed: idx_git_commits_timestamp, idx_git_commits_session, idx_git_commits_project, idx_session_commits_session, idx_session_commits_commit, and the bus_events indexes (idx_bus_events_timestamp, idx_bus_events_type, idx_bus_events_session, idx_bus_events_repo). Currently only events table indexes are listed.

Verdict

REQUEST_CHANGES - The documentation contains factual errors about the schema (non-existent migration 7 and idx_events_tool_id index) that would mislead developers and LLMs consulting these docs.


Automated review by Claude Code

@evansenter evansenter changed the base branch from main to perf-tool-id-index January 8, 2026 21:47
@claude

claude Bot commented Jan 8, 2026

Copy link
Copy Markdown

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

Code Review

Summary

This PR adds comprehensive database schema documentation to docs/SCHEMA.md and links it from CLAUDE.md and README.md. The documentation covers all 8 tables, indexes, FTS setup, migration history, and schema evolution guidelines.

Previous Review Correction

The previous review flagged migration 7 and idx_events_tool_id as non-existent. This was incorrect - the reviewer checked against main instead of the base branch perf-tool-id-index, which already contains:

  • SCHEMA_VERSION = 7
  • Migration 7 (add_tool_id_index)
  • The idx_events_tool_id index

Those issues are invalid - the documentation correctly describes the schema in the base branch.

Issues Found

Critical

None

Important

None

Suggestions

  • docs/SCHEMA.md:54 - The events table documentation shows uuid TEXT UNIQUE NOT NULL but the actual schema uses a composite unique constraint: UNIQUE(session_id, uuid). The uuid is NOT unique alone. Consider updating line 54 to accurately reflect this:

    uuid TEXT NOT NULL,  -- Unique within session (see UNIQUE constraint below)

    And add the constraint at the end of the CREATE TABLE statement.

  • docs/SCHEMA.md:135-160 - The index documentation only lists events table indexes. Several other indexes exist in the codebase but are not documented:

    • idx_git_commits_timestamp, idx_git_commits_session, idx_git_commits_project
    • idx_session_commits_session, idx_session_commits_commit
    • idx_bus_events_timestamp, idx_bus_events_type, idx_bus_events_session, idx_bus_events_repo

    Consider adding a complete index list or noting that only events indexes are documented.

Verdict

REQUEST_CHANGES - Two documentation accuracy suggestions that would improve correctness for LLMs and developers consulting these docs.


Automated review by Claude Code

evansenter and others added 2 commits January 8, 2026 21:50
- Create docs/SCHEMA.md with table definitions, indexes, and migration history
- Link from CLAUDE.md and README.md
- Documents design principles: don't over-distill, aggregate→drill-down
- Explains critical indexes like idx_events_tool_id for performance

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ndexes

- Fix events.uuid to show UNIQUE(session_id, uuid) composite constraint
- Add complete index list for git_commits, session_commits, bus_events tables

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Jan 8, 2026

Copy link
Copy Markdown

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

Code Review

Summary

This PR adds comprehensive database schema documentation to docs/SCHEMA.md covering all 8 tables, indexes, FTS setup, and migration history. It also adds a new test for the idx_events_tool_id index and links the docs from CLAUDE.md and README.md.

Issues Found

Critical

None

Important

None

Suggestions

  • docs/SCHEMA.md:54 - The events table documentation shows uuid TEXT UNIQUE NOT NULL but the actual schema in storage.py uses:

    uuid TEXT NOT NULL,
    ...
    UNIQUE(session_id, uuid)

    The uuid is unique only within a session (composite constraint), not globally unique. Consider updating line 54 to reflect this accurately.

  • docs/SCHEMA.md:135-160 - The index documentation only lists events table indexes. Several other indexes exist but are not documented:

    • idx_git_commits_timestamp, idx_git_commits_session, idx_git_commits_project
    • idx_session_commits_session, idx_session_commits_commit
    • idx_bus_events_timestamp, idx_bus_events_type, idx_bus_events_session, idx_bus_events_repo

    Consider adding a complete index list or noting that only key events indexes are documented for brevity.

Verdict

REQUEST_CHANGES - Two documentation accuracy suggestions that would improve correctness for LLMs and developers consulting these docs.


Automated review by Claude Code

@claude

claude Bot commented Jan 8, 2026

Copy link
Copy Markdown

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

Code Review

Summary

This PR adds comprehensive database schema documentation to docs/SCHEMA.md and links it from CLAUDE.md and README.md. The documentation covers design principles, all 8 tables with column definitions, complete index documentation, FTS5 setup, migration history, and schema evolution guidelines.

Previous Feedback Resolution

Both suggestions from the previous review have been addressed:

  • ✅ The uuid field now correctly shows TEXT NOT NULL with a comment explaining it's unique within session, plus the UNIQUE(session_id, uuid) composite constraint at the end
  • ✅ A new "Other Table Indexes" section now documents all indexes for git_commits, session_commits, and bus_events tables

Issues Found

Critical

None

Important

None

Suggestions

None

Verdict

APPROVE - Documentation is comprehensive, accurate, and addresses all previous feedback. The schema documentation will be valuable for both developers and LLMs working with this codebase.


Automated review by Claude Code

@evansenter evansenter deleted the branch perf-tool-id-index January 8, 2026 21:52
@evansenter evansenter closed this Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant