Skip to content

Conversation

@qdaxb
Copy link
Contributor

@qdaxb qdaxb commented Nov 20, 2025

Summary by CodeRabbit

  • Chores
    • Added database indexes to optimize query performance across multiple data models.
    • Introduced a verification script to confirm database indexes are properly configured.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 20, 2025

Walkthrough

This PR adds comprehensive database indexing across three models (Kind, SharedTeam, Subtask) by declaring column-level and composite table-level indexes. A new verification script validates that all indexes are properly created in the database.

Changes

Cohort / File(s) Summary
Model indexing: Kind
backend/app/models/kind.py
Added Index import. Added column-level indexes to user_id, is_active, and created_at. Introduced four composite indexes in __table_args__: idx_kind_user_kind, idx_kind_user_kind_active, idx_kind_user_name_namespace, idx_kind_user_kind_name_namespace.
Model indexing: SharedTeam
backend/app/models/shared_team.py
Added column-level index to is_active. Introduced two composite indexes in __table_args__: idx_user_active, idx_team_active.
Model indexing: Subtask
backend/app/models/subtask.py
Added Index import. Added column-level indexes to user_id, task_id, team_id, executor_namespace, executor_name, status, and created_at. Introduced five composite indexes in __table_args__: idx_subtask_task_user, idx_subtask_task_status, idx_subtask_task_role_status, idx_subtask_task_message, idx_subtask_executor.
Index verification
backend/scripts/verify_indexes.py
New script that verifies existence of database indexes across kinds, subtasks, and shared_teams tables using SQLAlchemy inspector. Includes verify_indexes() function with error handling and per-table reporting of found vs. expected indexes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Index additions follow consistent, repetitive patterns across files
  • Column-level and composite index definitions are straightforward declarations
  • New verification script is linear and self-contained

Areas requiring attention:

  • Verify composite index column ordering aligns with intended query patterns (e.g., user_id typically leads most composite indexes)
  • Confirm the verification script's expected index list matches the actual index definitions in models
  • Ensure no duplicate or conflicting indexes exist between column-level and composite declarations

Poem

🐰 Database hops now swiftly trace,
Through indexed rows at quickened pace,
With composite keys and lookups keen,
The fastest Kind model ever seen!
Subtasks and teams now glide like air,
When indexes nestle here and there.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'perf: add database index' accurately describes the main change—adding database indexes across multiple models and a verification script.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add_database_index

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (5)
backend/app/models/shared_team.py (1)

19-27: SharedTeam index set looks solid; verify redundancy vs. actual query patterns

Adding index=True on is_active plus idx_user_active(user_id, is_active) and idx_team_active(team_id, is_active) should help lookups of active shared teams by user or team. Since user_id, team_id, and is_active also have single-column indexes and there is already idx_user_team, it’s worth checking real query plans/slow logs to confirm each index is pulling its weight and trimming any that end up unused to avoid extra write and storage cost over time.

backend/app/models/subtask.py (1)

7-60: Subtask indexing strategy is reasonable; keep an eye on overlap and index bloat

Indexing user_id, task_id, team_id, executor fields, status, and created_at plus the composite subtasks indexes (idx_subtask_task_user, ..._task_status, ..._task_role_status, ..._task_message, idx_subtask_executor) lines up well with typical filtering patterns. It is, however, a dense set of overlapping indexes (e.g., task_id and status both indexed alone and together), so after this ships it’s worth checking EXPLAIN plans/slow-query logs to confirm which combinations are actually used and consider dropping any clearly redundant single-column indexes to keep insert/update cost and index size under control.

backend/scripts/verify_indexes.py (2)

19-103: Index verification logic is clear; consider documenting or extending coverage

The inspector-based checks for the named composite indexes on kinds, subtasks, and shared_teams are straightforward and the output is easy to read. Right now the script only asserts the presence of those explicitly named indexes; it doesn’t verify the single-column indexes created via index=True on the models (e.g., user_id, is_active, status, created_at). That’s fine if the intent is “sanity-check composite indexes only”, but it may be worth either (a) documenting this scope in the module docstring/help text or (b) later extending the script to also validate critical single-column indexes so schema drift in migrations gets caught early.


104-111: Broad Exception catch at entrypoint (BLE001) – either narrow or explicitly allow

Catching Exception in the __main__ block is a pragmatic way to ensure a clean non-zero exit with a traceback if anything goes wrong, but Ruff flags this as BLE001. If you want to keep the broad catch (which is reasonable for a CLI), consider adding an inline comment and # noqa: BLE001 to make the intent explicit, or alternatively narrow it to the concrete exceptions you expect from SQLAlchemy/DB connection failures so the linter stays happy without a suppression.

backend/app/models/kind.py (1)

8-32: Kind indexes are well targeted; monitor overlap across user/kind/name/namespace

Indexing user_id, is_active, and created_at plus the four composite indexes (idx_kind_user_kind, ..._user_kind_active, ..._user_name_namespace, ..._user_kind_name_namespace) should materially speed up the common queries over kinds by user, kind, name, namespace, and active status. As with the Subtask/SharedTeam models, this is a fairly rich index set with some overlap, so after deployment it’s worth watching write/query performance and pruning any indexes that turn out unused or redundant to keep the table lean.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1fa8e66 and 56733d9.

📒 Files selected for processing (4)
  • backend/app/models/kind.py (2 hunks)
  • backend/app/models/shared_team.py (1 hunks)
  • backend/app/models/subtask.py (2 hunks)
  • backend/scripts/verify_indexes.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/scripts/verify_indexes.py (3)
backend/app/models/kind.py (1)
  • Kind (14-34)
backend/app/models/subtask.py (1)
  • Subtask (26-60)
backend/app/models/shared_team.py (1)
  • SharedTeam (11-28)
🪛 Ruff (0.14.5)
backend/scripts/verify_indexes.py

107-107: Do not catch blind exception: Exception

(BLE001)

@qdaxb qdaxb closed this Nov 27, 2025
@qdaxb qdaxb deleted the add_database_index branch November 27, 2025 08:04
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.

2 participants