Add Support for Custom assistant_id in Create Assistant Endpoint #336
Conversation
WalkthroughAdds a migration to enforce composite uniqueness (project_id, assistant_id), updates models to match, modifies assistant creation to generate/check assistant_id and handle duplicates with 409, and introduces tests covering successful explicit IDs and duplicate ID conflicts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as API (create_assistant)
participant CRUD as CRUD Layer
participant DB as Database
Client->>API: POST /assistants (project_id, payload)
API->>CRUD: create_assistant(project_id, data)
Note right of CRUD: Ensure assistant_id: provided or UUID
CRUD->>DB: SELECT assistant WHERE project_id & assistant_id
alt Exists (duplicate)
DB-->>CRUD: Found
CRUD-->>API: Raise HTTP 409 Conflict
API-->>Client: 409 Duplicate assistant_id in project
else Not found
DB-->>CRUD: None
CRUD->>DB: INSERT assistant (project_id, assistant_id, ...)
DB-->>CRUD: Row inserted
CRUD-->>API: Assistant object
API-->>Client: 201 Created (assistant)
end
Note over DB: Enforced by unique constraint (project_id, assistant_id)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
backend/app/models/assistants.py (2)
4-6: Import UniqueConstraint from SQLAlchemy for better compatibilityDepending on your SQLModel version,
sqlmodel.UniqueConstraintmay not be exported. Importing it from SQLAlchemy is safer and consistent with the other SQLAlchemy imports in this module.Apply this diff:
-from sqlalchemy import Column, String, Text +from sqlalchemy import Column, String, Text, UniqueConstraint -from sqlmodel import Field, Relationship, SQLModel, UniqueConstraint +from sqlmodel import Field, Relationship, SQLModel
52-57: Normalize/validate client-provided assistant_id to avoid near-duplicatesAccepting an external assistant_id is great. To prevent subtle duplicates (e.g., trailing spaces), add a validator to trim whitespace and reject empty strings post-trim. Optionally, constrain the charset if you want to disallow spaces or special chars.
Example (outside the selected lines), using Pydantic v2 field validator:
from pydantic import field_validator class AssistantCreate(SQLModel): ... @field_validator("assistant_id") @classmethod def normalize_assistant_id(cls, v: str | None): if v is None: return v v = v.strip() if not v: raise ValueError("assistant_id cannot be empty or whitespace") # Optional: enforce a conservative charset # if not re.fullmatch(r"[A-Za-z0-9_\-.:/]+", v): # raise ValueError("assistant_id contains invalid characters") return vbackend/app/alembic/versions/38f0e8c8dc92_alter_unique_constraint_assistant_table.py (1)
9-11: Remove unused imports flagged by Ruff
sqlalchemy as saandsqlmodel.sql.sqltypesare not used in this migration.Apply this diff:
-from alembic import op -import sqlalchemy as sa -import sqlmodel.sql.sqltypes +from alembic import opbackend/app/tests/crud/test_assistants.py (1)
189-227: LGTM: duplicate assistant_id conflict is enforced with 409Great coverage of the duplicate path and the exact error shape. Consider adding a complementary test that demonstrates the same assistant_id can be reused across different projects (composite uniqueness semantics), to prevent regressions.
If helpful, I can draft a new test that:
- Creates Project A and Project B,
- Creates assistant with assistant_id="same_id" in A,
- Creates assistant with assistant_id="same_id" in B,
- Asserts both succeed (no 409).
Do you want me to scaffold that test given your project factory/setup?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
backend/app/alembic/versions/38f0e8c8dc92_alter_unique_constraint_assistant_table.py(1 hunks)backend/app/crud/assistants.py(1 hunks)backend/app/models/assistants.py(2 hunks)backend/app/tests/crud/test_assistants.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
backend/app/tests/crud/test_assistants.py (4)
backend/app/tests/conftest.py (1)
db(18-35)backend/app/tests/utils/utils.py (1)
get_project(70-89)backend/app/models/assistants.py (1)
AssistantCreate(47-76)backend/app/crud/assistants.py (1)
create_assistant(167-199)
backend/app/models/assistants.py (4)
backend/app/tests/api/routes/test_assistants.py (1)
assistant_id(25-26)backend/app/alembic/versions/4aa1f48c6321_add_inconistency_fixes.py (1)
upgrade(20-49)backend/app/alembic/versions/8eefcfedc409_create_assistant_table.py (1)
upgrade(20-40)backend/app/seed_data/seed_data.py (1)
AssistantData(56-65)
backend/app/alembic/versions/38f0e8c8dc92_alter_unique_constraint_assistant_table.py (2)
backend/app/alembic/versions/4aa1f48c6321_add_inconistency_fixes.py (1)
upgrade(20-49)backend/app/alembic/versions/8eefcfedc409_create_assistant_table.py (1)
upgrade(20-40)
backend/app/crud/assistants.py (3)
backend/app/utils.py (1)
mask_string(158-167)backend/app/seed_data/seed_data.py (1)
create_assistant(236-276)backend/app/api/routes/assistants.py (1)
create_assistant_route(52-70)
🪛 Ruff (0.12.2)
backend/app/alembic/versions/38f0e8c8dc92_alter_unique_constraint_assistant_table.py
9-9: sqlalchemy imported but unused
Remove unused import: sqlalchemy
(F401)
10-10: sqlmodel.sql.sqltypes imported but unused
Remove unused import: sqlmodel.sql.sqltypes
(F401)
⏰ 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)
- GitHub Check: checks (3.11.7, 6)
🔇 Additional comments (2)
backend/app/models/assistants.py (1)
12-16: Composite uniqueness per project looks correctMoving uniqueness to a composite constraint on (project_id, assistant_id) aligns with the PR goal and preserves per-project uniqueness. The column-level unique flag removal on assistant_id is consistent with the migration.
backend/app/tests/crud/test_assistants.py (1)
160-187: LGTM: covers explicit assistant_id creation pathThis test verifies the new explicit-ID path end-to-end, including persistence of the provided ID. Looks good.
backend/app/alembic/versions/38f0e8c8dc92_alter_unique_constraint_assistant_table.py
Show resolved
Hide resolved
* take assistant id as input in create assistant api
Summary
Target issue: #339
This PR updates the assistant table and related logic to support externally provided assistant_id values instead of always generating them internally.
Glific requires the ability to assign assistant_id values based on IDs returned from OpenAI for backward compatibility. This change ensures their workflow is supported without breaking existing logic.
Also ,enforced uniqueness of assistant_id within a project using a composite unique constraint (project_id, assistant_id).
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit