-
Notifications
You must be signed in to change notification settings - Fork 5
refactor: rename integrations to deployments across API and UI #833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis PR renames "integration" concepts to "deployment" across the codebase: route names/paths and redirects, tab identifiers, path params and payload fields (integration_id/integration_name → deployment_id/deployment_name), serialized labels ("Integration Name" → "Deployment Name"), UI assets/constants (integrations_img → deployments_img), public helper method names (e.g., BotIntegration.api_integration_id → api_deployment_id), streaming API endpoints (/v3/integrations/... → /v3/deployments/...) with backward-compatible redirects, and updates tests and example widgets. It also adds a Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
fc87cda to
520af3a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bots/models/bot_integration.py (1)
518-572: Fix the typo in the new deployment helper name.The renamed helper is now spelled
api_deploymet_id(). Shipping the typo locks in a misspelled public API and forces every caller (including external users) to carry the mistake forward. Please spell it correctly asapi_deployment_id()here and update all call sites in this PR before we merge so the deployment terminology change is coherent.Apply this diff to start the rename in this file:
- and f"Deployment ID {self.api_deploymet_id()}" + and f"Deployment ID {self.api_deployment_id()}" ... - deployment_id=self.api_deploymet_id(), + deployment_id=self.api_deployment_id(), ... - def api_deploymet_id(self) -> str: + def api_deployment_id(self) -> str: from routers.bots_api import api_hashids return api_hashids.encode(self.id)(Be sure to update the corresponding references across the codebase after renaming the method.)
routers/root.py (1)
458-506: Restore exported symbolintegrations_stats_routefor compatibility.The test pipeline is failing with
ImportError: cannot import name 'integrations_stats_route' from routers.root. Other modules still import the old name, so removing it breaks the build. Please provide a legacy alias alongside the new function until all call sites are migrated.def deployments_route(...): ... return render_recipe_page(request, page_slug, RecipeTabs.deployments, example_id) +# Backward compatibility for modules still importing the old name +integrations_stats_route = deployments_stats_route
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
bots/admin.py(2 hunks)bots/models/bot_integration.py(4 hunks)bots/models/convo_msg.py(8 hunks)daras_ai_v2/api_examples_widget.py(3 hunks)daras_ai_v2/bot_integration_connect.py(1 hunks)daras_ai_v2/bot_integration_widgets.py(3 hunks)daras_ai_v2/bots.py(2 hunks)daras_ai_v2/breadcrumbs.py(1 hunks)daras_ai_v2/icons.py(1 hunks)daras_ai_v2/meta_content.py(3 hunks)daras_ai_v2/variables_widget.py(1 hunks)functions/models.py(2 hunks)recipes/VideoBots.py(7 hunks)recipes/VideoBotsStats.py(4 hunks)routers/bots_api.py(9 hunks)routers/root.py(5 hunks)routers/twilio_ws_api.py(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Python tests
routers/root.py
[error] 1-1: ImportError: cannot import name 'integrations_stats_route' from routers.root
🪛 Ruff (0.14.3)
routers/bots_api.py
133-133: Unused function argument: request
(ARG001)
133-133: Unused function argument: response
(ARG001)
routers/root.py
441-441: Unused function argument: run_slug
(ARG001)
441-441: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
442-442: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
460-460: Unused function argument: page_slug
(ARG001)
462-462: Unused function argument: run_slug
(ARG001)
462-462: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
463-463: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
464-464: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
495-495: Unused function argument: run_slug
(ARG001)
495-495: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
496-496: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
497-497: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
505-505: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
535-535: Unused function argument: page_slug
(ARG001)
536-536: Unused function argument: run_slug
(ARG001)
536-536: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
537-537: Unused function argument: example_id
(ARG001)
537-537: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
538-538: Unused function argument: integration_id
(ARG001)
538-538: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: Unused function argument: deployment_name
(ARG001)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
597-597: Unused function argument: deployment_name
(ARG001)
597-597: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🔇 Additional comments (12)
daras_ai_v2/variables_widget.py (1)
215-220: LGTM! Clean implementation of the deprecated variable badge.The implementation follows the established pattern for displaying variable metadata badges and is consistent with the existing "System provided" and "Template variable" indicators. The truthiness check for
deprecatedprovides flexibility to support various value types (boolean, string, timestamp, etc.).Since the AI summary mentions a
VariableSchemaTypedDict that includes the newdeprecatedfield, verify that this type definition exists and properly includes the field:functions/models.py (2)
59-59: LGTM!The icon reference update aligns with the deployment terminology migration.
160-160: LGTM!The
deprecatedfield addition supports marking deprecated variables in the UI, which aligns with the backward compatibility strategy mentioned in the PR.daras_ai_v2/breadcrumbs.py (1)
99-99: Verify the singular form "Deployment" is consistent with UI conventions.The prefix changed from "Deployments" (plural) to "Deployment" (singular). Please confirm this aligns with how other tab labels are displayed throughout the UI.
daras_ai_v2/meta_content.py (3)
112-112: LGTM!The tab reference update is consistent with the deployment terminology migration.
197-197: LGTM!The tab reference update is consistent with the deployment terminology migration.
235-235: LGTM!The tab reference update is consistent with the deployment terminology migration.
recipes/VideoBotsStats.py (1)
726-737: Good backward compatibility approach.The strategy of maintaining deprecated
integration_idandintegration_namealiases while introducingdeployment_idanddeployment_nameensures existing integrations continue to work.bots/models/convo_msg.py (2)
98-98: LGTM!Column name updates from "Integration Name" to "Deployment Name" are consistent with the terminology migration.
Also applies to: 118-118
626-626: LGTM!Column name updates from "Integration Name" to "Deployment Name" are consistent with the terminology migration.
Also applies to: 642-642
daras_ai_v2/icons.py (1)
76-76: LGTM!The constant rename from
integrations_imgtodeployments_imgis consistent with the terminology migration.daras_ai_v2/bot_integration_connect.py (1)
31-31: LGTM!The tab reference update to
RecipeTabs.deploymentsis consistent with the terminology migration.
| @app.post( | ||
| "/v3/integrations/stream", | ||
| response_model=CreateStreamResponse, | ||
| responses={ | ||
| 402: {}, | ||
| 308: {"description": "Permanent redirect to /v3/deployments/stream"}, | ||
| }, | ||
| operation_id=VideoBotsPage.slug_versions[0] + "__stream_create_redirect", | ||
| tags=["Copilot Deployments"], | ||
| name="Copilot Deployments Create Stream (deprecated - redirects)", | ||
| deprecated=True, | ||
| include_in_schema=True, | ||
| ) | ||
| def stream_create_integrations(request: CreateStreamRequest, response: Response): | ||
| """Backward compatibility endpoint. Redirects to /v3/deployments/stream""" | ||
| redirect_url = str( | ||
| furl(settings.API_BASE_URL) / app.url_path_for(stream_create.__name__) | ||
| ) | ||
| return RedirectResponse(url=redirect_url, status_code=HTTP_308_PERMANENT_REDIRECT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep legacy POST semantics for /v3/integrations/stream.
Returning 308 Permanent Redirect here is a breaking change for existing clients. The old /v3/integrations/stream POST returned 200 with the JSON payload (including stream_url). Many HTTP clients do not automatically replay POST bodies across 308 redirects, so callers will now fail instead of seamlessly receiving the stream URL. Please continue issuing the original response on this deprecated route (while marking it deprecated in docs) by delegating to stream_create rather than redirecting.
@app.post(
"/v3/integrations/stream",
...
)
def stream_create_integrations(request: CreateStreamRequest, response: Response):
- """Backward compatibility endpoint. Redirects to /v3/deployments/stream"""
- redirect_url = str(
- furl(settings.API_BASE_URL) / app.url_path_for(stream_create.__name__)
- )
- return RedirectResponse(url=redirect_url, status_code=HTTP_308_PERMANENT_REDIRECT)
+ """Backward compatibility endpoint. Returns the same payload as /v3/deployments/stream."""
+ return stream_create(request, response)🧰 Tools
🪛 Ruff (0.14.3)
133-133: Unused function argument: request
(ARG001)
133-133: Unused function argument: response
(ARG001)
🤖 Prompt for AI Agents
In routers/bots_api.py around lines 120 to 138, the deprecated POST
/v3/integrations/stream currently returns an HTTP 308 redirect which breaks
clients that expect the original 200 JSON response; instead, call/delegate to
the existing stream_create handler so the endpoint returns the original response
body (including stream_url) while still marked deprecated in docs. Replace the
RedirectResponse logic with a direct call/return of stream_create(request,
response) (preserving any necessary parameters and response model) and keep
deprecated=True and include_in_schema=True so docs still show it as deprecated.
520af3a to
a973d07
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (5)
recipes/VideoBotsStats.py (1)
735-757: Mark legacy integration aliases as deprecatedWe're still sending the legacy
integration_id/integration_namekeys for compatibility, but because the deprecation flags remain commented out they continue to surface in the UI—undercutting the terminology refactor. Now thatVariableSchemasupportsdeprecated, please enable those flags so the old fields are hidden while the new deployment keys remain visible.Apply this diff:
- # mark deprecated variables so they're excluded from UI - # variables_schema["integration_id"] = {"role": "system", "deprecated": True} - # variables_schema["integration_name"] = {"role": "system", "deprecated": True} + # mark deprecated variables so they're excluded from UI + variables_schema["integration_id"] = {"role": "system", "deprecated": True} + variables_schema["integration_name"] = {"role": "system", "deprecated": True}bots/models/convo_msg.py (1)
333-338: Rename toapi_deployment_idand add a compatibility shimThe newly introduced method name drops the “n” (
api_deploymet_id). That typo now propagates across every caller and enshrines the wrong identifier in the public surface. Please rename the canonical method toapi_deployment_id, update call sites, and (if you need to keep the old spelling temporarily) add a thin wrapper that delegates to the correctly spelled version so nothing breaks while callers migrate.Apply this diff locally and update call sites accordingly:
- def api_deploymet_id(self) -> str: + def api_deployment_id(self) -> str: from routers.bots_api import api_hashids return api_hashids.encode(self.id) + + def api_deploymet_id(self) -> str: # deprecated alias + return self.api_deployment_id()daras_ai_v2/bots.py (1)
640-641: Fix typo in method name:api_deploymet_id→api_deployment_id.Same typo as noted elsewhere:
api_deploymet_id()should beapi_deployment_id(). This method name should be corrected throughout the codebase.recipes/VideoBots.py (1)
1814-1814: Fix typo in method name:api_deploymet_id→api_deployment_id.Same typo noted elsewhere: the method should be
api_deployment_id()notapi_deploymet_id().routers/bots_api.py (1)
120-138: Breaking change: POST redirect will fail for many clients.As noted in the previous review, returning a 308 redirect for the deprecated POST
/v3/integrations/streamendpoint is a breaking change. Many HTTP clients (including common libraries in Python, JavaScript, etc.) do not automatically replay POST request bodies across 308 redirects, causing existing integrations to fail.The original endpoint returned
200 OKwith a JSON payload containing thestream_url. To maintain backward compatibility, the deprecated endpoint should directly call the new handler and return the same response, not redirect.Apply this diff to fix the breaking change:
@app.post( "/v3/integrations/stream", ... ) def stream_create_integrations(request: CreateStreamRequest, response: Response): - """Backward compatibility endpoint. Redirects to /v3/deployments/stream""" - redirect_url = str( - furl(settings.API_BASE_URL) / app.url_path_for(stream_create.__name__) - ) - return RedirectResponse(url=redirect_url, status_code=HTTP_308_PERMANENT_REDIRECT) + """Backward compatibility endpoint. Returns the same payload as /v3/deployments/stream.""" + return stream_create(request, response)Note: The static analysis warnings about unused
requestandresponsearguments are false positives—these are required by FastAPI's dependency injection system.Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
bots/admin.py(2 hunks)bots/models/bot_integration.py(4 hunks)bots/models/convo_msg.py(8 hunks)daras_ai_v2/api_examples_widget.py(3 hunks)daras_ai_v2/bot_integration_connect.py(1 hunks)daras_ai_v2/bot_integration_widgets.py(3 hunks)daras_ai_v2/bots.py(2 hunks)daras_ai_v2/breadcrumbs.py(1 hunks)daras_ai_v2/icons.py(1 hunks)daras_ai_v2/meta_content.py(3 hunks)daras_ai_v2/variables_widget.py(1 hunks)functions/models.py(2 hunks)recipes/VideoBots.py(7 hunks)recipes/VideoBotsStats.py(4 hunks)routers/bots_api.py(9 hunks)routers/root.py(5 hunks)routers/twilio_ws_api.py(1 hunks)tests/test_public_endpoints.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- routers/twilio_ws_api.py
- daras_ai_v2/meta_content.py
- functions/models.py
- daras_ai_v2/bot_integration_widgets.py
- bots/models/bot_integration.py
- daras_ai_v2/variables_widget.py
- daras_ai_v2/bot_integration_connect.py
🧰 Additional context used
🧬 Code graph analysis (9)
daras_ai_v2/bots.py (2)
bots/models/bot_integration.py (2)
api_deploymet_id(560-563)Platform(28-75)bots/models/convo_msg.py (1)
api_deploymet_id(333-336)
tests/test_public_endpoints.py (3)
routers/root.py (2)
RecipeTabs(932-976)deployments_stats_route(458-479)bots/models/bot_integration.py (1)
api_deploymet_id(560-563)bots/models/convo_msg.py (1)
api_deploymet_id(333-336)
daras_ai_v2/breadcrumbs.py (1)
routers/root.py (1)
RecipeTabs(932-976)
recipes/VideoBotsStats.py (5)
routers/root.py (1)
RecipeTabs(932-976)bots/models/bot_integration.py (3)
api_deploymet_id(560-563)get_app_url(775-781)Platform(28-75)bots/models/convo_msg.py (1)
api_deploymet_id(333-336)bots/models/saved_run.py (1)
get_app_url(172-178)bots/models/published_run.py (1)
get_app_url(243-246)
bots/admin.py (5)
routers/root.py (1)
deployments_stats_route(458-479)bots/models/bot_integration.py (1)
api_deploymet_id(560-563)bots/models/convo_msg.py (1)
api_deploymet_id(333-336)bots/admin_links.py (1)
open_in_new_tab(13-26)daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)
routers/bots_api.py (1)
recipes/VideoBots.py (1)
VideoBotsPage(142-2126)
routers/root.py (3)
daras_ai_v2/bot_integration_widgets.py (1)
get_web_widget_embed_code(364-385)bots/models/bot_integration.py (1)
BotIntegration(163-626)routers/account.py (1)
TabData(246-248)
recipes/VideoBots.py (3)
routers/root.py (1)
RecipeTabs(932-976)daras_ai_v2/base.py (1)
current_app_url(213-230)bots/models/bot_integration.py (4)
BotIntegration(163-626)Platform(28-75)get_icon(36-51)api_deploymet_id(560-563)
bots/models/convo_msg.py (1)
bots/models/bot_integration.py (1)
api_deploymet_id(560-563)
🪛 Ruff (0.14.3)
routers/bots_api.py
133-133: Unused function argument: request
(ARG001)
133-133: Unused function argument: response
(ARG001)
routers/root.py
441-441: Unused function argument: run_slug
(ARG001)
441-441: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
442-442: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
460-460: Unused function argument: page_slug
(ARG001)
462-462: Unused function argument: run_slug
(ARG001)
462-462: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
463-463: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
464-464: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
495-495: Unused function argument: run_slug
(ARG001)
495-495: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
496-496: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
497-497: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
505-505: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
535-535: Unused function argument: page_slug
(ARG001)
536-536: Unused function argument: run_slug
(ARG001)
536-536: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
537-537: Unused function argument: example_id
(ARG001)
537-537: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
538-538: Unused function argument: integration_id
(ARG001)
538-538: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: Unused function argument: deployment_name
(ARG001)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
597-597: Unused function argument: deployment_name
(ARG001)
597-597: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
⏰ 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: test (3.10.12, 1.8.3)
🔇 Additional comments (8)
daras_ai_v2/api_examples_widget.py (3)
385-390: LGTM!The variable usage and UI label have been updated consistently to use
deployment_idinstead ofintegration_id. These changes align with the function signature update.
345-346: No action required—backend is already compatible.The backend at
routers/bots_api.pylines 40–42 already accepts thedeployment_idfield viaAliasChoices, which also maintains backward compatibility withintegration_id. The frontend change is properly supported.
333-333: No action required—the original review concern is incorrect.The function parameter rename from
integration_idtodeployment_idis fully compatible with the existing codebase. The only call site at line 549 indaras_ai_v2/bot_integration_widgets.pypasses the argument as a positional parameter (bot_api_example_generator(bi.api_deploymet_id())), which works regardless of parameter name. No keyword argument calls exist that would break.daras_ai_v2/bots.py (1)
642-693: Good backward compatibility implementation.The approach of maintaining deprecated aliases (
integration_id,integration_name) while introducing the newdeployment_idanddeployment_namevariables is well-designed. Marking deprecated variables in the schema withdeprecated: Trueensures they're hidden from the UI while still available for existing integrations. This provides a smooth migration path.recipes/VideoBots.py (1)
1788-1819: Approve the refactored deployment settings rendering.The refactoring from
integrations_maptodeployments_map(line 1801) and the updated parameter naming (deployments: list[BotIntegration]on line 1789) correctly align with the new terminology. The logic for handling multiple deployments with a selectbox is preserved and works correctly.routers/bots_api.py (3)
233-254: Approve GET redirect for backward compatibility.The 301 redirect for the GET endpoint
/v3/integrations/stream/{request_id}is acceptable because GET requests are idempotent and most HTTP clients correctly follow redirects for GET requests. This provides a clean migration path while maintaining backward compatibility.
38-43: Excellent use ofAliasChoicesfor backward compatibility.Using Pydantic's
validation_alias=AliasChoices("deployment_id", "integration_id")is the correct approach to accept both the newdeployment_idand deprecatedintegration_idfield names. This allows existing API clients to continue working while new clients adopt the updated terminology.
287-293: Verify error message clarity for end users.The error message on line 292 now reads
"Bot Deployment with id={request.deployment_id} not found". Sincedeployment_idcould actually contain anintegration_idvalue (due to the alias), this might be confusing for users still using the old field name. Consider updating the error message to acknowledge both field names for clarity.Example improvement:
raise HTTPException( status_code=404, - detail=f"Bot Deployment with id={request.deployment_id} not found", + detail=f"Bot Deployment with deployment_id={request.deployment_id} not found. (Note: integration_id is deprecated, use deployment_id instead.)", )
a973d07 to
83c031b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (10)
bots/models/convo_msg.py (3)
296-296: Critical: Typo in method name persists.The method call
api_deploymet_id()is misspelled (missing 'n' in "deployment"). This issue was flagged in previous reviews but remains unfixed.
333-336: Critical: Method name typo remains unfixed.The method
api_deploymet_id()is still misspelled (should beapi_deployment_id()). This was flagged in previous reviews but hasn't been corrected. The same typo exists inbots/models/bot_integration.pyat line 559 (shown in relevant code snippets).
437-438: Typo in method call; key name is correct.Line 437 calls the misspelled
api_deploymet_id()method (flagged in previous reviews). However, line 438 correctly usesdeployment_namefor the key.recipes/VideoBotsStats.py (2)
116-121: Restore deployment redirect by usingdeployment_id.The fallback redirect still sends
integration_idinpath_params, so the generated URL drops the identifier and lands users on/deployments/stats/(404). Switch the key todeployment_idso legacy?bi_id=links resolve correctly.- path_params=dict(integration_id=bi.api_deploymet_id()), + path_params=dict(deployment_id=bi.api_deploymet_id()),
755-757: Actually mark the legacy aliases as deprecated.The UI now renders a “Deprecated” badge, but these aliases never get the flag set, so the badge never appears and the old names keep showing as healthy system vars. Tag them here so the new deprecation signal reaches the widget.
- # mark deprecated variables so they're excluded from UI - # variables_schema["integration_id"] = {"role": "system", "deprecated": True} - # variables_schema["integration_name"] = {"role": "system", "deprecated": True} + # mark deprecated aliases so they stay available without cluttering the UI + if "integration_id" in variables_schema: + variables_schema["integration_id"]["deprecated"] = True + if "integration_name" in variables_schema: + variables_schema["integration_name"]["deprecated"] = Truerecipes/VideoBots.py (1)
1607-1608: Duplicate: Method naming inconsistency already flagged.This issue was already identified in a previous review. The method is still called
render_integrations_tab()while it should be renamed torender_deployments_tab()to match the deployments terminology.routers/bots_api.py (1)
120-138: Duplicate: POST redirect issue already flagged.This backward-compatibility redirect was already identified as problematic in previous reviews. The HTTP 308 redirect for a POST endpoint will not work correctly for many HTTP clients because they don't automatically replay the request body. The previous review comment suggests delegating to
stream_create(request, response)directly instead of redirecting.routers/twilio_ws_api.py (1)
47-47: Duplicate: Typo in method name already flagged.The use of
api_deploymet_id()with the typo was already identified in previous reviews. The method should beapi_deployment_id().daras_ai_v2/bot_integration_connect.py (1)
33-33: Duplicate: Typo in method name already flagged.The use of
api_deploymet_id()with the typo was already identified in previous reviews. The method should beapi_deployment_id().tests/test_public_endpoints.py (1)
73-77: Correct the deployment accessor typo
api_deploymet_id()is a misspelling of the intendedapi_deployment_id(). This rename leaks to every caller and effectively changes the public API surface to the wrong name. Please rename the helper inbots/models/bot_integration.py(and the mirrored method inbots/models/convo_msg.py) toapi_deployment_id(), add an alias if you need temporary compatibility, and update this call accordingly.- deployment_id=bi.api_deploymet_id(), + deployment_id=bi.api_deployment_id(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
bots/admin.py(2 hunks)bots/models/bot_integration.py(4 hunks)bots/models/convo_msg.py(8 hunks)daras_ai_v2/api_examples_widget.py(3 hunks)daras_ai_v2/bot_integration_connect.py(1 hunks)daras_ai_v2/bot_integration_widgets.py(3 hunks)daras_ai_v2/bots.py(2 hunks)daras_ai_v2/breadcrumbs.py(1 hunks)daras_ai_v2/icons.py(1 hunks)daras_ai_v2/meta_content.py(3 hunks)daras_ai_v2/variables_widget.py(1 hunks)functions/models.py(2 hunks)recipes/VideoBots.py(7 hunks)recipes/VideoBotsStats.py(4 hunks)routers/bots_api.py(9 hunks)routers/root.py(5 hunks)routers/twilio_ws_api.py(1 hunks)tests/test_integrations_api.py(1 hunks)tests/test_public_endpoints.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- functions/models.py
- daras_ai_v2/bots.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-16T11:42:40.993Z
Learnt from: nikochiko
Repo: GooeyAI/gooey-server PR: 703
File: daras_ai_v2/settings.py:301-308
Timestamp: 2025-06-16T11:42:40.993Z
Learning: In daras_ai_v2/settings.py, when using a localhost frontend with api.gooey.ai backend deployment, hard-coded "/explore/" paths work correctly as relative URLs, but EXPLORE_URL computed from APP_BASE_URL may point to the wrong domain (localhost instead of api.gooey.ai).
Applied to files:
recipes/VideoBotsStats.py
🧬 Code graph analysis (14)
daras_ai_v2/bot_integration_connect.py (3)
routers/root.py (1)
RecipeTabs(932-976)bots/models/bot_integration.py (1)
api_deploymet_id(560-563)bots/models/convo_msg.py (1)
api_deploymet_id(333-336)
tests/test_public_endpoints.py (3)
routers/root.py (2)
RecipeTabs(932-976)deployments_stats_route(458-479)bots/models/bot_integration.py (1)
api_deploymet_id(560-563)bots/models/convo_msg.py (1)
api_deploymet_id(333-336)
daras_ai_v2/meta_content.py (1)
routers/root.py (1)
RecipeTabs(932-976)
tests/test_integrations_api.py (1)
bots/models/bot_integration.py (1)
api_deploymet_id(560-563)
daras_ai_v2/breadcrumbs.py (1)
routers/root.py (1)
RecipeTabs(932-976)
routers/twilio_ws_api.py (2)
bots/models/bot_integration.py (1)
api_deploymet_id(560-563)bots/models/convo_msg.py (1)
api_deploymet_id(333-336)
bots/models/convo_msg.py (1)
bots/models/bot_integration.py (1)
api_deploymet_id(560-563)
routers/bots_api.py (2)
recipes/VideoBots.py (1)
VideoBotsPage(142-2126)bots/models/bot_integration.py (1)
BotIntegration(163-626)
bots/admin.py (5)
routers/root.py (1)
deployments_stats_route(458-479)bots/models/bot_integration.py (1)
api_deploymet_id(560-563)bots/models/convo_msg.py (1)
api_deploymet_id(333-336)bots/admin_links.py (1)
open_in_new_tab(13-26)daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)
bots/models/bot_integration.py (1)
bots/models/convo_msg.py (1)
api_deploymet_id(333-336)
routers/root.py (3)
daras_ai_v2/bot_integration_widgets.py (1)
get_web_widget_embed_code(364-385)bots/models/bot_integration.py (1)
BotIntegration(163-626)routers/account.py (1)
TabData(246-248)
daras_ai_v2/bot_integration_widgets.py (2)
bots/models/bot_integration.py (1)
api_deploymet_id(560-563)daras_ai_v2/api_examples_widget.py (1)
bot_api_example_generator(333-435)
recipes/VideoBotsStats.py (5)
routers/root.py (1)
RecipeTabs(932-976)bots/models/bot_integration.py (2)
api_deploymet_id(560-563)get_app_url(775-781)bots/models/convo_msg.py (1)
api_deploymet_id(333-336)bots/models/saved_run.py (1)
get_app_url(172-178)bots/models/published_run.py (1)
get_app_url(243-246)
recipes/VideoBots.py (3)
routers/root.py (1)
RecipeTabs(932-976)daras_ai_v2/base.py (1)
current_app_url(213-230)bots/models/bot_integration.py (2)
Platform(28-75)api_deploymet_id(560-563)
🪛 Ruff (0.14.3)
routers/bots_api.py
133-133: Unused function argument: request
(ARG001)
133-133: Unused function argument: response
(ARG001)
routers/root.py
441-441: Unused function argument: run_slug
(ARG001)
441-441: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
442-442: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
460-460: Unused function argument: page_slug
(ARG001)
462-462: Unused function argument: run_slug
(ARG001)
462-462: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
463-463: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
464-464: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
495-495: Unused function argument: run_slug
(ARG001)
495-495: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
496-496: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
497-497: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
505-505: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
535-535: Unused function argument: page_slug
(ARG001)
536-536: Unused function argument: run_slug
(ARG001)
536-536: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
537-537: Unused function argument: example_id
(ARG001)
537-537: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
538-538: Unused function argument: integration_id
(ARG001)
538-538: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: Unused function argument: deployment_name
(ARG001)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
597-597: Unused function argument: deployment_name
(ARG001)
597-597: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
⏰ 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: test (3.10.12, 1.8.3)
🔇 Additional comments (4)
daras_ai_v2/icons.py (1)
76-76: Constant rename verified as complete.The rename from
integrations_imgtodeployments_imgis confirmed complete and correct. No stale references to the old constant name remain in the codebase. The constant is properly defined at line 76 and is actively used in 4 files (recipes/VideoBots.py, routers/root.py, functions/models.py, and daras_ai_v2/bot_integration_widgets.py), all correctly referencing it via the module namespace pattern (icons.deployments_img).bots/models/convo_msg.py (3)
98-99: Column rename looks good.The rename from "Integration Name" to "Deployment Name" is consistent with the PR's refactoring objective.
Also applies to: 118-118
381-381: Column rename is correct.The "Deployment Name" column key is properly renamed as part of the refactor.
626-626: Column rename looks good.The rename from "Integration Name" to "Deployment Name" in
FeedbackQuerySet.to_df()is consistent with the refactoring objective.Also applies to: 642-642
83c031b to
87fd9c6
Compare
- Rename API endpoints: /v3/integrations/ -> /v3/deployments/ - Add backward compatibility redirects for old endpoints - Rename integration_id to deployment_id in models and API - Update UI labels: "Integrations" -> "Deployments"/"Deploy" - Update RecipeTabs.integrations -> RecipeTabs.deployments - Mark deprecated integration_id/integration_name variables Maintains backward compatibility: old integration_id field and endpoints still work via redirects and field aliases.
87fd9c6 to
e2aebe6
Compare
There was a problem hiding this 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
♻️ Duplicate comments (1)
routers/bots_api.py (1)
120-138: Critical: POST redirects break backward compatibility.Returning
308 Permanent Redirectfor POST/v3/integrations/streamis a breaking change. Many HTTP clients (including popular libraries likerequestsin Python andfetchin JavaScript) do not automatically replay POST request bodies across 308 redirects, causing calls to fail instead of seamlessly working.To maintain true backward compatibility, delegate directly to
stream_createrather than redirecting:def stream_create_integrations(request: CreateStreamRequest, response: Response): - """Backward compatibility endpoint. Redirects to /v3/deployments/stream""" - redirect_url = str( - furl(settings.API_BASE_URL) / app.url_path_for(stream_create.__name__) - ) - return RedirectResponse(url=redirect_url, status_code=HTTP_308_PERMANENT_REDIRECT) + """Backward compatibility endpoint. Returns the same response as /v3/deployments/stream""" + return stream_create(request, response)
🧹 Nitpick comments (2)
daras_ai_v2/variables_widget.py (1)
215-220: LGTM! Clean implementation of the deprecated badge.The conditional rendering follows the established pattern for variable metadata badges and safely handles the deprecated flag using
.get().Optional: Consider visual differentiation for deprecated variables.
While the current implementation is consistent with other informational badges, deprecated variables might benefit from more prominent visual styling (e.g., warning color, icon) to draw attention to items that will be removed:
if schema.get("deprecated"): + gui.html('<i class="fa-solid fa-triangle-exclamation text-warning pe-1"></i>') gui.write( "Deprecated", help="This variable is deprecated and will be removed in a future version. ", - className="text-muted small", + className="text-warning small", )Nitpick: Minor formatting inconsistency.
Line 218 has a trailing space after the period, while line 212 does not. For consistency:
- help="This variable is deprecated and will be removed in a future version. ", + help="This variable is deprecated and will be removed in a future version.",routers/bots_api.py (1)
233-254: GET redirect is acceptable but consider documenting the breaking change.The 301 redirect for GET requests is generally safe since GET is idempotent and most clients handle GET redirects correctly. However, clients will need to update their bookmarked/stored URLs. Consider adding a deprecation timeline in the API documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
bots/admin.py(2 hunks)bots/models/bot_integration.py(4 hunks)bots/models/convo_msg.py(8 hunks)daras_ai_v2/api_examples_widget.py(3 hunks)daras_ai_v2/bot_integration_connect.py(1 hunks)daras_ai_v2/bot_integration_widgets.py(4 hunks)daras_ai_v2/bots.py(2 hunks)daras_ai_v2/breadcrumbs.py(1 hunks)daras_ai_v2/icons.py(1 hunks)daras_ai_v2/meta_content.py(3 hunks)daras_ai_v2/variables_widget.py(1 hunks)functions/models.py(2 hunks)recipes/VideoBots.py(8 hunks)recipes/VideoBotsStats.py(4 hunks)routers/bots_api.py(9 hunks)routers/root.py(5 hunks)routers/twilio_ws_api.py(1 hunks)tests/test_integrations_api.py(1 hunks)tests/test_public_endpoints.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- daras_ai_v2/breadcrumbs.py
- tests/test_public_endpoints.py
- functions/models.py
- daras_ai_v2/meta_content.py
- daras_ai_v2/bot_integration_widgets.py
- daras_ai_v2/icons.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-16T11:42:40.993Z
Learnt from: nikochiko
Repo: GooeyAI/gooey-server PR: 703
File: daras_ai_v2/settings.py:301-308
Timestamp: 2025-06-16T11:42:40.993Z
Learning: In daras_ai_v2/settings.py, when using a localhost frontend with api.gooey.ai backend deployment, hard-coded "/explore/" paths work correctly as relative URLs, but EXPLORE_URL computed from APP_BASE_URL may point to the wrong domain (localhost instead of api.gooey.ai).
Applied to files:
recipes/VideoBotsStats.py
🧬 Code graph analysis (11)
daras_ai_v2/bot_integration_connect.py (2)
routers/root.py (1)
RecipeTabs(932-976)bots/models/bot_integration.py (1)
api_deployment_id(560-563)
tests/test_integrations_api.py (1)
bots/models/bot_integration.py (1)
api_deployment_id(560-563)
bots/models/convo_msg.py (1)
bots/models/bot_integration.py (1)
api_deployment_id(560-563)
routers/twilio_ws_api.py (2)
bots/models/bot_integration.py (1)
api_deployment_id(560-563)bots/models/convo_msg.py (1)
api_deployment_id(333-336)
daras_ai_v2/bots.py (2)
bots/models/bot_integration.py (2)
api_deployment_id(560-563)Platform(28-75)bots/models/convo_msg.py (1)
api_deployment_id(333-336)
routers/bots_api.py (2)
recipes/VideoBots.py (1)
VideoBotsPage(142-2126)bots/models/bot_integration.py (1)
BotIntegration(163-626)
recipes/VideoBots.py (6)
daras_ai_v2/bot_integration_widgets.py (1)
deployments_welcome_screen(27-59)routers/root.py (1)
RecipeTabs(932-976)daras_ai_v2/base.py (1)
current_app_url(213-230)daras_ai_v2/breadcrumbs.py (1)
get_title_breadcrumbs(50-121)bots/models/published_run.py (1)
PublishedRun(118-346)bots/models/bot_integration.py (4)
BotIntegration(163-626)Platform(28-75)get_icon(36-51)api_deployment_id(560-563)
bots/models/bot_integration.py (1)
bots/models/convo_msg.py (1)
api_deployment_id(333-336)
bots/admin.py (5)
routers/root.py (1)
deployments_stats_route(458-479)bots/models/bot_integration.py (1)
api_deployment_id(560-563)bots/models/convo_msg.py (1)
api_deployment_id(333-336)bots/admin_links.py (1)
open_in_new_tab(13-26)daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)
recipes/VideoBotsStats.py (5)
routers/root.py (1)
RecipeTabs(932-976)bots/models/bot_integration.py (2)
api_deployment_id(560-563)get_app_url(775-781)bots/models/convo_msg.py (1)
api_deployment_id(333-336)bots/models/saved_run.py (1)
get_app_url(172-178)bots/models/published_run.py (1)
get_app_url(243-246)
routers/root.py (2)
daras_ai_v2/bot_integration_widgets.py (1)
get_web_widget_embed_code(364-385)bots/models/bot_integration.py (1)
BotIntegration(163-626)
🪛 Ruff (0.14.3)
routers/bots_api.py
133-133: Unused function argument: request
(ARG001)
133-133: Unused function argument: response
(ARG001)
routers/root.py
441-441: Unused function argument: run_slug
(ARG001)
441-441: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
442-442: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
460-460: Unused function argument: page_slug
(ARG001)
462-462: Unused function argument: run_slug
(ARG001)
462-462: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
463-463: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
464-464: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
495-495: Unused function argument: run_slug
(ARG001)
495-495: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
496-496: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
497-497: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
505-505: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
535-535: Unused function argument: page_slug
(ARG001)
536-536: Unused function argument: run_slug
(ARG001)
536-536: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
537-537: Unused function argument: example_id
(ARG001)
537-537: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
538-538: Unused function argument: integration_id
(ARG001)
538-538: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: Unused function argument: deployment_name
(ARG001)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
597-597: Unused function argument: deployment_name
(ARG001)
597-597: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🔇 Additional comments (28)
daras_ai_v2/bot_integration_connect.py (1)
30-34: LGTM!The tab routing and path parameters have been correctly updated to use deployment terminology, and the method call
api_deployment_id()is properly spelled.routers/twilio_ws_api.py (1)
47-47: LGTM!The WebSocket proxy API now correctly uses
api_deployment_id()to pass the bot integration identifier.bots/models/bot_integration.py (4)
519-522: LGTM!The display name has been updated to show "Deployment ID" instead of "Integration ID", consistent with the terminology change across the codebase.
548-550: LGTM!The bot test link generation now correctly uses
deployment_idanddeployment_nameas path parameters, aligning with the renamed routing structure.
560-563: LGTM!The public API method
api_deployment_id()is correctly defined and will be used throughout the codebase to replace the previousapi_integration_id()method.
570-572: LGTM!The web widget configuration now uses
deployment_idas the key, maintaining consistency with the deployment terminology throughout the API surface.daras_ai_v2/api_examples_widget.py (3)
333-333: LGTM!The function signature has been updated to use
deployment_idinstead ofintegration_id, maintaining consistency with the broader API changes.
345-346: LGTM!The API example code correctly references the deployment terminology in both the comment and the JSON payload field.
390-390: LGTM!The user-facing label has been updated to "Your Deployment ID", consistent with the terminology change.
bots/models/convo_msg.py (5)
98-98: LGTM!The DataFrame column label has been updated from "Integration Name" to "Deployment Name", consistent with the terminology change.
296-296: LGTM!The fallback logic now uses
api_deployment_id()to generate a unique user identifier when no other user ID fields are set.
333-336: LGTM!The public API method
api_deployment_id()is correctly defined for the Conversation model, maintaining consistency with the BotIntegration model.
437-439: LGTM!The message serialization correctly uses both
api_deployment_id()for the conversation ID anddeployment_namefor the bot integration name.
626-626: LGTM!The feedback DataFrame column label has been updated to "Deployment Name", maintaining consistency across all data export paths.
routers/bots_api.py (3)
40-43: LGTM!Using
validation_aliaswithAliasChoicesprovides backward compatibility, allowing clients to send eitherdeployment_idor the deprecatedintegration_idfield. This is a clean approach to the API migration.
287-293: LGTM!The initialization correctly decodes
deployment_idand provides a clear error message referencing "Bot Deployment" when the ID is not found.
309-310: LGTM!The error message clearly indicates a deployment mismatch and includes a note that
integration_idis deprecated, helping developers migrate to the new terminology.tests/test_integrations_api.py (1)
19-19: LGTM!The test correctly uses
api_deployment_id()to generate the identifier. Note that this test validates the backward-compatibility endpoint/v3/integrations/stream/, which should continue working after addressing the redirect issue flagged inrouters/bots_api.py.bots/admin.py (2)
43-43: LGTM!The import has been updated to reference
deployments_stats_routeinstead of the previousintegrations_stats_route, consistent with the routing changes.
347-357: LGTM!The admin integration stats URL generation has been correctly updated to use
deployment_idthroughout—in variable naming, path parameters, and the display label.daras_ai_v2/bots.py (1)
640-692: LGTM! Clean refactoring with proper backward compatibility.The deployment variable naming is well-implemented:
- Lines 640-641 introduce
deployment_idanddeployment_nameas the primary variables- Lines 647-648 preserve backward-compatible aliases (
integration_id,integration_name) that map to the same values- Lines 691-692 correctly mark the deprecated aliases in the schema with
"deprecated": Trueto exclude them from the UIThis ensures existing integrations continue to work while new code adopts the clearer "deployment" terminology.
recipes/VideoBotsStats.py (2)
726-758: LGTM! Consistent refactoring with proper deprecation marking.The export function variables correctly mirror the pattern from
bots.py:
- Lines 726-727 create
deployment_idanddeployment_namefrom the BotIntegration- Lines 732-733 use the new deployment naming in the variables dict
- Lines 735-736 preserve backward-compatible
integration_id/integration_namealiases- Lines 757-758 properly mark deprecated variables in the schema (now uncommented ✓)
This ensures exported data uses the new terminology while maintaining compatibility with existing consumers.
116-120: No issues found; the code is correct.The redirect URL construction properly accepts
path_paramswithdeployment_idthroughout the entire chain:
app_url()acceptspath_paramsand unpacks it tourl_path()url_path()accepts it as**kwargsand passes toget_route_path()get_route_path()unpacks it to FastAPI'surl_path_for()for path parameter interpolationThe code at lines 116–120 in VideoBotsStats.py is correct.
recipes/VideoBots.py (2)
1607-1610: LGTM! Method naming now consistent with deployments terminology.The method has been correctly renamed from
render_integrations_tab()torender_deployments_tab(), addressing the past review comment. Both the call site (line 1608) and definition (line 1610) use the updated name.
1788-1819: LGTM! Clean parameter renaming with correct usage.The method signature and implementation correctly use
deploymentsterminology:
- Line 1789: Parameter renamed from
integrationstodeployments- Line 1797: Local variable
deployments_mapreplacesintegrations_map- Lines 1801-1808: Map operations use the renamed variables consistently
All references within the method body align with the new naming.
routers/root.py (3)
509-545: LGTM! Backward-compatible redirects correctly preserve page slugs.The redirect logic at lines 542-544 correctly addresses the past review concern:
new_url.path.segments = [ "deployments" if seg == "integrations" else seg for seg in new_url.path.segments ]This only replaces the exact segment
"integrations"with"deployments", leaving other segments (including slugs that contain "integrations" as a substring) unchanged. For example:
- ✅
/my-workflow/integrations/123/→/my-workflow/deployments/123/- ✅
/my-integrations-workflow/run/→/my-integrations-workflow/run/(unchanged)The comprehensive route patterns (lines 511-531) cover all legacy endpoints:
/integrations/add/→/deployments/add//integrations/{id}/stats/→/deployments/{id}/stats//integrations/{id}/analysis/→/deployments/{id}/analysis//integrations/and/integrations/{id}/→/deployments/and/deployments/{id}/
484-506: LGTM! Clean route with proper ID decoding.The new
deployments_routecorrectly:
- Accepts optional
deployment_idpath parameter (lines 497)- Decodes it using
api_hashidsand populatesgui.session_state["bi_id"](lines 501-505)- Handles missing/invalid IDs with 404 (line 505)
- Delegates to
render_recipe_pagewithRecipeTabs.deployments(line 506)The pattern matches both list views (
/deployments/) and detail views (/deployments/{id}/).
568-593: LGTM! Chat route migrated to deployment terminology.The
chat_routenow usesdeployment_idanddeployment_nameparameters (line 570), correctly:
- Decoding the
deployment_idviaapi_hashids(line 576)- Looking up the BotIntegration (line 576)
- Returning 404 for invalid IDs (line 578)
The route path pattern at line 568 uses the new naming:
/chat/{deployment_name}-{deployment_id}/
There was a problem hiding this 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
♻️ Duplicate comments (1)
routers/bots_api.py (1)
134-138: Restore 200-response semantics for the legacy stream endpointRedirecting the POST
/v3/integrations/streamwith HTTP 308 is a breaking change; most HTTP clients do not replay non-idempotent requests, so legacy callers never receive the JSON payload (includingstream_url). Please keep the old behaviour until clients migrate.Apply this diff to delegate to
stream_createand preserve the original response:def stream_create_integrations(request: CreateStreamRequest, response: Response): - """Backward compatibility endpoint. Redirects to /v3/deployments/stream""" - redirect_url = str( - furl(settings.API_BASE_URL) / app.url_path_for(stream_create.__name__) - ) - return RedirectResponse(url=redirect_url, status_code=HTTP_308_PERMANENT_REDIRECT) + """Backward compatibility endpoint. Returns the same payload as /v3/deployments/stream.""" + return stream_create(request, response)
🧹 Nitpick comments (1)
bots/admin.py (1)
342-357: Rename admin label to match deployments terminology.The method still renders “Integration Stats”, which now mislabels the action after the deploy* rename. Please update the display string (and, if desired, the method name) to keep the admin UI consistent with “Deployment Stats”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
bots/admin.py(2 hunks)bots/models/bot_integration.py(4 hunks)bots/models/convo_msg.py(8 hunks)daras_ai_v2/api_examples_widget.py(3 hunks)daras_ai_v2/bot_integration_connect.py(1 hunks)daras_ai_v2/bot_integration_widgets.py(4 hunks)daras_ai_v2/bots.py(2 hunks)daras_ai_v2/breadcrumbs.py(1 hunks)daras_ai_v2/icons.py(1 hunks)daras_ai_v2/meta_content.py(3 hunks)daras_ai_v2/variables_widget.py(1 hunks)functions/models.py(2 hunks)recipes/VideoBots.py(8 hunks)recipes/VideoBotsStats.py(4 hunks)routers/bots_api.py(9 hunks)routers/root.py(5 hunks)routers/twilio_ws_api.py(1 hunks)tests/test_integrations_api.py(1 hunks)tests/test_public_endpoints.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- daras_ai_v2/meta_content.py
- daras_ai_v2/bots.py
- routers/twilio_ws_api.py
- functions/models.py
- tests/test_integrations_api.py
- daras_ai_v2/api_examples_widget.py
- bots/models/bot_integration.py
- daras_ai_v2/bot_integration_connect.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-16T11:42:40.993Z
Learnt from: nikochiko
Repo: GooeyAI/gooey-server PR: 703
File: daras_ai_v2/settings.py:301-308
Timestamp: 2025-06-16T11:42:40.993Z
Learning: In daras_ai_v2/settings.py, when using a localhost frontend with api.gooey.ai backend deployment, hard-coded "/explore/" paths work correctly as relative URLs, but EXPLORE_URL computed from APP_BASE_URL may point to the wrong domain (localhost instead of api.gooey.ai).
Applied to files:
recipes/VideoBotsStats.py
🧬 Code graph analysis (9)
daras_ai_v2/breadcrumbs.py (1)
routers/root.py (1)
RecipeTabs(932-976)
tests/test_public_endpoints.py (3)
routers/root.py (2)
RecipeTabs(932-976)deployments_stats_route(458-479)bots/models/bot_integration.py (1)
api_deployment_id(560-563)bots/models/convo_msg.py (1)
api_deployment_id(333-336)
bots/models/convo_msg.py (1)
bots/models/bot_integration.py (1)
api_deployment_id(560-563)
recipes/VideoBotsStats.py (5)
routers/root.py (1)
RecipeTabs(932-976)bots/models/bot_integration.py (3)
api_deployment_id(560-563)get_app_url(775-781)Platform(28-75)bots/models/convo_msg.py (1)
api_deployment_id(333-336)bots/models/saved_run.py (1)
get_app_url(172-178)bots/models/published_run.py (1)
get_app_url(243-246)
bots/admin.py (5)
routers/root.py (1)
deployments_stats_route(458-479)bots/models/bot_integration.py (1)
api_deployment_id(560-563)bots/models/convo_msg.py (1)
api_deployment_id(333-336)bots/admin_links.py (1)
open_in_new_tab(13-26)daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)
daras_ai_v2/bot_integration_widgets.py (2)
bots/models/bot_integration.py (1)
api_deployment_id(560-563)daras_ai_v2/api_examples_widget.py (1)
bot_api_example_generator(333-435)
routers/root.py (3)
daras_ai_v2/bot_integration_widgets.py (1)
get_web_widget_embed_code(364-385)bots/models/bot_integration.py (1)
BotIntegration(163-626)routers/account.py (1)
TabData(246-248)
recipes/VideoBots.py (4)
daras_ai_v2/bot_integration_widgets.py (1)
deployments_welcome_screen(27-59)routers/root.py (1)
RecipeTabs(932-976)daras_ai_v2/breadcrumbs.py (1)
get_title_breadcrumbs(50-121)bots/models/bot_integration.py (3)
Platform(28-75)get_icon(36-51)api_deployment_id(560-563)
routers/bots_api.py (2)
recipes/VideoBots.py (1)
VideoBotsPage(142-2126)bots/models/bot_integration.py (1)
BotIntegration(163-626)
🪛 Ruff (0.14.3)
routers/root.py
441-441: Unused function argument: run_slug
(ARG001)
441-441: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
442-442: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
460-460: Unused function argument: page_slug
(ARG001)
462-462: Unused function argument: run_slug
(ARG001)
462-462: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
463-463: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
464-464: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
495-495: Unused function argument: run_slug
(ARG001)
495-495: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
496-496: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
497-497: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
505-505: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
535-535: Unused function argument: page_slug
(ARG001)
536-536: Unused function argument: run_slug
(ARG001)
536-536: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
537-537: Unused function argument: example_id
(ARG001)
537-537: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
538-538: Unused function argument: integration_id
(ARG001)
538-538: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
570-570: Unused function argument: deployment_name
(ARG001)
570-570: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
597-597: Unused function argument: deployment_name
(ARG001)
597-597: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
routers/bots_api.py
133-133: Unused function argument: request
(ARG001)
133-133: Unused function argument: response
(ARG001)
⏰ 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: test (3.10.12, 1.8.3)
🔇 Additional comments (6)
bots/models/convo_msg.py (6)
98-98: LGTM! Column label updated consistently.The column label has been changed from "Integration Name" to "Deployment Name" in both the data row and column definition, maintaining consistency throughout the DataFrame export.
Also applies to: 118-118
296-296: LGTM! Method call updated correctly.The fallback logic correctly calls the renamed
api_deployment_id()method.
381-381: LGTM! Column label updated.The dictionary key has been updated from "Integration Name" to "Deployment Name", consistent with the renaming throughout the file.
437-438: LGTM! Method call and field name updated correctly.Both the method call to
api_deployment_id()and the dictionary key to "deployment_name" have been updated consistently.
626-626: LGTM! Column label updated consistently.The column label has been changed from "Integration Name" to "Deployment Name" in both the feedback data row and column definition.
Also applies to: 642-642
333-336: Method rename verified successfully across codebase.The renaming of
api_integration_id()toapi_deployment_id()is complete with zero references to the old method name remaining. The new method is actively used in 20+ locations across tests, recipes, routers, and model files—all correctly updated.
Q/A checklist
How to check import time?
You can visualize this using tuna:
To measure import time for a specific library:
To reduce import times, import libraries that take a long time inside the functions that use them instead of at the top of the file:
Legal Boilerplate
Look, I get it. The entity doing business as “Gooey.AI” and/or “Dara.network” was incorporated in the State of Delaware in 2020 as Dara Network Inc. and is gonna need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Dara Network Inc can use, modify, copy, and redistribute my contributions, under its choice of terms.