Conversation
|
Warning Review limit reached
More reviews will be available in 49 minutes and 44 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
📝 WalkthroughWalkthroughThis pull request introduces a 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c2c578e6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…api_call field, filter history and builder conversations by surface
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
daras_ai_v2/base.py (1)
2360-2372:⚠️ Potential issue | 🟠 MajorFix/extend legacy
SavedRun.surfacebackfill before using surface-only history filtering
bots/migrations/0121_savedrun_surface_alter_savedrun_is_api_call.pyaddssurfacewith defaultSavedRun.Surface.runand backfills only rows whereis_api_call=True→surface=SavedRun.Surface.api. Existing rows that represent non-API origins (including deployments and builder prompts) will therefore remainSurface.run.if ( for_param != "all" and self.request.user and not self.current_workspace.is_personal ): qs = qs.filter(uid=self.request.user.uid).exclude( surface=SavedRun.Surface.deployment )This can misclassify historical deployment/builder-prompt runs in the history view because the filters in
daras_ai_v2/base.pyexclude bysurface(deployment/builder_prompt). Add additional migration backfill logic (or derive missing categories from legacy fields) so pre-existing data matches the intendedsurfacevalues.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@daras_ai_v2/base.py` around lines 2360 - 2372, The history filtering misclassifies legacy rows because SavedRun.surface was only partially backfilled by the migration that set surface from is_api_call; update data backfill or derive surfaces before filtering: add a new data migration (or extend the existing backfill) that sets SavedRun.surface for legacy rows based on existing legacy flags/columns (e.g., set surface = SavedRun.Surface.api when is_api_call=True, surface = SavedRun.Surface.deployment for rows with the deployment indicator, surface = SavedRun.Surface.builder_prompt for builder_prompt indicator, otherwise surface = SavedRun.Surface.run), and then ensure the code path in the view (the block that references self.request.user, self.current_workspace, and filters/excludes by SavedRun.Surface.deployment and SavedRun.Surface.builder_prompt) relies on that completed backfill; alternatively, if you cannot run a migration immediately, update the view to compute an inferred surface for filtering (using the same legacy columns) before applying .exclude(surface=...) so historical records are classified correctly.
🧹 Nitpick comments (1)
bots/models/saved_run.py (1)
146-160: ⚡ Quick winConsider adding a database index for the surface field.
The surface field is used in admin filtering (line 462 in bots/admin.py) and the review context indicates surface-based filtering will be common. A composite index would improve query performance for typical admin filters.
🔍 Suggested index to add to the indexes list
indexes = [ models.Index(fields=["-created_at"]), models.Index(fields=["-updated_at"]), models.Index(fields=["workflow"]), + models.Index(fields=["workflow", "surface", "-updated_at"]), models.Index(fields=["uid"]),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bots/models/saved_run.py` around lines 146 - 160, The surface IntegerField in the SavedRun model lacks a DB index, hurting admin filtering; update the field definition for surface in bots/models/saved_run.py to add db_index=True (e.g., surface = models.IntegerField(..., db_index=True)) and also add a Meta.indexes entry on the SavedRun model with models.Index(fields=['surface'], name='savedrun_surface_idx') so queries and admin filters using surface are served by an index.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@daras_ai_v2/base.py`:
- Around line 2360-2372: The history filtering misclassifies legacy rows because
SavedRun.surface was only partially backfilled by the migration that set surface
from is_api_call; update data backfill or derive surfaces before filtering: add
a new data migration (or extend the existing backfill) that sets
SavedRun.surface for legacy rows based on existing legacy flags/columns (e.g.,
set surface = SavedRun.Surface.api when is_api_call=True, surface =
SavedRun.Surface.deployment for rows with the deployment indicator, surface =
SavedRun.Surface.builder_prompt for builder_prompt indicator, otherwise surface
= SavedRun.Surface.run), and then ensure the code path in the view (the block
that references self.request.user, self.current_workspace, and filters/excludes
by SavedRun.Surface.deployment and SavedRun.Surface.builder_prompt) relies on
that completed backfill; alternatively, if you cannot run a migration
immediately, update the view to compute an inferred surface for filtering (using
the same legacy columns) before applying .exclude(surface=...) so historical
records are classified correctly.
---
Nitpick comments:
In `@bots/models/saved_run.py`:
- Around line 146-160: The surface IntegerField in the SavedRun model lacks a DB
index, hurting admin filtering; update the field definition for surface in
bots/models/saved_run.py to add db_index=True (e.g., surface =
models.IntegerField(..., db_index=True)) and also add a Meta.indexes entry on
the SavedRun model with models.Index(fields=['surface'],
name='savedrun_surface_idx') so queries and admin filters using surface are
served by an index.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1155aec-5917-43c8-9eab-8cae72b12078
📒 Files selected for processing (23)
bots/admin.pybots/migrations/0121_savedrun_surface_alter_savedrun_is_api_call.pybots/models/published_run.pybots/models/saved_run.pybots/tasks.pyceleryapp/tasks.pydaras_ai_v2/base.pydaras_ai_v2/bot_integration_widgets.pydaras_ai_v2/bots.pydaras_ai_v2/breadcrumbs.pydaras_ai_v2/gooey_builder.pydaras_ai_v2/safety_checker.pyfunctions/gooey_builder_workflow_tools.pyfunctions/workflow_tools.pylivekit_agent.pyrecipes/BulkRunner.pyrecipes/ModelTrainer.pyrecipes/VideoBotsStats.pyrouters/api.pyrouters/twilio_api.pywidgets/demo_button.pywidgets/workflow_bulk_runs_list.pywidgets/workflow_image.py
…api_call field, filter history and builder conversations by surface
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.