Alembic migration to make session_name required on messages#166
Conversation
WalkthroughThe changes enforce that every message must be associated with a session. This is achieved by updating the database schema, data models, and schemas to make the Changes
Sequence Diagram(s)sequenceDiagram
participant MigrationScript
participant DB
participant SessionsTable
participant SessionPeersTable
MigrationScript->>DB: Find messages with null session_name
loop For each workspace-peer with orphaned messages
MigrationScript->>SessionsTable: Insert default session (handle conflicts)
MigrationScript->>SessionPeersTable: Associate peer with new session (handle conflicts)
MigrationScript->>DB: Update orphaned messages and embeddings to new session
end
MigrationScript->>DB: Ensure no orphaned messages remain
MigrationScript->>DB: Alter messages and embeddings tables to make session_name non-nullable
MigrationScript->>DB: Update collection name length constraint to 1025
Estimated code review effort3 (~40 minutes) Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
📓 Path-based instructions (3)src/**/*.py📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
src/models.py📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
src/{models,crud}.py📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
🧠 Learnings (3)📓 Common learningssrc/models.py (8)Learnt from: CR Learnt from: CR Learnt from: Rajat-Ahuja1997 Learnt from: dr-frmr Learnt from: Rajat-Ahuja1997 Learnt from: CR Learnt from: Rajat-Ahuja1997 Learnt from: CR migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (10)Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: dr-frmr Learnt from: dr-frmr Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: VVoruganti 🪛 Ruff (0.12.2)migrations/versions/05486ce795d5_make_session_name_required_on_messages.py42-42: Possible SQL injection vector through string-based query construction (S608) 42-42: Trailing comma missing Add trailing comma (COM812) 50-54: Possible SQL injection vector through string-based query construction (S608) 54-54: Trailing comma missing Add trailing comma (COM812) 60-60: Trailing comma missing Add trailing comma (COM812) 68-68: Trailing comma missing Add trailing comma (COM812) 73-86: Possible SQL injection vector through string-based query construction (S608) 86-86: Trailing comma missing Add trailing comma (COM812) 91-103: Possible SQL injection vector through string-based query construction (S608) 103-103: Trailing comma missing Add trailing comma (COM812) 106-106: Trailing comma missing Add trailing comma (COM812) 111-117: Possible SQL injection vector through string-based query construction (S608) 117-117: Trailing comma missing Add trailing comma (COM812) 122-128: Possible SQL injection vector through string-based query construction (S608) 128-128: Trailing comma missing Add trailing comma (COM812) 133-133: Possible SQL injection vector through string-based query construction (S608) 133-133: Trailing comma missing Add trailing comma (COM812) 137-139: Create your own exception (TRY002) 137-139: Avoid specifying long messages outside the exception class (TRY003) 138-138: Trailing comma missing Add trailing comma (COM812) 148-148: Possible SQL injection vector through string-based query construction (S608) 148-148: Trailing comma missing Add trailing comma (COM812) 149-149: Trailing comma missing Add trailing comma (COM812) 153-155: Create your own exception (TRY002) 153-155: Avoid specifying long messages outside the exception class (TRY003) 154-154: Trailing comma missing Add trailing comma (COM812) 167-167: Trailing comma missing Add trailing comma (COM812) 179-179: Trailing comma missing Add trailing comma (COM812) 185-185: Trailing comma missing Add trailing comma (COM812) 🧰 Additional context used📓 Path-based instructions (3)src/**/*.py📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
src/models.py📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
src/{models,crud}.py📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
🧠 Learnings (3)📓 Common learningssrc/models.py (8)Learnt from: CR Learnt from: CR Learnt from: Rajat-Ahuja1997 Learnt from: dr-frmr Learnt from: Rajat-Ahuja1997 Learnt from: CR Learnt from: Rajat-Ahuja1997 Learnt from: CR migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (10)Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: dr-frmr Learnt from: dr-frmr Learnt from: Rajat-Ahuja1997 Learnt from: Rajat-Ahuja1997 Learnt from: VVoruganti 🪛 Ruff (0.12.2)migrations/versions/05486ce795d5_make_session_name_required_on_messages.py42-42: Possible SQL injection vector through string-based query construction (S608) 42-42: Trailing comma missing Add trailing comma (COM812) 50-54: Possible SQL injection vector through string-based query construction (S608) 54-54: Trailing comma missing Add trailing comma (COM812) 60-60: Trailing comma missing Add trailing comma (COM812) 68-68: Trailing comma missing Add trailing comma (COM812) 73-86: Possible SQL injection vector through string-based query construction (S608) 86-86: Trailing comma missing Add trailing comma (COM812) 91-103: Possible SQL injection vector through string-based query construction (S608) 103-103: Trailing comma missing Add trailing comma (COM812) 106-106: Trailing comma missing Add trailing comma (COM812) 111-117: Possible SQL injection vector through string-based query construction (S608) 117-117: Trailing comma missing Add trailing comma (COM812) 122-128: Possible SQL injection vector through string-based query construction (S608) 128-128: Trailing comma missing Add trailing comma (COM812) 133-133: Possible SQL injection vector through string-based query construction (S608) 133-133: Trailing comma missing Add trailing comma (COM812) 137-139: Create your own exception (TRY002) 137-139: Avoid specifying long messages outside the exception class (TRY003) 138-138: Trailing comma missing Add trailing comma (COM812) 148-148: Possible SQL injection vector through string-based query construction (S608) 148-148: Trailing comma missing Add trailing comma (COM812) 149-149: Trailing comma missing Add trailing comma (COM812) 153-155: Create your own exception (TRY002) 153-155: Avoid specifying long messages outside the exception class (TRY003) 154-154: Trailing comma missing Add trailing comma (COM812) 167-167: Trailing comma missing Add trailing comma (COM812) 179-179: Trailing comma missing Add trailing comma (COM812) 185-185: Trailing comma missing Add trailing comma (COM812) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py(1 hunks)src/models.py(1 hunks)src/schemas.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/**/*.py: Follow isort conventions with absolute imports preferred
snake_case for variables/functions; PascalCase for classes
Line length: 88 chars (Black compatible)
Explicit error handling with appropriate exception types
Docstrings: Use Google style docstrings
Use environment variables via python-dotenv (.env)
Use specific exception types (ResourceNotFoundException, ValidationException, etc.)
Proper logging with context instead of print statements
Files:
src/schemas.pysrc/models.py
src/models.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/models.py: Use explicit type hints with SQLAlchemy mapped_column annotations
All tables use text IDs (nanoid format) as primary keys
Composite foreign keys for multi-tenant relationships
Feature flags on workspace, peer, and session levels
JSONB metadata fields for extensibility
HNSW indexes for vector similarity search
Files:
src/models.py
src/{models,crud}.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
Token counting on messages for usage tracking
Files:
src/models.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file `migrations/versions/d429de0e5338_adopt_peer_paradigm.py`, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: dr-frmr
PR: plastic-labs/honcho#106
File: migrations/versions/a1b2c3d4e5f6_initial_schema.py:172-179
Timestamp: 2025-05-13T15:48:42.783Z
Learning: The `queue` and `active_queue_sessions` tables in the database schema reference `sessions.id` (numeric) rather than `sessions.public_id` (text) because they are internal-only tables not exposed through the API, where performance considerations outweigh consistent identifier usage.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#144
File: src/crud.py:1250-1250
Timestamp: 2025-06-26T18:39:54.942Z
Learning: Rajat-Ahuja1997 is comfortable exposing token counts in MessageCreate schema as a public property when it improves code maintainability and avoids type checking errors.
src/schemas.py (2)
Learnt from: dr-frmr
PR: #131
File: tests/test_schema_validations.py:76-80
Timestamp: 2025-06-17T21:40:26.817Z
Learning: In the MessageCreate schema, peer_name is the actual field name with "peer_id" as an alias, so when constructing MessageCreate(peer_id="value"), it sets the peer_name attribute to "value".
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.893Z
Learning: Applies to src/models.py : JSONB metadata fields for extensibility
src/models.py (3)
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.893Z
Learning: Applies to src/models.py : Use explicit type hints with SQLAlchemy mapped_column annotations
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.893Z
Learning: Applies to src/models.py : All tables use text IDs (nanoid format) as primary keys
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (4)
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file migrations/versions/d429de0e5338_adopt_peer_paradigm.py, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
🪛 Ruff (0.12.2)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py
39-39: Possible SQL injection vector through string-based query construction
(S608)
39-39: Trailing comma missing
Add trailing comma
(COM812)
47-51: Possible SQL injection vector through string-based query construction
(S608)
51-51: Trailing comma missing
Add trailing comma
(COM812)
57-57: Trailing comma missing
Add trailing comma
(COM812)
66-66: Trailing comma missing
Add trailing comma
(COM812)
71-83: Possible SQL injection vector through string-based query construction
(S608)
83-83: Trailing comma missing
Add trailing comma
(COM812)
88-99: Possible SQL injection vector through string-based query construction
(S608)
99-99: Trailing comma missing
Add trailing comma
(COM812)
102-102: Trailing comma missing
Add trailing comma
(COM812)
107-113: Possible SQL injection vector through string-based query construction
(S608)
113-113: Trailing comma missing
Add trailing comma
(COM812)
118-118: Possible SQL injection vector through string-based query construction
(S608)
118-118: Trailing comma missing
Add trailing comma
(COM812)
122-124: Create your own exception
(TRY002)
122-124: Avoid specifying long messages outside the exception class
(TRY003)
123-123: Trailing comma missing
Add trailing comma
(COM812)
🔇 Additional comments (1)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (1)
131-138: Downgrade implementation is appropriate.The downgrade function correctly makes the column nullable again while preserving the default sessions created during upgrade. The comment clearly documents this behavior.
| peer_name: str = Field(serialization_alias="peer_id") | ||
| # NOTE: Messages in Honcho 2.0 could historically be stored outside of a session. See models.py for more details. | ||
| session_name: str | None = Field(serialization_alias="session_id") | ||
| session_name: str = Field(serialization_alias="session_id") |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Schema change aligns with database constraints.
The change to make session_name required is consistent with the database migration and model changes. This enforces data integrity at the API level.
Since this is a breaking API change, please verify that API documentation has been updated to reflect this requirement:
🌐 Web query:
Does the Honcho API documentation mention that session_name/session_id is now a required field for Message creation?
💡 Result:
Based on the available Honcho API documentation, there is no explicit mention that session_name or session_id are required fields when creating a message. The documentation for creating a batch of messages for a session indicates that the session_id is a required path parameter, but it does not specify the necessity of session_name for message creation. (docs.honcho.dev)
However, in the Honcho Python SDK, the create method for messages requires the session_id as a parameter, but not session_name. (github.com)
Given this, it appears that session_id is necessary for message creation, but session_name is not. For the most accurate and up-to-date information, I recommend consulting the latest Honcho API documentation or reaching out to Honcho's support channels.
Citations:
- 1: https://docs.honcho.dev/api-reference/endpoint/messages/create-batch-messages-for-session?utm_source=openai
- 2: https://github.com/plastic-labs/honcho-python/blob/main/api.md?utm_source=openai
Update API documentation to reflect required session_name
The Honcho API docs and Python SDK currently don’t state that session_name (aliased as session_id) is now a required field when creating messages. Please update:
- The API reference for Create Batch Messages for Session to note that
session_id(a.k.a.session_name) is mandatory in the request body. - The Honcho Python SDK documentation (
createmethod for messages) to includesession_name/session_idas a required parameter.
🤖 Prompt for AI Agents
In src/schemas.py at line 145, the field session_name is marked with a
serialization alias session_id and is required. Update the API documentation for
the Create Batch Messages for Session endpoint to explicitly state that
session_id (session_name) is a mandatory field in the request body.
Additionally, update the Honcho Python SDK documentation for the create method
related to messages to include session_name/session_id as a required parameter,
ensuring both docs clearly reflect this requirement.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/models.py (1)
180-182: Fix type annotation to match non-nullable constraint.The type annotation should be
strinstead ofstr | Nonesincenullable=False.migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (3)
62-84: Handle unique constraint conflicts on default session names.The insertion could fail if a session with the same name already exists for the workspace.
134-136: Use a specific exception type instead of generic Exception.Define and use a migration-specific exception for better error handling.
150-152: Use a specific exception type for consistency.Similar to the earlier exception, use a specific exception type here as well.
Define a custom exception at the module level and use it for both error cases:
+class OrphanedRecordsError(Exception): + """Raised when orphaned records remain after migration.""" + pass + def upgrade() -> None: ... if remaining_orphaned_embeddings and remaining_orphaned_embeddings > 0: - raise Exception( + raise OrphanedRecordsError( f"Still have {remaining_orphaned_embeddings} orphaned message embeddings after migration" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py(1 hunks)src/models.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/**/*.py: Follow isort conventions with absolute imports preferred
snake_case for variables/functions; PascalCase for classes
Line length: 88 chars (Black compatible)
Explicit error handling with appropriate exception types
Docstrings: Use Google style docstrings
Use environment variables via python-dotenv (.env)
Use specific exception types (ResourceNotFoundException, ValidationException, etc.)
Proper logging with context instead of print statements
Files:
src/models.py
src/models.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/models.py: Use explicit type hints with SQLAlchemy mapped_column annotations
All tables use text IDs (nanoid format) as primary keys
Composite foreign keys for multi-tenant relationships
Feature flags on workspace, peer, and session levels
JSONB metadata fields for extensibility
HNSW indexes for vector similarity search
Files:
src/models.py
src/{models,crud}.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
Token counting on messages for usage tracking
Files:
src/models.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file `migrations/versions/d429de0e5338_adopt_peer_paradigm.py`, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: dr-frmr
PR: plastic-labs/honcho#106
File: migrations/versions/a1b2c3d4e5f6_initial_schema.py:172-179
Timestamp: 2025-05-13T15:48:42.783Z
Learning: The `queue` and `active_queue_sessions` tables in the database schema reference `sessions.id` (numeric) rather than `sessions.public_id` (text) because they are internal-only tables not exposed through the API, where performance considerations outweigh consistent identifier usage.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#144
File: src/crud.py:1250-1250
Timestamp: 2025-06-26T18:39:54.942Z
Learning: Rajat-Ahuja1997 is comfortable exposing token counts in MessageCreate schema as a public property when it improves code maintainability and avoids type checking errors.
src/models.py (4)
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.893Z
Learning: Applies to src/models.py : All tables use text IDs (nanoid format) as primary keys
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.893Z
Learning: Applies to src/models.py : Use explicit type hints with SQLAlchemy mapped_column annotations
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.893Z
Learning: Applies to src/models.py : Feature flags on workspace, peer, and session levels
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (10)
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file migrations/versions/d429de0e5338_adopt_peer_paradigm.py, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: Rajat-Ahuja1997
PR: #166
File: migrations/versions/05486ce795d5_make_session_name_required_on_messages.py:60-114
Timestamp: 2025-07-21T20:57:42.096Z
Learning: Peer names in the Honcho codebase are restricted to the pattern r"^[a-zA-Z0-9_-]+$" (same as session names), which prevents SQL injection and JSON syntax errors when peer names are interpolated into SQL queries and JSON strings during migrations.
Learnt from: dr-frmr
PR: #131
File: tests/test_schema_validations.py:76-80
Timestamp: 2025-06-17T21:40:26.817Z
Learning: In the MessageCreate schema, peer_name is the actual field name with "peer_id" as an alias, so when constructing MessageCreate(peer_id="value"), it sets the peer_name attribute to "value".
Learnt from: dr-frmr
PR: #131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The get_or_create_session function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.893Z
Learning: Applies to src/**/*.py : Explicit error handling with appropriate exception types
Learnt from: Rajat-Ahuja1997
PR: #131
File: src/crud.py:503-505
Timestamp: 2025-06-18T14:50:59.967Z
Learning: The Honcho project prefers upsert behavior for update operations across all resources (sessions, peers, workspaces). Update operations should create the resource if it doesn't exist rather than failing fast. This is an explicit design decision that differs from typical REST semantics but provides a more forgiving API experience.
Learnt from: Rajat-Ahuja1997
PR: #144
File: src/crud.py:1250-1250
Timestamp: 2025-06-26T18:39:54.942Z
Learning: Rajat-Ahuja1997 is comfortable exposing token counts in MessageCreate schema as a public property when it improves code maintainability and avoids type checking errors.
🪛 Ruff (0.12.2)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py
40-40: Possible SQL injection vector through string-based query construction
(S608)
40-40: Trailing comma missing
Add trailing comma
(COM812)
48-52: Possible SQL injection vector through string-based query construction
(S608)
52-52: Trailing comma missing
Add trailing comma
(COM812)
58-58: Trailing comma missing
Add trailing comma
(COM812)
67-67: Trailing comma missing
Add trailing comma
(COM812)
72-84: Possible SQL injection vector through string-based query construction
(S608)
84-84: Trailing comma missing
Add trailing comma
(COM812)
89-100: Possible SQL injection vector through string-based query construction
(S608)
100-100: Trailing comma missing
Add trailing comma
(COM812)
103-103: Trailing comma missing
Add trailing comma
(COM812)
108-114: Possible SQL injection vector through string-based query construction
(S608)
114-114: Trailing comma missing
Add trailing comma
(COM812)
119-125: Possible SQL injection vector through string-based query construction
(S608)
125-125: Trailing comma missing
Add trailing comma
(COM812)
130-130: Possible SQL injection vector through string-based query construction
(S608)
130-130: Trailing comma missing
Add trailing comma
(COM812)
134-136: Create your own exception
(TRY002)
134-136: Avoid specifying long messages outside the exception class
(TRY003)
135-135: Trailing comma missing
Add trailing comma
(COM812)
145-145: Possible SQL injection vector through string-based query construction
(S608)
145-145: Trailing comma missing
Add trailing comma
(COM812)
146-146: Trailing comma missing
Add trailing comma
(COM812)
150-152: Create your own exception
(TRY002)
150-152: Avoid specifying long messages outside the exception class
(TRY003)
151-151: Trailing comma missing
Add trailing comma
(COM812)
🔇 Additional comments (2)
src/models.py (1)
247-247: LGTM! Type annotation correctly matches the non-nullable constraint.The
session_namefield is properly defined as required with matching type annotation and column constraint.migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (1)
159-169: LGTM! Downgrade appropriately reverses schema changes.The function correctly makes the columns nullable again and clearly documents that it doesn't attempt to restore the original data state.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (1)
135-137: Use specific exception types for better error handling.Generic exceptions make it difficult for callers to handle specific error cases.
Define a custom exception at the module level:
class MigrationError(Exception): """Raised when migration encounters an error.""" passThen use it:
- raise Exception( + raise MigrationError( f"Still have {remaining_orphaned} orphaned messages after migration" )- raise Exception( + raise MigrationError( f"Still have {remaining_orphaned_embeddings} orphaned message embeddings after migration" )Also applies to: 151-153
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file `migrations/versions/d429de0e5338_adopt_peer_paradigm.py`, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: dr-frmr
PR: plastic-labs/honcho#106
File: migrations/versions/a1b2c3d4e5f6_initial_schema.py:172-179
Timestamp: 2025-05-13T15:48:42.783Z
Learning: The `queue` and `active_queue_sessions` tables in the database schema reference `sessions.id` (numeric) rather than `sessions.public_id` (text) because they are internal-only tables not exposed through the API, where performance considerations outweigh consistent identifier usage.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#144
File: src/crud.py:1250-1250
Timestamp: 2025-06-26T18:39:54.942Z
Learning: Rajat-Ahuja1997 is comfortable exposing token counts in MessageCreate schema as a public property when it improves code maintainability and avoids type checking errors.
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (9)
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file migrations/versions/d429de0e5338_adopt_peer_paradigm.py, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: Rajat-Ahuja1997
PR: #166
File: migrations/versions/05486ce795d5_make_session_name_required_on_messages.py:60-114
Timestamp: 2025-07-21T20:57:42.096Z
Learning: Peer names in the Honcho codebase are restricted to the pattern r"^[a-zA-Z0-9_-]+$" (same as session names), which prevents SQL injection and JSON syntax errors when peer names are interpolated into SQL queries and JSON strings during migrations.
Learnt from: dr-frmr
PR: #131
File: tests/test_schema_validations.py:76-80
Timestamp: 2025-06-17T21:40:26.817Z
Learning: In the MessageCreate schema, peer_name is the actual field name with "peer_id" as an alias, so when constructing MessageCreate(peer_id="value"), it sets the peer_name attribute to "value".
Learnt from: dr-frmr
PR: #131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The get_or_create_session function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.
Learnt from: Rajat-Ahuja1997
PR: #131
File: src/crud.py:503-505
Timestamp: 2025-06-18T14:50:59.967Z
Learning: The Honcho project prefers upsert behavior for update operations across all resources (sessions, peers, workspaces). Update operations should create the resource if it doesn't exist rather than failing fast. This is an explicit design decision that differs from typical REST semantics but provides a more forgiving API experience.
Learnt from: Rajat-Ahuja1997
PR: #144
File: src/crud.py:1250-1250
Timestamp: 2025-06-26T18:39:54.942Z
Learning: Rajat-Ahuja1997 is comfortable exposing token counts in MessageCreate schema as a public property when it improves code maintainability and avoids type checking errors.
🪛 Ruff (0.12.2)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py
40-40: Possible SQL injection vector through string-based query construction
(S608)
40-40: Trailing comma missing
Add trailing comma
(COM812)
48-52: Possible SQL injection vector through string-based query construction
(S608)
52-52: Trailing comma missing
Add trailing comma
(COM812)
58-58: Trailing comma missing
Add trailing comma
(COM812)
66-66: Trailing comma missing
Add trailing comma
(COM812)
71-84: Possible SQL injection vector through string-based query construction
(S608)
84-84: Trailing comma missing
Add trailing comma
(COM812)
89-101: Possible SQL injection vector through string-based query construction
(S608)
101-101: Trailing comma missing
Add trailing comma
(COM812)
104-104: Trailing comma missing
Add trailing comma
(COM812)
109-115: Possible SQL injection vector through string-based query construction
(S608)
115-115: Trailing comma missing
Add trailing comma
(COM812)
120-126: Possible SQL injection vector through string-based query construction
(S608)
126-126: Trailing comma missing
Add trailing comma
(COM812)
131-131: Possible SQL injection vector through string-based query construction
(S608)
131-131: Trailing comma missing
Add trailing comma
(COM812)
135-137: Create your own exception
(TRY002)
135-137: Avoid specifying long messages outside the exception class
(TRY003)
136-136: Trailing comma missing
Add trailing comma
(COM812)
146-146: Possible SQL injection vector through string-based query construction
(S608)
146-146: Trailing comma missing
Add trailing comma
(COM812)
147-147: Trailing comma missing
Add trailing comma
(COM812)
151-153: Create your own exception
(TRY002)
151-153: Avoid specifying long messages outside the exception class
(TRY003)
152-152: Trailing comma missing
Add trailing comma
(COM812)
🔇 Additional comments (4)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (4)
1-24: Clean migration setup with appropriate imports.The migration header and imports follow Alembic conventions correctly.
26-44: Effective orphaned message detection.The logic correctly identifies messages without sessions and provides helpful logging.
87-128: Comprehensive handling of session associations and message updates.The migration correctly:
- Creates session-peer associations with conflict handling
- Updates both messages and message_embeddings tables
- Uses precise WHERE clauses to target only orphaned records
160-171: Appropriate downgrade implementation.The downgrade correctly reverses the schema changes while preserving data integrity. The comments clearly document that default sessions and message assignments are retained.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (2)
81-81: Fix invalid JSON syntax in internal_metadata.The JSON key contains a space which makes it invalid. Keys in JSON cannot contain spaces without being escaped.
Apply this fix:
- '{{"migration note" : "Default session created for orphaned messages from peer {peer_name}"}}', + '{{"migration_note": "Default session created for orphaned messages from peer {peer_name}"}}',
137-139: Use a specific exception type instead of generic Exception.Using a generic
Exceptionmakes error handling more difficult for callers.Define and use a specific exception:
+class MigrationError(Exception): + """Raised when migration encounters an error.""" + pass + def upgrade() -> None: ... if remaining_orphaned and remaining_orphaned > 0: - raise Exception( + raise MigrationError( f"Still have {remaining_orphaned} orphaned messages after migration" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py(1 hunks)src/models.py(3 hunks)
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/**/*.py: Follow isort conventions with absolute imports preferred
snake_case for variables/functions; PascalCase for classes
Line length: 88 chars (Black compatible)
Explicit error handling with appropriate exception types
Docstrings: Use Google style docstrings
Use environment variables via python-dotenv (.env)
Use specific exception types (ResourceNotFoundException, ValidationException, etc.)
Proper logging with context instead of print statements
Files:
src/models.py
src/models.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/models.py: Use explicit type hints with SQLAlchemy mapped_column annotations
All tables use text IDs (nanoid format) as primary keys
Composite foreign keys for multi-tenant relationships
Feature flags on workspace, peer, and session levels
JSONB metadata fields for extensibility
HNSW indexes for vector similarity search
Files:
src/models.py
src/{models,crud}.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
Token counting on messages for usage tracking
Files:
src/models.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file `migrations/versions/d429de0e5338_adopt_peer_paradigm.py`, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: dr-frmr
PR: plastic-labs/honcho#106
File: migrations/versions/a1b2c3d4e5f6_initial_schema.py:172-179
Timestamp: 2025-05-13T15:48:42.783Z
Learning: The `queue` and `active_queue_sessions` tables in the database schema reference `sessions.id` (numeric) rather than `sessions.public_id` (text) because they are internal-only tables not exposed through the API, where performance considerations outweigh consistent identifier usage.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#144
File: src/crud.py:1250-1250
Timestamp: 2025-06-26T18:39:54.942Z
Learning: Rajat-Ahuja1997 is comfortable exposing token counts in MessageCreate schema as a public property when it improves code maintainability and avoids type checking errors.
src/models.py (7)
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : All tables use text IDs (nanoid format) as primary keys
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : Use explicit type hints with SQLAlchemy mapped_column annotations
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : Feature flags on workspace, peer, and session levels
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: Rajat-Ahuja1997
PR: #166
File: migrations/versions/05486ce795d5_make_session_name_required_on_messages.py:60-114
Timestamp: 2025-07-21T20:57:42.135Z
Learning: Peer names in the Honcho codebase are restricted to the pattern r"^[a-zA-Z0-9_-]+$" (same as session names), which prevents SQL injection and JSON syntax errors when peer names are interpolated into SQL queries and JSON strings during migrations.
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : Composite foreign keys for multi-tenant relationships
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (10)
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file migrations/versions/d429de0e5338_adopt_peer_paradigm.py, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: Rajat-Ahuja1997
PR: #166
File: migrations/versions/05486ce795d5_make_session_name_required_on_messages.py:60-114
Timestamp: 2025-07-21T20:57:42.135Z
Learning: Peer names in the Honcho codebase are restricted to the pattern r"^[a-zA-Z0-9_-]+$" (same as session names), which prevents SQL injection and JSON syntax errors when peer names are interpolated into SQL queries and JSON strings during migrations.
Learnt from: dr-frmr
PR: #131
File: tests/test_schema_validations.py:76-80
Timestamp: 2025-06-17T21:40:26.817Z
Learning: In the MessageCreate schema, peer_name is the actual field name with "peer_id" as an alias, so when constructing MessageCreate(peer_id="value"), it sets the peer_name attribute to "value".
Learnt from: dr-frmr
PR: #131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The get_or_create_session function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/**/*.py : Explicit error handling with appropriate exception types
Learnt from: Rajat-Ahuja1997
PR: #131
File: src/crud.py:503-505
Timestamp: 2025-06-18T14:50:59.967Z
Learning: The Honcho project prefers upsert behavior for update operations across all resources (sessions, peers, workspaces). Update operations should create the resource if it doesn't exist rather than failing fast. This is an explicit design decision that differs from typical REST semantics but provides a more forgiving API experience.
Learnt from: Rajat-Ahuja1997
PR: #144
File: src/crud.py:1250-1250
Timestamp: 2025-06-26T18:39:54.942Z
Learning: Rajat-Ahuja1997 is comfortable exposing token counts in MessageCreate schema as a public property when it improves code maintainability and avoids type checking errors.
🪛 Ruff (0.12.2)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py
42-42: Possible SQL injection vector through string-based query construction
(S608)
42-42: Trailing comma missing
Add trailing comma
(COM812)
50-54: Possible SQL injection vector through string-based query construction
(S608)
54-54: Trailing comma missing
Add trailing comma
(COM812)
60-60: Trailing comma missing
Add trailing comma
(COM812)
68-68: Trailing comma missing
Add trailing comma
(COM812)
73-86: Possible SQL injection vector through string-based query construction
(S608)
86-86: Trailing comma missing
Add trailing comma
(COM812)
91-103: Possible SQL injection vector through string-based query construction
(S608)
103-103: Trailing comma missing
Add trailing comma
(COM812)
106-106: Trailing comma missing
Add trailing comma
(COM812)
111-117: Possible SQL injection vector through string-based query construction
(S608)
117-117: Trailing comma missing
Add trailing comma
(COM812)
122-128: Possible SQL injection vector through string-based query construction
(S608)
128-128: Trailing comma missing
Add trailing comma
(COM812)
133-133: Possible SQL injection vector through string-based query construction
(S608)
133-133: Trailing comma missing
Add trailing comma
(COM812)
137-139: Create your own exception
(TRY002)
137-139: Avoid specifying long messages outside the exception class
(TRY003)
138-138: Trailing comma missing
Add trailing comma
(COM812)
148-148: Possible SQL injection vector through string-based query construction
(S608)
148-148: Trailing comma missing
Add trailing comma
(COM812)
149-149: Trailing comma missing
Add trailing comma
(COM812)
153-155: Create your own exception
(TRY002)
153-155: Avoid specifying long messages outside the exception class
(TRY003)
154-154: Trailing comma missing
Add trailing comma
(COM812)
167-167: Trailing comma missing
Add trailing comma
(COM812)
179-179: Trailing comma missing
Add trailing comma
(COM812)
185-185: Trailing comma missing
Add trailing comma
(COM812)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/**/*.py: Follow isort conventions with absolute imports preferred
snake_case for variables/functions; PascalCase for classes
Line length: 88 chars (Black compatible)
Explicit error handling with appropriate exception types
Docstrings: Use Google style docstrings
Use environment variables via python-dotenv (.env)
Use specific exception types (ResourceNotFoundException, ValidationException, etc.)
Proper logging with context instead of print statements
Files:
src/models.py
src/models.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/models.py: Use explicit type hints with SQLAlchemy mapped_column annotations
All tables use text IDs (nanoid format) as primary keys
Composite foreign keys for multi-tenant relationships
Feature flags on workspace, peer, and session levels
JSONB metadata fields for extensibility
HNSW indexes for vector similarity search
Files:
src/models.py
src/{models,crud}.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
Token counting on messages for usage tracking
Files:
src/models.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file `migrations/versions/d429de0e5338_adopt_peer_paradigm.py`, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: dr-frmr
PR: plastic-labs/honcho#106
File: migrations/versions/a1b2c3d4e5f6_initial_schema.py:172-179
Timestamp: 2025-05-13T15:48:42.783Z
Learning: The `queue` and `active_queue_sessions` tables in the database schema reference `sessions.id` (numeric) rather than `sessions.public_id` (text) because they are internal-only tables not exposed through the API, where performance considerations outweigh consistent identifier usage.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#144
File: src/crud.py:1250-1250
Timestamp: 2025-06-26T18:39:54.942Z
Learning: Rajat-Ahuja1997 is comfortable exposing token counts in MessageCreate schema as a public property when it improves code maintainability and avoids type checking errors.
src/models.py (7)
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : All tables use text IDs (nanoid format) as primary keys
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : Use explicit type hints with SQLAlchemy mapped_column annotations
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : Feature flags on workspace, peer, and session levels
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: Rajat-Ahuja1997
PR: #166
File: migrations/versions/05486ce795d5_make_session_name_required_on_messages.py:60-114
Timestamp: 2025-07-21T20:57:42.135Z
Learning: Peer names in the Honcho codebase are restricted to the pattern r"^[a-zA-Z0-9_-]+$" (same as session names), which prevents SQL injection and JSON syntax errors when peer names are interpolated into SQL queries and JSON strings during migrations.
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : Composite foreign keys for multi-tenant relationships
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (10)
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script migrations/versions/d429de0e5338_adopt_peer_paradigm.py are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file migrations/versions/d429de0e5338_adopt_peer_paradigm.py, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.
Learnt from: Rajat-Ahuja1997
PR: #131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:438-441
Timestamp: 2025-06-18T16:44:57.733Z
Learning: In the Honcho database schema, messages.app_id references workspaces.public_id (string nanoid), not workspaces.id (integer primary key). This means that during migrations when workspaces.public_id is renamed to workspaces.id, the foreign key relationship with messages.app_id remains valid.
Learnt from: Rajat-Ahuja1997
PR: #166
File: migrations/versions/05486ce795d5_make_session_name_required_on_messages.py:60-114
Timestamp: 2025-07-21T20:57:42.135Z
Learning: Peer names in the Honcho codebase are restricted to the pattern r"^[a-zA-Z0-9_-]+$" (same as session names), which prevents SQL injection and JSON syntax errors when peer names are interpolated into SQL queries and JSON strings during migrations.
Learnt from: dr-frmr
PR: #131
File: tests/test_schema_validations.py:76-80
Timestamp: 2025-06-17T21:40:26.817Z
Learning: In the MessageCreate schema, peer_name is the actual field name with "peer_id" as an alias, so when constructing MessageCreate(peer_id="value"), it sets the peer_name attribute to "value".
Learnt from: dr-frmr
PR: #131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The get_or_create_session function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/**/*.py : Explicit error handling with appropriate exception types
Learnt from: Rajat-Ahuja1997
PR: #131
File: src/crud.py:503-505
Timestamp: 2025-06-18T14:50:59.967Z
Learning: The Honcho project prefers upsert behavior for update operations across all resources (sessions, peers, workspaces). Update operations should create the resource if it doesn't exist rather than failing fast. This is an explicit design decision that differs from typical REST semantics but provides a more forgiving API experience.
Learnt from: Rajat-Ahuja1997
PR: #144
File: src/crud.py:1250-1250
Timestamp: 2025-06-26T18:39:54.942Z
Learning: Rajat-Ahuja1997 is comfortable exposing token counts in MessageCreate schema as a public property when it improves code maintainability and avoids type checking errors.
🪛 Ruff (0.12.2)
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py
42-42: Possible SQL injection vector through string-based query construction
(S608)
42-42: Trailing comma missing
Add trailing comma
(COM812)
50-54: Possible SQL injection vector through string-based query construction
(S608)
54-54: Trailing comma missing
Add trailing comma
(COM812)
60-60: Trailing comma missing
Add trailing comma
(COM812)
68-68: Trailing comma missing
Add trailing comma
(COM812)
73-86: Possible SQL injection vector through string-based query construction
(S608)
86-86: Trailing comma missing
Add trailing comma
(COM812)
91-103: Possible SQL injection vector through string-based query construction
(S608)
103-103: Trailing comma missing
Add trailing comma
(COM812)
106-106: Trailing comma missing
Add trailing comma
(COM812)
111-117: Possible SQL injection vector through string-based query construction
(S608)
117-117: Trailing comma missing
Add trailing comma
(COM812)
122-128: Possible SQL injection vector through string-based query construction
(S608)
128-128: Trailing comma missing
Add trailing comma
(COM812)
133-133: Possible SQL injection vector through string-based query construction
(S608)
133-133: Trailing comma missing
Add trailing comma
(COM812)
137-139: Create your own exception
(TRY002)
137-139: Avoid specifying long messages outside the exception class
(TRY003)
138-138: Trailing comma missing
Add trailing comma
(COM812)
148-148: Possible SQL injection vector through string-based query construction
(S608)
148-148: Trailing comma missing
Add trailing comma
(COM812)
149-149: Trailing comma missing
Add trailing comma
(COM812)
153-155: Create your own exception
(TRY002)
153-155: Avoid specifying long messages outside the exception class
(TRY003)
154-154: Trailing comma missing
Add trailing comma
(COM812)
167-167: Trailing comma missing
Add trailing comma
(COM812)
179-179: Trailing comma missing
Add trailing comma
(COM812)
185-185: Trailing comma missing
Add trailing comma
(COM812)
🔇 Additional comments (5)
src/models.py (3)
181-182: Comment accurately reflects the migration.The updated comment properly documents that orphaned messages have been assigned to default sessions.
247-247: Consistent enforcement of non-nullable session_name.Making
session_namenon-nullable inMessageEmbeddingaligns with the same change in theMessagemodel.
305-305: Increased collection name length limit.The change from 512 to 1024 characters provides more flexibility for collection names.
migrations/versions/05486ce795d5_make_session_name_required_on_messages.py (2)
72-86: Good handling of potential conflicts.The use of
ON CONFLICT DO NOTHINGproperly handles cases where default sessions or session-peer associations already exist, making the migration idempotent.Also applies to: 91-103
164-168: Proper constraint management.Using
constraint_existsbefore dropping constraints prevents errors if the constraint doesn't exist.Also applies to: 182-186
| CheckConstraint("length(id) = 21", name="id_length"), | ||
| CheckConstraint("id ~ '^[A-Za-z0-9_-]+$'", name="id_format"), | ||
| CheckConstraint("length(name) <= 512", name="name_length"), | ||
| CheckConstraint("length(name) <= 1024", name="name_length"), |
There was a problem hiding this comment.
currently, if two peers (peer1, peer2) have names where len(peer1.name) + len(peer2.name) > 512, the collection will fail to build. the collection name length should be the max length of two peer names + 1 for some character to divide them, e.g. peer1.name_peer2.name
There was a problem hiding this comment.
Should be 1025 then right?
| CheckConstraint("length(id) = 21", name="id_length"), | ||
| CheckConstraint("id ~ '^[A-Za-z0-9_-]+$'", name="id_format"), | ||
| CheckConstraint("length(name) <= 512", name="name_length"), | ||
| CheckConstraint("length(name) <= 1024", name="name_length"), |
There was a problem hiding this comment.
Should be 1025 then right?
…labs#166) * model changes * add migration * fix migration note * handle MessageEmbedding table * fix json issue and make script idempotent * add collections name_length change * make 1025 * coderabbit
Tested locally:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation