feat: add support for workflow tags #792
Conversation
📝 WalkthroughWalkthroughAdds a Tag model (with TagCategory) and a migration to create it; adds ManyToMany 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
widgets/workflow_search.py (1)
498-511: Avoid duplicate rows from M2M tag join; add distinct() and prefetch tagsIncluding "tags__name" in the SearchVector introduces a JOIN that can duplicate PublishedRun rows. Deduplicate after filtering, and prefetch tags for render callers to prevent N+1 queries.
Apply in-place:
- qs = qs.filter(Q(search=search) | Q(workflow__in=workflow_search)) + qs = qs.filter(Q(search=search) | Q(workflow__in=workflow_search)).distinct()Additionally (outside this hunk), consider prefetching for result rendering:
def get_filtered_published_runs( user: AppUser | None, search_filters: SearchFilters ) -> QuerySet: qs = PublishedRun.objects.all() qs = build_search_filter(qs, search_filters, user=user) qs = build_workflow_access_filter(qs, user) qs = build_sort_filter(qs, SortOptions.get(search_filters.sort)) - return qs[:25] + return qs.prefetch_related("tags")[:25]widgets/saved_workflow.py (1)
947-955: Bug: set() comparison on model instances is unreliable; compare by PKsDjango model instances are not safely hashable for set semantics across different querysets. Compare primary keys instead to avoid false positives/negatives in change detection.
- tags: list[Tag], + tags: list[Tag], ): - return ( + return ( published_run.title != title or published_run.notes != notes or published_run.saved_run != saved_run or published_run.photo_url != photo_url - or set(published_run.tags.all()) != set(tags) + or set( + published_run.tags.values_list("pk", flat=True) + ) != {t.pk for t in tags} )
🧹 Nitpick comments (6)
bots/migrations/0107_tag_publishedrun_tags_publishedrunversion_tags.py (1)
26-26: In bots/migrations/0107_tag_publishedrun_tags_publishedrunversion_tags.py line 26: Consider aligning themax_length=64for tag names with other “name” fields—e.g. workspaces use 100 and managed_secrets use 255—by either increasing this limit or otherwise validating it against your expected tag-name lengths.bots/models/published_run.py (1)
275-276: Potential issue with empty list default for tags.Setting
tags = []whentags is Nonecould cause issues if the same list object is reused. While unlikely in this context, it's safer to maintain the None value or create a new list each time.- if tags is None: - tags = []Then modify line 291 to handle None:
- version.tags.add(*tags) + if tags: + version.tags.add(*tags)widgets/saved_workflow.py (3)
75-77: Prefetch tags at callers to avoid N+1This loop will trigger a query per PublishedRun when used in lists. Ensure upstream queries prefetch "tags" (e.g., workflow_search.get_filtered_published_runs and examples tab).
780-796: Multiselect options: pass keys explicitly for claritygui.multiselect will iterate dict keys anyway, but passing keys is clearer and avoids accidental misuse if the widget API changes.
- options = {t.pk: t for t in Tag.get_options()} + options = {t.pk: t for t in Tag.get_options()} gui.write("Tags", className="fs-5 mb-3") if not isinstance(gui.session_state.get("published_run_tags"), list): gui.session_state["published_run_tags"] = [ tag.pk for tag in pr.tags.all() ] with gui.div(className="flex-grow-1"): tag_pks = gui.multiselect( label="", - options=options, + options=options.keys(), format_func=lambda pk: options[pk].render(), key="published_run_tags", allow_none=True, ) tags = [options[pk] for pk in tag_pks]
2179-2185: Examples tab uses hide_access_level — OKConsider prefetch_related("tags") on the examples queryset for consistency/perf when rendering tag pills.
daras_ai_v2/base.py (1)
486-494: Honor Tag.featured_priority and stable ordering when rendering header pillsCurrent code shows all tags and ignores the “0 = don’t show” contract. Filter out non-featured tags and order by priority desc then name.
- if tags := list(self.current_pr.tags.all()): - with gui.div(className=""): - for tag in tags: - gui.pill( - tag.render(), className="border border-secondary me-1" - ) + tags_qs = self.current_pr.tags.all() + tags = [t for t in tags_qs if getattr(t, "featured_priority", 1) > 0] + tags.sort(key=lambda t: (-getattr(t, "featured_priority", 1), t.name.lower())) + if tags: + with gui.div(className=""): + for tag in tags: + gui.pill(tag.render(), className="border border-secondary me-1")
📜 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 (8)
bots/admin.py(2 hunks)bots/migrations/0107_tag_publishedrun_tags_publishedrunversion_tags.py(1 hunks)bots/models/published_run.py(12 hunks)daras_ai_v2/base.py(16 hunks)scripts/init_tags.py(1 hunks)scripts/run-prod.sh(1 hunks)widgets/saved_workflow.py(5 hunks)widgets/workflow_search.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
scripts/init_tags.py (1)
bots/models/published_run.py (2)
Tag(420-452)TagCategory(411-417)
bots/admin.py (1)
bots/models/published_run.py (1)
Tag(420-452)
widgets/saved_workflow.py (1)
bots/models/published_run.py (1)
get_share_badge_html(319-333)
daras_ai_v2/base.py (1)
bots/models/published_run.py (4)
Tag(420-452)render(435-436)get_options(440-441)PublishedRun(124-359)
⏰ 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 (32)
bots/migrations/0107_tag_publishedrun_tags_publishedrunversion_tags.py (2)
58-63: Good implementation of case-insensitive uniqueness.The use of
Upper("name")for the unique constraint ensures case-insensitive uniqueness, which is a good practice for tag names. The custom violation error message is also user-friendly.
1-1: Fix timestamp inconsistency with September 2025 date.The migration timestamp shows "2025-09-04 13:19" which appears to be in September 2025. This is likely a typo and should be corrected to 2024.
-# Generated by Django 5.1.3 on 2025-09-04 13:19 +# Generated by Django 5.1.3 on 2024-09-04 13:19Likely an incorrect or invalid review comment.
bots/models/published_run.py (1)
38-39: Good consistency in tags parameter handling.The tags parameter is consistently typed as
list[Tag] | None = Noneacross all methods, which provides good type safety and API consistency.Also applies to: 52-54
bots/admin.py (2)
970-983: LGTM! Well-structured admin configuration for Tag model.The TagAdmin class is properly configured with appropriate display fields, search capabilities, and filters. The readonly fields for timestamps follow Django best practices.
30-30: Import and registration are correctly implemented.The Tag model is properly imported and registered with the admin interface using the
@admin.registerdecorator.Also applies to: 970-970
scripts/run-prod.sh (1)
15-16: Idempotent tag seeding step looks goodRuns after migrations and before Gunicorn; ignore_conflicts in the seeding script keeps this safe across concurrent boots.
widgets/workflow_search.py (1)
382-391: Param rename tohide_access_levelis fully applied
Call sites match the updated signature andrgfinds no remaininghide_visibility_pilloccurrences.scripts/init_tags.py (1)
1-21: LGTM: safe, idempotent defaultsbulk_create with ignore_conflicts and concise defaults are appropriate for boot-time seeding.
widgets/saved_workflow.py (12)
31-34: API rename to hide_access_level is consistentSignature and downstream use align with the new name.
83-91: Propagation of hide_access_level to breadcrumbs is correct
169-172: Breadcrumbs param rename (hide_access_level) wired correctly
230-231: Access-level badge gating matches new flagNo behavior change besides the rename.
846-856: Create path now carries tags — goodTags are persisted on new PublishedRun creation.
864-871: Update path now carries tags — goodIncluded in change detection and versioning updates.
984-987: Merge “Duplicate”/“Save as New” labeling — nice simplification
1023-1033: Save-as-new carries tags — good
1037-1041: UI copy tweak (“Version History”) — fine
1123-1132: Admin root-version update preserves tags — good
1512-1516: Prefetch tags in get_pr_from_example_id — good N+1 prevention
1955-1964: Publish-after-login path preserves tags — gooddaras_ai_v2/base.py (12)
38-38: Importing Tag model — LGTMNeeded for type hints and UI rendering.
767-771: Minor layout wrapper — OKThe flex wrapper around change-notes input is fine.
854-855: Pass tags on create — LGTMTags correctly propagated to new PublishedRun.
869-870: Pass tags on update — LGTMChange detection and update path receive tags.
984-987: Duplicate vs Save-as-New label — LGTMClearer UX depending on version recency.
1029-1031: Duplicate carries tags — LGTMPreserves source metadata on copy.
1039-1039: Version history header tweak — LGTM
1129-1131: Root workflow versioning retains tags — LGTM
1512-1516: Prefetch tags for PR fetch — LGTMAvoids N+1 in header/UI rendering.
1962-1963: Publish-after-login propagates tags — LGTM
2183-2184: No remaininghide_visibility_pillusages. Ran a global search (rg -nP 'hide_visibility_pill') and found zero matches—allrender_saved_workflow_previewcall sites now compile against the updatedhide_access_level/hide_version_notessignature.
1546-1559: create_published_run: downstream tags handling verified —PublishedRun.objects.create_with_versionacceptsNonefortags(defaults to empty list) and correctly adds and sets M2M tags viaadd_versionandupdate_fields_to_latest_version.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
bots/admin.py (1)
970-983: Make Tag admin faster to manage and align with “featured_priority”.
- Allow inline editing of featured_priority.
- Order by featured_priority (desc), then name.
- Optional: silence Ruff RUF012 for Django admin class vars.
@admin.register(Tag) class TagAdmin(admin.ModelAdmin): - list_display = [ + list_display = [ "name", "category", "icon", "featured_priority", "created_at", "updated_at", ] + list_editable = ["featured_priority"] + ordering = ["-featured_priority", "name"] search_fields = ["name", "category"] list_filter = ["category"] readonly_fields = ["created_at", "updated_at"] + # noqa: RUF012 - Django admin uses class attributes intentionallywidgets/saved_workflow.py (1)
75-76: Avoid N+1 and respect featured_priority when rendering tags.
- In lists, this loop can cause N+1 if tags aren’t prefetched.
- Hide non-featured tags (featured_priority == 0) and sort by priority.
- for tag in published_run.tags.all(): + for tag in published_run.tags.filter(featured_priority__gt=0) + .order_by("-featured_priority", "name"): gui.pill(tag.render(), className="border border-dark me-1")Pair with prefetch_related("tags") at query sites (see base.py comments).
daras_ai_v2/base.py (2)
485-495: Header tags: filter/sort and keep mobile tidy.Show only featured tags and order by priority. Minor polish.
- if tags := list(self.current_pr.tags.all()): + if tags := list( + self.current_pr.tags.filter(featured_priority__gt=0) + .order_by("-featured_priority", "name") + ):
2173-2177: Prefetch tags in list queries to avoid N+1 in previews.Both examples and saved tabs render tag pills via render_saved_workflow_preview.
- example_runs, cursor = paginate_queryset( - qs=qs, + example_runs, cursor = paginate_queryset( + qs=qs.prefetch_related("tags"), ordering=["-example_priority", "-updated_at"], cursor=self.request.query_params, ) @@ - published_runs, cursor = paginate_queryset( - qs=qs, ordering=["-updated_at"], cursor=self.request.query_params + published_runs, cursor = paginate_queryset( + qs=qs.prefetch_related("tags"), + ordering=["-updated_at"], + cursor=self.request.query_params, )Also applies to: 2213-2215
bots/models/published_run.py (2)
297-309: Ensure atomicity when syncing latest version fields and tags.update_fields_to_latest_version may be called outside an existing transaction. Make it atomic.
- def update_fields_to_latest_version(self): + @transaction.atomic + def update_fields_to_latest_version(self): latest_version = self.versions.latest() self.saved_run = latest_version.saved_run self.last_edited_by = latest_version.changed_by self.title = latest_version.title self.notes = latest_version.notes self.public_access = latest_version.public_access self.workspace_access = latest_version.workspace_access self.photo_url = latest_version.photo_url self.tags.set(latest_version.tags.all()) self.save()
444-452: Enforce non-negative featured_priority at the model level.Mirror the migration check in model Meta for consistency.
class Meta: constraints = [ models.UniqueConstraint( Upper("name"), name="tag_upper_name_is_unique", violation_error_message="This tag already exists", ) - ] + ] + [ + models.CheckConstraint( + check=models.Q(featured_priority__gte=0), + name="tag_featured_priority_non_negative", + ) + ] ordering = ["name"] indexes = [models.Index(fields=["name"])]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
bots/admin.py(2 hunks)bots/migrations/0107_tag_publishedrun_tags_publishedrunversion_tags.py(1 hunks)bots/models/published_run.py(12 hunks)daras_ai_v2/base.py(16 hunks)scripts/init_tags.py(1 hunks)scripts/run-prod.sh(1 hunks)widgets/saved_workflow.py(5 hunks)widgets/workflow_search.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- widgets/workflow_search.py
- scripts/init_tags.py
- scripts/run-prod.sh
🧰 Additional context used
🧬 Code graph analysis (3)
widgets/saved_workflow.py (2)
daras_ai_v2/chat_explore.py (1)
render(16-28)bots/models/published_run.py (1)
get_share_badge_html(319-333)
daras_ai_v2/base.py (1)
bots/models/published_run.py (4)
Tag(420-452)render(435-436)get_options(440-441)PublishedRun(124-359)
bots/admin.py (1)
bots/models/published_run.py (1)
Tag(420-452)
🪛 Ruff (0.13.1)
bots/migrations/0107_tag_publishedrun_tags_publishedrunversion_tags.py
9-11: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
13-80: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
bots/admin.py
972-979: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
980-980: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
981-981: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
982-982: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
bots/models/published_run.py
444-450: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
451-451: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
452-452: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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 (10)
bots/admin.py (1)
30-30: Importing Tag into admin is correct.No issues.
widgets/saved_workflow.py (2)
230-231: OK to swap to hide_access_level for share badge.
31-31: Param rename to hide_access_level — verified.No remaining hide_visibility_pill usages; updated call sites found at widgets/saved_workflow.py:31, 88, 169, 230; widgets/workflow_search.py:389; daras_ai_v2/base.py:2183.
bots/migrations/0107_tag_publishedrun_tags_publishedrunversion_tags.py (1)
42-48: Add DB check to prevent negative featured_priority.Prevents invalid priorities and matches help_text semantics. You can add it to this migration’s model options.
options={ "ordering": ["name"], "indexes": [ models.Index(fields=["name"], name="bots_tag_name_9ec93a_idx") ], - "constraints": [ + "constraints": [ models.UniqueConstraint( django.db.models.functions.text.Upper("name"), name="tag_upper_name_is_unique", violation_error_message="This tag already exists", - ) + ), + models.CheckConstraint( + check=models.Q(featured_priority__gte=0), + name="tag_featured_priority_non_negative", + ), ], },Also applies to: 53-63
daras_ai_v2/base.py (5)
1512-1516: Nice: prefetch tags for example_id path.This prevents N+1 in detail views.
2180-2185: Good rename: hide_access_level applied in examples list.
1546-1559: Tags plumbed through create/update/duplicate flows—LGTM.Data propagation looks consistent.
Also applies to: 1962-1963, 1130-1131
780-796: Multiselect: guard against stale PKs and drop None.Current code can raise KeyError and allows None.
with gui.div(className="d-flex mb-3 align-items-center gap-2"): options = {t.pk: t for t in Tag.get_options()} gui.write("Tags", className="fs-5 mb-3") if not isinstance(gui.session_state.get("published_run_tags"), list): gui.session_state["published_run_tags"] = [ tag.pk for tag in pr.tags.all() ] + else: + gui.session_state["published_run_tags"] = [ + pk for pk in gui.session_state["published_run_tags"] if pk in options + ] with gui.div(className="flex-grow-1"): tag_pks = gui.multiselect( label="", options=options, format_func=lambda pk: options[pk].render(), key="published_run_tags", - allow_none=True, + allow_none=False, ) - tags = [options[pk] for pk in tag_pks] + tags = [options[pk] for pk in tag_pks if pk in options]
947-955: Fix TypeError: unhashable model instances in set comparison.Compare PKs; allow tags to be None.
- tags: list[Tag], + tags: list[Tag] | None, ): return ( published_run.title != title or published_run.notes != notes or published_run.saved_run != saved_run or published_run.photo_url != photo_url - or set(published_run.tags.all()) != set(tags) + or set(published_run.tags.values_list("pk", flat=True)) + != {t.pk for t in (tags or [])} )bots/models/published_run.py (1)
155-156: Tags M2M wiring looks correct.Field definitions and propagation through duplicate/version are consistent.
Also applies to: 394-396, 239-249
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
widgets/workflow_search.py (1)
523-530: Consider using a more descriptive exception message.The static analysis tool suggests avoiding long messages outside the exception class. Consider using a shorter, more descriptive message.
- case _: - raise ValueError(f"Invalid value: {value}") + case _: + raise ValueError("Unsupported ordering value type")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
daras_ai_v2/base.py(16 hunks)widgets/base_header.py(1 hunks)widgets/saved_workflow.py(5 hunks)widgets/workflow_search.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
widgets/saved_workflow.py (1)
bots/models/published_run.py (1)
get_share_badge_html(319-333)
daras_ai_v2/base.py (3)
bots/models/published_run.py (4)
Tag(420-452)get_options(440-441)render(435-436)PublishedRun(124-359)widgets/author.py (1)
render_author_from_workspace(52-82)widgets/base_header.py (1)
render_breadcrumbs_with_author(41-77)
🪛 Ruff (0.13.1)
daras_ai_v2/base.py
484-484: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
widgets/workflow_search.py
530-530: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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 (23)
widgets/base_header.py (1)
70-77: LGTM! Clean tags rendering implementation.The conditional rendering logic is correct - tags are only displayed when the published run is associated with the current saved run, which prevents showing stale tags for different runs. The use of tag.render() method and pill styling is consistent with other parts of the UI.
widgets/workflow_search.py (4)
11-11: LGTM! Proper import for enhanced ordering functionality.The OrderBy import is correctly added to support the refactored sorting logic in
build_sort_filter.
390-390: LGTM! Parameter renamed tohide_access_levelfor consistency.The parameter rename from
hide_visibility_pilltohide_access_levelaligns with similar changes across the codebase and maintains consistency withwidgets/saved_workflow.py.
406-431: LGTM! Cleaner sort implementation with improved distinct performance.The refactor consolidates the sorting logic into a single return path and uses a more efficient
distinctapproach. The use ofget_field_from_orderinghelper ensures correct field extraction for the distinct clause.
513-513: Good addition! Tags now searchable in workflow discovery.The addition of
"tags__name"to the search vector enables users to find published runs by their associated tags, improving discoverability.widgets/saved_workflow.py (5)
31-31: LGTM! Parameter renamed for semantic clarity.The parameter name change from
hide_visibility_pilltohide_access_levelbetter describes the intent and aligns with similar changes across the codebase.
86-86: LGTM! Consistent parameter passing.The updated parameter name is correctly passed to the
render_footer_breadcrumbsfunction.
125-126: LGTM! Clean tag pills rendering.The tag rendering implementation is clean and follows the established pattern used elsewhere in the codebase. The use of
tag.render()and consistent styling with"border ms-2"className maintains UI consistency.
170-170: LGTM! Function signature updated correctly.The parameter name change is properly reflected in the function signature.
231-232: LGTM! Consistent parameter usage in conditional rendering.The access level check now uses the renamed parameter consistently.
daras_ai_v2/base.py (13)
38-38: LGTM! Clean import for Tag model.The import is properly added to support the new tagging functionality.
425-434: LGTM! Improved header layout with proper conditional rendering.The refactored header logic provides better separation between different tab types and includes proper early returns for non-Run/Preview tabs. The image styling consolidation is also cleaner.
436-486: LGTM! Enhanced Run/Preview header layout with proper responsive design.The enhanced layout provides better structure for images, titles, and social buttons with proper responsive behavior. The breadcrumb rendering integration maintains clean separation of concerns.
489-491: LGTM! Clean container styling for example display.The styling improvement provides better visual separation and spacing for example content.
764-792: Fix multiselect robustness issues.There are two issues with the current implementation:
- Django model instances are unhashable and cannot be used in sets for comparison
- The multiselect allows
Nonevalues and doesn't handle stale PKsDjango models override
__eq__()but don't define__hash__(), making them unhashable when used in sets.Apply the following fixes:
+ else: + # sanitize any stale/deleted PKs in session state + gui.session_state["published_run_tags"] = [ + pk for pk in gui.session_state["published_run_tags"] if pk in options + ] with gui.div(className="flex-grow-1"): tag_pks = gui.multiselect( label="", options=options, format_func=lambda pk: options[pk].render(), key="published_run_tags", - allow_none=True, + allow_none=False, ) - tags = [options[pk] for pk in tag_pks] + tags = [options[pk] for pk in tag_pks if pk in options]
851-851: LGTM! Tags parameter added to creation and version flows.The tags parameter is correctly integrated into both the
create_published_runandadd_versionflows, enabling proper tag propagation through the publishing workflow.Also applies to: 866-866
944-951: Fix unhashable Django model comparison.Django model instances that override
__eq__()but not__hash__()are unhashable and cannot be used in sets.The comparison will raise
TypeError: unhashable type: 'Tag'. Compare primary keys instead:- tags: list[Tag], + tags: list[Tag] | None, ): return ( published_run.title != title or published_run.notes != notes or published_run.saved_run != saved_run or published_run.photo_url != photo_url - or set(published_run.tags.all()) != set(tags) + or set(published_run.tags.values_list("pk", flat=True)) + != {t.pk for t in (tags or [])} )
981-983: LGTM! Simplified button logic with better terminology.The refactoring consolidates the "Save as New" and "Duplicate" button handling with clearer terminology based on context (latest version vs. older version).
1026-1026: LGTM! Tags propagated correctly through duplication and versioning flows.Tags are properly carried through all the workflow duplication and versioning operations, maintaining data consistency.
Also applies to: 1127-1127, 1959-1959
1036-1036: LGTM! Version history header with proper icon.The version history header provides clear visual indication with the time icon.
1509-1512: LGTM! Optimized query with tags prefetch.The
prefetch_related("tags")addition optimizes database queries when retrieving published runs with their associated tags, preventing N+1 query issues.
1543-1543: LGTM! Tags parameter added to PublishedRun creation.The tags parameter is correctly added to the
create_published_runmethod signature and properly passed to the underlying creation method.Also applies to: 1555-1555
2180-2180: LGTM! Consistent parameter usage in examples tab.The parameter rename is properly applied in the examples tab rendering.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
widgets/workflow_search.py (1)
523-530: Helper function for extracting field names from ordering values.The function correctly handles both
OrderByobjects and string-based ordering expressions. Consider extracting the error message to a constant or providing more context about what constitutes a valid value.def get_field_from_ordering(value: str | OrderBy) -> str: match value: case OrderBy(): return value.expression.name case str(): return value.lstrip("-") case _: - raise ValueError(f"Invalid value: {value}") + raise ValueError( + f"Invalid ordering value: {value}. " + f"Expected str or OrderBy, got {type(value).__name__}" + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
widgets/base_header.py(1 hunks)widgets/workflow_search.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- widgets/base_header.py
🧰 Additional context used
🪛 Ruff (0.13.1)
widgets/workflow_search.py
530-530: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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 (5)
widgets/workflow_search.py (5)
11-11: LGTM! Import added for ordering field extraction.The
OrderByimport is necessary for the newget_field_from_orderinghelper function.
390-390: Parameter renamed to improve clarity.The rename from
hide_visibility_pilltohide_access_levelbetter reflects the semantic purpose of the parameter.
406-432: Refactored sorting logic with consistent return path.Good refactoring! The sorting logic now uses a single return path with tuple accumulation for all cases, improving maintainability and consistency.
513-513: Tags added to search vector for full-text search.Adding
"tags__name"to the search vector enables searching by tag names, which aligns well with the PR's tag support feature.
428-431: No action required — repository configured for PostgreSQLdaras_ai_v2/settings.py sets DATABASES['default']['ENGINE'] = "django.db.backends.postgresql" (line 134). The field-argument form of distinct() is PostgreSQL-specific but is supported by this configuration; change it only if you plan to support non‑Postgres backends.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
widgets/base_header.py(1 hunks)widgets/saved_workflow.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- widgets/base_header.py
🧰 Additional context used
🧬 Code graph analysis (1)
widgets/saved_workflow.py (2)
daras_ai_v2/chat_explore.py (1)
render(16-28)bots/models/published_run.py (1)
get_share_badge_html(319-333)
⏰ 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 (3)
widgets/saved_workflow.py (3)
31-31: LGTM! Parameter rename improves clarity.The rename from
hide_visibility_pilltohide_access_levelbetter describes the functionality and aligns with the broader code changes in this PR.
64-64: LGTM! Function calls updated to use the new parameter name.Both calls to
render_title_pillsandrender_footer_breadcrumbscorrectly use the renamedhide_access_levelparameter.Also applies to: 86-86
170-170: LGTM! Parameter name updated consistently.The
hide_access_levelparameter is correctly used throughout therender_footer_breadcrumbsfunction, maintaining consistency with the broader parameter rename.Also applies to: 231-232
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
bots/models/published_run.py (2)
298-309: Make update_fields_to_latest_version atomic to avoid partial updates.This method may be called outside an existing transaction; wrap it to ensure the M2M set and save are atomic.
def update_fields_to_latest_version(self): - latest_version = self.versions.latest() - self.saved_run = latest_version.saved_run - self.last_edited_by = latest_version.changed_by - self.title = latest_version.title - self.notes = latest_version.notes - self.public_access = latest_version.public_access - self.workspace_access = latest_version.workspace_access - self.photo_url = latest_version.photo_url - self.tags.set(latest_version.tags.all()) - - self.save() + with transaction.atomic(): + latest_version = self.versions.latest() + self.saved_run = latest_version.saved_run + self.last_edited_by = latest_version.changed_by + self.title = latest_version.title + self.notes = latest_version.notes + self.public_access = latest_version.public_access + self.workspace_access = latest_version.workspace_access + self.photo_url = latest_version.photo_url + self.tags.set(latest_version.tags.all()) + self.save()
436-444: Escape tag name to prevent HTML injection (keep icon markup).If tag names are ever user-controlled, rendering via
gui.htmlwithout escaping is unsafe.- def render(self, **props) -> None: - import gooey_gui as gui + def render(self, **props) -> None: + import gooey_gui as gui + from django.utils.html import escape @@ - with gui.div(className=className, **props), gui.link(to=self.get_app_url()): - gui.html(f"{self.icon} {self.name}" if self.icon else self.name) + with gui.div(className=className, **props), gui.link(to=self.get_app_url()): + safe_name = escape(self.name) + gui.html(f"{self.icon} {safe_name}" if self.icon else safe_name)Note: add the import as shown above.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bots/models/published_run.py(12 hunks)widgets/base_header.py(1 hunks)widgets/saved_workflow.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- widgets/base_header.py
🧰 Additional context used
🧬 Code graph analysis (2)
widgets/saved_workflow.py (1)
bots/models/published_run.py (2)
render(436-443)get_share_badge_html(320-334)
bots/models/published_run.py (3)
daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)bots/models/saved_run.py (1)
get_app_url(172-178)routers/root.py (1)
explore_page(248-260)
🪛 Ruff (0.13.1)
bots/models/published_run.py
461-467: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
468-468: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
469-469: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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 (3)
widgets/saved_workflow.py (2)
125-127: N+1 queries risk: prefetch tags on PublishedRun querysets.
published_run.tags.all()in a loop will N+1 withoutprefetch_related('tags')at callers.
31-31: Add a backward-compatible alias for the renamed kwarg or verify callersrg search for
hide_visibility_pillreturned no matches in this repo; absence of matches isn't proof external callers are updated — add a compatibility shim or confirm there are no external callsites.Suggested shim:
-def render_saved_workflow_preview( +def render_saved_workflow_preview( page_cls: typing.Union["BasePage", typing.Type["BasePage"]], published_run: PublishedRun, *, workflow_pill: str | None = None, show_workspace_author: bool = False, show_run_count: bool = False, - hide_access_level: bool = False, + hide_access_level: bool = False, + hide_visibility_pill: typing.Optional[bool] = None, hide_version_notes: bool = False, hide_last_editor: bool = False, ): + if hide_visibility_pill is not None: + hide_access_level = hide_visibility_pillbots/models/published_run.py (1)
4-4: Avoid unbounded/stale functools.cache for Tag.get_options; use Django cache + invalidation.Current
@cachereturns stale results and is unbounded. Prefer Django cache with TTL and signal-based invalidation.-from functools import cache +from django.core.cache import cache as django_cache @@ class Tag(models.Model): @@ - @classmethod - @cache - def get_options(cls) -> list["Tag"]: - return list(cls.objects.all()) + @classmethod + def get_options(cls) -> list["Tag"]: + cache_key = "tag:all-options:v1" + tags = django_cache.get(cache_key) + if tags is not None: + return tags + tags = list(cls.objects.all()) + django_cache.set(cache_key, tags, timeout=300) # 5m TTL + return tags + +# In the same module (bottom): +from django.db.models.signals import post_save, post_delete +from django.dispatch import receiver + +@receiver([post_save, post_delete], sender=Tag) +def _invalidate_tag_options_cache(**_): + django_cache.delete("tag:all-options:v1")Also applies to: 445-449
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
widgets/workflow_search.py (1)
523-531: LGTM!The new
get_field_from_orderingfunction correctly handles both string andOrderByinstances:
- For
OrderByinstances, it extracts the field name from the expression- For strings, it removes the leading
-to get the base field name- Provides appropriate error handling for unexpected types
Consider using a more specific exception class instead of a generic
ValueError:- case _: - raise ValueError(f"Invalid value: {value}") + case _: + raise TypeError(f"Expected str or OrderBy, got {type(value).__name__}: {value}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
bots/models/published_run.py(12 hunks)daras_ai_v2/base.py(18 hunks)daras_ai_v2/profiles.py(1 hunks)routers/account.py(3 hunks)widgets/explore.py(1 hunks)widgets/saved_workflow.py(6 hunks)widgets/workflow_search.py(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
widgets/explore.py (1)
widgets/workflow_search.py (1)
get_query_params(72-73)
daras_ai_v2/profiles.py (1)
widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
routers/account.py (4)
bots/models/published_run.py (1)
PublishedRun(124-359)daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)widgets/workflow_search.py (1)
get_query_params(72-73)widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
widgets/workflow_search.py (3)
bots/models/published_run.py (1)
PublishedRun(124-359)bots/models/workflow.py (1)
WorkflowAccessLevel(20-186)routers/root.py (1)
explore_page(248-260)
widgets/saved_workflow.py (6)
daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)widgets/demo_button.py (1)
get_demo_bots(41-50)widgets/workflow_search.py (2)
SearchFilters(63-73)get_query_params(72-73)bots/models/published_run.py (3)
PublishedRun(124-359)render(435-436)get_share_badge_html(319-333)bots/models/workflow.py (2)
page_cls(240-243)Workflow(189-257)routers/root.py (1)
explore_page(248-260)
daras_ai_v2/base.py (3)
bots/models/published_run.py (4)
Tag(420-452)get_options(440-441)render(435-436)PublishedRun(124-359)widgets/base_header.py (1)
render_breadcrumbs_with_author(41-75)widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
🪛 Ruff (0.13.1)
widgets/workflow_search.py
530-530: Avoid specifying long messages outside the exception class
(TRY003)
daras_ai_v2/base.py
484-484: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
bots/models/published_run.py
444-450: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
451-451: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
452-452: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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). (2)
- GitHub Check: Analyze (python)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (40)
widgets/saved_workflow.py (8)
15-15: LGTM!The import for
get_app_route_urlis correctly added to support the new tag pill linking functionality introduced in therender_pill_with_linkhelper function.
22-22: LGTM!The type hint import for
SearchFiltersis correctly added to support the newsearch_filtersparameter in the updated function signature.
25-37: LGTM!The signature change correctly removes the
page_clsparameter and internally computes it frompublished_run.workflow.page_cls. This improves the API by reducing coupling and making the function more self-contained.
66-70: LGTM!The call to
render_title_pillsproperly passes the new parameters including thesearch_filterswhich enables tag-based navigation links in the title pill area.
92-92: LGTM!The parameter rename from
hide_visibility_pilltohide_access_levelis consistent with the broader PR changes and better reflects the semantic meaning of hiding access level information.
100-155: LGTM!The updated
render_title_pillsfunction correctly:
- Accepts the new keyword-only parameters
show_workflow_pillandsearch_filters- Renders workflow pill when
show_workflow_pillis true with proper URL generation- Renders tag pills from
published_run.tags.all()with search-enabled links- Uses the new
render_pill_with_linkhelper for consistent pill renderingThe tag pill rendering logic properly iterates through tags and creates navigable links using updated search filters.
157-171: LGTM!The new
render_pill_with_linkhelper function is well-designed:
- Provides a consistent interface for rendering clickable pills
- Supports optional background styling with
text_bgparameter- Handles HTML escaping properly with the
unsafe_allow_htmlparameter- Uses proper CSS classes and responsive styling
214-214: LGTM!The parameter rename from
hide_visibility_pilltohide_access_levelis consistent throughout the file and better represents the actual functionality of hiding access level information in the footer.Also applies to: 275-276
widgets/explore.py (1)
76-76: LGTM!The change to use
new_filters.get_query_params()instead ofnew_filters.model_dump(exclude_defaults=True)is consistent with the new method introduced in theSearchFiltersclass and provides better encapsulation of query parameter handling.routers/account.py (3)
13-13: LGTM!Removing the unused
Workflowimport is a good cleanup that follows the signature changes inrender_saved_workflow_previewwhere the workflow data is now derived internally from thePublishedRun.
153-153: LGTM!The usage of
search_filters.get_query_params()is consistent with the new method added to theSearchFiltersclass and provides better encapsulation.
352-352: LGTM!The updated call to
render_saved_workflow_previewcorrectly uses the new signature with:
pras the first parameter instead of the old(page_cls, pr, workflow_pill)patternshow_workflow_pill=Trueto enable workflow pill renderingdaras_ai_v2/profiles.py (1)
268-268: LGTM!The updated call to
render_saved_workflow_previewcorrectly uses the new signature by passingprdirectly and enabling the workflow pill withshow_workflow_pill=True. This aligns with the broader refactoring to simplify the API.widgets/workflow_search.py (7)
11-11: LGTM!The import of
OrderByfrom Django ORM is correctly added to support the newget_field_from_orderingfunction that handles both string andOrderByinstances.
18-18: LGTM!Removing the unused
Workflowimport is good cleanup, as the workflow data is now handled differently in the rendering pipeline.
72-74: LGTM!The new
get_query_params()method provides better encapsulation for converting search filters to query parameters. The implementation usingmodel_dump(exclude_defaults=True)is appropriate for excluding empty/default values from the query string.
180-180: LGTM!Using
search_filters.get_query_params()instead of direct model dumping provides better encapsulation and consistency with the new API pattern.
383-391: LGTM!The updated call to
render_saved_workflow_previewcorrectly:
- Uses the new signature with
pras the first parameter- Enables workflow pill display with
show_workflow_pill=True- Uses the renamed
hide_access_levelparameter instead ofhide_visibility_pill- Passes
search_filtersfor tag navigation functionalityThis aligns with the broader refactoring to support tag-based navigation and improved preview rendering.
406-431: LGTM!The refactored sorting logic improves code organization by:
- Collecting all field names into a
fieldstuple- Using a single
returnstatement withorder_by(*fields).distinct("id", ...)- Avoiding early returns from each case
The distinct clause correctly uses
get_field_from_ordering(f)to extract field names for both string andOrderByinstances.
513-513: LGTM!Adding
"tags__name"to the search vector enables search functionality across tag names, which is essential for the tag-based workflow discovery feature introduced in this PR.bots/models/published_run.py (9)
4-4: Replace functools.cache with TTL cache + invalidation.The
@cachedecorator creates an unbounded cache that never expires, which can return stale tag data when tags are created, updated, or deleted. This is particularly problematic for a dynamic system where tags can be managed through admin interfaces.Switch to Django's cache framework with proper TTL and invalidation:
-from functools import cache +from django.core.cache import cache as django_cache +from django.db.models.signals import post_save, post_delete +from django.dispatch import receiver class Tag(models.Model): # ... other fields ... - @classmethod - @cache - def get_options(cls) -> list["Tag"]: - return list(cls.objects.all()) + @classmethod + def get_options(cls) -> list["Tag"]: + cache_key = "tag:all-options:v1" + tags = django_cache.get(cache_key) + if tags is not None: + return tags + tags = list(cls.objects.all()) + django_cache.set(cache_key, tags, timeout=300) # 5 min TTL + return tags +@receiver([post_save, post_delete], sender=Tag) +def _invalidate_tag_options_cache(**_): + django_cache.delete("tag:all-options:v1")Also applies to: 439-442
38-54: LGTM!The updated queryset methods correctly accept the new
tagsparameter and properly forward it through the creation and versioning pipeline. The parameter typing aslist[Tag] | None = Noneis appropriate and handles the optional nature of tags.Also applies to: 69-100
155-155: LGTM!The tags field is properly defined as a many-to-many relationship with appropriate configuration:
- References the
Tagmodel correctly- Uses descriptive
related_name="published_runs"- Marked as
blank=Truefor optional tagging
248-248: LGTM!The
duplicatemethod correctly propagates tags by passingtags=list(self.tags.all())to thecreate_with_versioncall, ensuring tags are preserved when duplicating published runs.
267-267: LGTM!The
add_versionmethod correctly handles tags by:
- Accepting the new
tagsparameter with proper typinglist[Tag] | None = None- Defaulting to empty list when
tags is None- Adding tags to the version with
version.tags.add(*tags)within the transactionAlso applies to: 275-277, 291-291
306-306: LGTM!The
update_fields_to_latest_versionmethod correctly synchronizes tags from the latest version to the published run usingself.tags.set(latest_version.tags.all()). This ensures tags are kept in sync during version updates.Note: The transaction handling concern from previous reviews has been addressed as this method is called within the transaction block in
add_version.
394-396: LGTM!The tags field on
PublishedRunVersionis correctly defined with:
- Many-to-many relationship to
Tag- Descriptive
related_name="published_run_versions"blank=Truefor optional tagging at the version level
411-417: LGTM!The
TagCategoryenum provides a well-structured set of categories for organizing tags:
- Covers common categorization needs (app, industry, medium, language, region)
- Includes "other" as a catch-all default
- Uses clear, descriptive names
420-453: LGTM!The
Tagmodel is well-designed with appropriate fields:
namewith reasonable 64-character limit- Optional
iconfor visual representationcategorywith proper choices and defaultfeatured_priorityfor ordering with helpful text- Standard timestamp fields
The
render()method provides a clean way to display tags with icons, and the Meta class enforces proper uniqueness and ordering.The case-insensitive unique constraint using
Upper("name")is the correct Django pattern, and the ordering byfeatured_priorityandnameprovides logical display order.daras_ai_v2/base.py (11)
38-38: LGTM!The import for
Tagfrombots.models.published_runis correctly added to support the new tag functionality in published runs and the publish form UI.
425-487: LGTM!The header rendering refactor improves organization by:
- Adding early returns for non-Run/Preview tabs to reduce nesting
- Grouping related header elements (image, title, social buttons) logically
- Creating distinct rendering paths for different tab types
- Maintaining responsive design considerations
The conditional rendering and responsive image handling are well-implemented.
777-793: Fix multiselect robustness: guard against stale/deleted PKs and drop None option.The current multiselect implementation has two issues:
- If a tag is deleted while cached in session state,
[options[pk] for pk in tag_pks]will raiseKeyErrorallow_none=Truecan introduceNoneinto tag selections- with gui.div(className="d-flex mb-3 align-items-center gap-2"): + with gui.div(className="d-flex mb-3 align-items-center gap-2"): options = {t.pk: t for t in Tag.get_options()} gui.write("Tags", className="fs-5 mb-3") if not isinstance(gui.session_state.get("published_run_tags"), list): gui.session_state["published_run_tags"] = [ tag.pk for tag in pr.tags.all() ] + else: + # sanitize any stale/deleted PKs in session state + gui.session_state["published_run_tags"] = [ + pk for pk in gui.session_state["published_run_tags"] if pk in options + ] with gui.div(className="flex-grow-1"): tag_pks = gui.multiselect( label="", options=options, format_func=lambda pk: options[pk].render(), key="published_run_tags", - allow_none=True, + allow_none=False, ) - tags = [options[pk] for pk in tag_pks] + tags = [options[pk] for pk in tag_pks if pk in options]
944-952: Fix unhashable Tag model comparison.The code
set(published_run.tags.all()) != set(tags)will raiseTypeError: unhashable type: 'Tag'because Django model instances are not hashable and cannot be used in sets.- tags: list[Tag], + tags: list[Tag] | None, ): return ( published_run.title != title or published_run.notes != notes or published_run.saved_run != saved_run or published_run.photo_url != photo_url - or set(published_run.tags.all()) != set(tags) + or set(published_run.tags.values_list("pk", flat=True)) + != {t.pk for t in (tags or [])} )
851-851: LGTM!The tag parameters are correctly passed to both
create_published_runandadd_versionmethods, enabling proper tag propagation through the publish flow.Also applies to: 866-866
981-983: LGTM!The button label logic correctly distinguishes between "Duplicate" for the latest version and "Save as New" for older versions, providing clear user intent indication.
1026-1026: LGTM!Tag propagation is correctly implemented throughout the application:
- In saved options modal for duplication
- In admin options for root workflow updates
- In the
create_published_runmethod signature and implementation- In the publish and redirect flow
All instances properly pass
tags=list(published_run.tags.all())to preserve tag associations.Also applies to: 1127-1127, 1543-1543, 1555-1555, 1959-1959
1036-1036: LGTM!The version history section header using
icons.timeprovides clear visual context for the chronological listing of workflow versions.
1509-1513: LGTM!The
prefetch_related("tags")optimization inget_pr_from_example_idaddresses the N+1 query issue flagged in past reviews, ensuring efficient tag data loading when retrieving published runs.
2179-2179: LGTM!The parameter rename to
hide_access_level=Truein the examples tab is consistent with the broader refactoring and correctly hides access level information in the example listings.
2210-2210: LGTM!The saved tab correctly passes
published_runsdirectly torender_saved_workflow_previewsince the function signature was updated to accept the list of published runs with default parameters.
nikochiko
force-pushed
the
tags
branch
2 times, most recently
from
4e08d70 to
f12c9de
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
bots/admin.py (1)
970-983: Optional: annotate ModelAdmin class attributes to silence Ruff RUF012Ruff flags list_display/search_fields/list_filter/readonly_fields as mutable class attributes. Either ignore for admin modules or annotate with ClassVar.
+from typing import ClassVar @@ class TagAdmin(admin.ModelAdmin): - list_display = [ + list_display: ClassVar[list[str]] = [ "name", "category", "icon", "featured_priority", "created_at", "updated_at", ] - search_fields = ["name", "category"] - list_filter = ["category"] - readonly_fields = ["created_at", "updated_at"] + search_fields: ClassVar[list[str]] = ["name", "category"] + list_filter: ClassVar[list[str]] = ["category"] + readonly_fields: ClassVar[list[str]] = ["created_at", "updated_at"]widgets/workflow_search.py (2)
430-433: Fix DISTINCT ON usage to avoid ordering mismatch and PostgreSQL constraintsUsing distinct("id", …fields) with order_by(*fields) risks:
- PostgreSQL requiring DISTINCT ON fields to be the leftmost ORDER BY terms.
- Unintended ordering (or errors) because "id" isn’t in order_by.
- Over-constraining DISTINCT ON; you typically only need DISTINCT ON (id) to collapse M2M duplicates.
Given we select only PublishedRun columns, plain DISTINCT will dedupe rows correctly and preserve ordering.
- return qs.order_by(*fields).distinct( - "id", - *(get_field_from_ordering(f) for f in fields), - ) + # Remove M2M duplicates while preserving desired ordering + return qs.order_by(*fields).distinct()If you must use DISTINCT ON, ensure ORDER BY begins with the same fields (in the same order), which would complicate the desired sort semantics.
525-533: Minor: guard for non-F expressions in get_field_from_orderingvalue.expression.name assumes an F() expression. If we ever pass a different expression type, this will raise. Not urgent, but worth keeping in mind.
daras_ai_v2/base.py (2)
777-793: Make tag multiselect robust to None and stale PKs; include existing tags in options
- allow_none=True can introduce None in selections.
- Mapping selected PKs directly via options[pk] can KeyError if a tag was deleted or is not in Tag.get_options().
- Also include current PR’s tags so existing non-featured tags remain visible/editable.
- with gui.div(className="d-flex mb-3 align-items-center gap-2"): + with gui.div(className="d-flex mb-3 align-items-center gap-2"): options = {t.pk: t for t in Tag.get_options()} + # Ensure currently-selected tags (possibly non-featured) remain selectable + options |= {t.pk: t for t in pr.tags.all()} gui.write("Tags", className="fs-5 mb-3") if not isinstance(gui.session_state.get("published_run_tags"), list): gui.session_state["published_run_tags"] = [ tag.pk for tag in pr.tags.all() ] with gui.div(className="flex-grow-1"): tag_pks = gui.multiselect( label="", options=options, format_func=lambda pk: options[pk].render(), key="published_run_tags", - allow_none=True, + allow_none=False, ) - tags = [options[pk] for pk in tag_pks] + # Guard against stale/unknown PKs + tags = [options[pk] for pk in tag_pks if pk in options]
463-486: Nit: make short-circuit clearerUse a clearer conditional when passing current_workspace.
- current_workspace=( - self.is_logged_in() and self.current_workspace or None - ), + current_workspace=(self.current_workspace if self.is_logged_in() else None),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
bots/admin.py(2 hunks)bots/migrations/0109_tag_publishedrun_tags_publishedrunversion_tags.py(1 hunks)bots/models/published_run.py(12 hunks)daras_ai_v2/base.py(20 hunks)daras_ai_v2/profiles.py(1 hunks)glossary_resources/tests.py(0 hunks)routers/account.py(4 hunks)scripts/init_tags.py(1 hunks)scripts/run-prod.sh(1 hunks)widgets/base_header.py(4 hunks)widgets/explore.py(1 hunks)widgets/saved_workflow.py(6 hunks)widgets/workflow_search.py(6 hunks)
💤 Files with no reviewable changes (1)
- glossary_resources/tests.py
🚧 Files skipped from review as they are similar to previous changes (5)
- scripts/run-prod.sh
- daras_ai_v2/profiles.py
- widgets/explore.py
- scripts/init_tags.py
- routers/account.py
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-09-24T07:23:39.496Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:23:39.496Z
Learning: In `widgets/workflow_search.py`, the `render_search_results()` function QuerySet at line 375 needs `.select_related("workspace", "saved_run", "last_edited_by").prefetch_related("tags", "versions")` to avoid N+1 queries when rendering search results with published run previews. Note: `created_by` should not be included as it's not used in workflow rendering.
Applied to files:
widgets/saved_workflow.pydaras_ai_v2/base.pywidgets/workflow_search.py
📚 Learning: 2025-09-24T07:06:56.177Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:06:56.177Z
Learning: In `daras_ai_v2/base.py`, the `_saved_tab()` method QuerySet at line 2199 needs `.select_related('saved_run', 'workspace', 'last_edited_by').prefetch_related('tags', 'versions')` to avoid N+1 queries when rendering published runs.
Applied to files:
widgets/saved_workflow.pydaras_ai_v2/base.py
📚 Learning: 2025-09-24T07:06:56.178Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:06:56.178Z
Learning: In `daras_ai_v2/base.py`, the `_examples_tab()` method QuerySet at line 2164 needs `.select_related('saved_run', 'workspace', 'last_edited_by').prefetch_related('tags', 'versions')` to optimize database queries for published run rendering.
Applied to files:
widgets/saved_workflow.pydaras_ai_v2/base.py
📚 Learning: 2025-09-24T07:28:14.980Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:28:14.980Z
Learning: In Django, methods like `.latest()`, `.first()`, and `.filter()` on prefetched related managers bypass the prefetch cache and hit the database because they modify the QuerySet with new conditions (ordering, limiting, filtering). Only accessing `.all()` on prefetched relationships uses the cached data. The `.latest()` method specifically adds ORDER BY and LIMIT clauses that create a different SQL query than what was prefetched.
Applied to files:
widgets/saved_workflow.py
📚 Learning: 2025-09-24T07:28:14.980Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:28:14.980Z
Learning: The PublishedRunVersion model has `get_latest_by = "created_at"` defined in its Meta class, which makes `.latest()` queries efficient. However, `prefetch_related('versions')` still doesn't help with `versions.latest()` calls because Django generates a separate `ORDER BY ... LIMIT 1` query that bypasses the prefetch cache. Since `versions.latest()` is only used in 3 locations in the codebase, the N+1 impact is minimal compared to tags and foreign key access.
Applied to files:
widgets/saved_workflow.py
📚 Learning: 2025-09-24T07:07:36.073Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: bots/models/published_run.py:4-4
Timestamp: 2025-09-24T07:07:36.073Z
Learning: In FastAPI/uvicorn deployments, functools.cache persists for the entire process lifetime (not per-request), meaning cached data remains until the uvicorn process is restarted.
Applied to files:
bots/models/published_run.py
🧬 Code graph analysis (5)
widgets/base_header.py (6)
daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)widgets/saved_workflow.py (1)
render_pill_with_link(157-170)widgets/author.py (1)
render_author_as_breadcrumb(12-49)widgets/workflow_search.py (2)
SearchFilters(63-73)get_query_params(72-73)daras_ai_v2/breadcrumbs.py (1)
TitleBreadCrumbs(16-47)routers/root.py (1)
explore_page(248-260)
widgets/saved_workflow.py (6)
daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)widgets/demo_button.py (1)
get_demo_bots(41-50)widgets/workflow_search.py (2)
SearchFilters(63-73)get_query_params(72-73)bots/models/published_run.py (3)
PublishedRun(123-358)render(434-435)get_share_badge_html(318-332)bots/models/workflow.py (5)
page_cls(241-244)Workflow(189-258)short_slug(227-228)emoji(236-238)short_title(231-233)routers/root.py (1)
explore_page(248-260)
daras_ai_v2/base.py (4)
bots/models/published_run.py (5)
Tag(419-450)get_options(438-439)render(434-435)PublishedRun(123-358)approved_example_q(353-358)widgets/base_header.py (1)
render_breadcrumbs_with_author(47-85)daras_ai_v2/grid_layout_widget.py (1)
grid_layout(25-38)widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
widgets/workflow_search.py (3)
bots/models/published_run.py (1)
PublishedRun(123-358)bots/models/workflow.py (1)
WorkflowAccessLevel(20-186)widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
bots/admin.py (1)
bots/models/published_run.py (1)
Tag(419-450)
🪛 Ruff (0.13.1)
bots/migrations/0109_tag_publishedrun_tags_publishedrunversion_tags.py
9-11: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
13-80: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
daras_ai_v2/base.py
484-484: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
widgets/workflow_search.py
532-532: Avoid specifying long messages outside the exception class
(TRY003)
bots/models/published_run.py
442-448: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
449-449: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
450-450: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
bots/admin.py
972-979: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
980-980: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
981-981: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
982-982: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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 (13)
bots/admin.py (1)
30-31: Registering Tag in admin: looks goodAdmin surface and fields make sense for Tag. No functional issues spotted.
Also applies to: 970-983
bots/migrations/0109_tag_publishedrun_tags_publishedrunversion_tags.py (1)
13-80: Migration design LGTM
- Tag model, M2M fields, unique Upper(name), ordering and index are appropriate.
- Note: Ruff RUF012 on Migration class attributes can be safely ignored for migrations.
If you prefer a more conventional import for readability, consider:
-import django.db.models.functions.text +from django.db.models.functions import Upper @@ - django.db.models.functions.text.Upper("name"), + Upper("name"),This is optional; current code works.
widgets/workflow_search.py (1)
375-377: Good: prefetch/select related to avoid N+1sPrefetching tags/versions and selecting workspace/last_edited_by/saved_run aligns with our earlier optimization notes. Thanks for applying the learning.
widgets/base_header.py (1)
76-86: Tag pills in header: nice UXLinking tags to Explore via SearchFilters is clean. Ensure the PR instance used here has tags prefetched; on example pages BasePage.get_pr_from_example_id now does prefetch, so we’re good.
Confirm there’s no code path calling render_breadcrumbs_with_author with a PR lacking tag prefetch. If any, add .prefetch_related("tags") at the call site.
widgets/saved_workflow.py (3)
64-71: Title pills integration is solidDeriving page_cls internally and rendering workflow/tag pills with SearchFilters keeps the API minimal and consistent.
147-154: N+1-safe tag rendering (given callers prefetch tags)Calls to published_run.tags.all() are fine since upstream QuerySets now prefetch('tags'). No changes needed.
209-277: Correct rename: hide_visibility_pill → hide_access_levelThe rename propagates correctly; footer logic matches the new parameter.
bots/models/published_run.py (3)
247-248: Duplication forwards tags correctlyForwarding tags in duplicate() is correct and matches the new API.
274-291: Version creation and tag propagation look correctDefaulting tags to [] avoids mutable-default pitfalls; adding tags via version.tags.add(*tags) within atomic is good.
305-307: Atomicity of tags.set + saveupdate_fields_to_latest_version is invoked from within an atomic() in add_version(); keep it that way for any other future callers.
daras_ai_v2/base.py (3)
1510-1514: Good: prefetch tags on example PR fetchThis supports tag pills in headers without N+1s.
2165-2171: Examples tab: solid queryset shapingselect_related + prefetch_related aligns with performance goals.
2204-2210: Saved tab: solid queryset shapingSame here; good balance for rendering many previews.
…rsion refactor: remove outdated comment for mobile only feat: add tag name to searched attributes
in saved options
…stances Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bots/models/published_run.py (1)
274-291: Fix empty-tag handling to avoid runtime crash.Line 290:
version.tags.add(*tags)raisesTypeErrorwhenevertagsis empty—exactly what happens for most calls where no tags are supplied (defaulttags=None→[], or an explicit empty list). That breakscreate_with_version/add_versionflows as soon as somebody publishes without tags. Switch to.set()(which accepts empty iterables) so we can safely handle both populated and empty tag lists.- if tags is None: - tags = [] + tag_iterable = [] if tags is None else tags with transaction.atomic(): version = PublishedRunVersion( published_run=self, version_id=get_random_doc_id(), saved_run=saved_run, changed_by=user, title=title, notes=notes, public_access=public_access, workspace_access=workspace_access, change_notes=change_notes, photo_url=photo_url, ) version.save() - version.tags.add(*tags) + version.tags.set(tag_iterable) self.update_fields_to_latest_version()
♻️ Duplicate comments (1)
daras_ai_v2/base.py (1)
778-793: Unresolved: Multiselect robustness issues remain.The past review comment flagged:
allow_none=Truecan introduceNoneinto selections (line 791)- Missing guard against stale/deleted tag PKs in session state
- Direct PK mapping
[options[pk] for pk in tag_pks]can raiseKeyErrorApply the previously suggested fix:
with gui.div(className="d-flex mb-3 align-items-center gap-2"): options = {t.pk: t for t in Tag.get_options()} gui.write("Tags", className="fs-5 mb-3") if not isinstance(gui.session_state.get("published_run_tags"), list): gui.session_state["published_run_tags"] = [ tag.pk for tag in pr.tags.all() ] + else: + # sanitize any stale/deleted PKs in session state + gui.session_state["published_run_tags"] = [ + pk for pk in gui.session_state["published_run_tags"] if pk in options + ] with gui.div(className="flex-grow-1"): tag_pks = gui.multiselect( label="", options=options, format_func=lambda pk: options[pk].render(), key="published_run_tags", - allow_none=True, + allow_none=False, ) - tags = [options[pk] for pk in tag_pks] + tags = [options[pk] for pk in tag_pks if pk in options]
🧹 Nitpick comments (1)
widgets/workflow_search.py (1)
385-387: Consider removing"versions"from prefetch.Based on learnings,
prefetch_related("versions")doesn't help withpublished_run.versions.latest()calls because Django generates a separateORDER BY ... LIMIT 1query that bypasses the prefetch cache. Sinceversions.latest()is only used in 3 locations in the codebase, the N+1 impact is minimal.Apply this diff if you want to optimize:
- qs = qs.prefetch_related("tags", "versions").select_related( + qs = qs.prefetch_related("tags").select_related( "workspace", "last_edited_by", "saved_run" )Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
bots/admin.py(2 hunks)bots/migrations/0111_tag_publishedrun_tags_publishedrunversion_tags.py(1 hunks)bots/models/published_run.py(12 hunks)daras_ai_v2/base.py(20 hunks)daras_ai_v2/profiles.py(1 hunks)glossary_resources/tests.py(0 hunks)routers/account.py(4 hunks)scripts/init_tags.py(1 hunks)scripts/run-prod.sh(1 hunks)widgets/base_header.py(4 hunks)widgets/explore.py(1 hunks)widgets/saved_workflow.py(6 hunks)widgets/workflow_search.py(7 hunks)
💤 Files with no reviewable changes (1)
- glossary_resources/tests.py
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/init_tags.py
- scripts/run-prod.sh
- routers/account.py
- widgets/explore.py
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-24T07:07:36.097Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: bots/models/published_run.py:4-4
Timestamp: 2025-09-24T07:07:36.097Z
Learning: In FastAPI/uvicorn deployments, functools.cache persists for the entire process lifetime (not per-request), meaning cached data remains until the uvicorn process is restarted.
Applied to files:
bots/models/published_run.py
📚 Learning: 2025-09-24T07:23:39.522Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:23:39.522Z
Learning: In `widgets/workflow_search.py`, the `render_search_results()` function QuerySet at line 375 needs `.select_related("workspace", "saved_run", "last_edited_by").prefetch_related("tags", "versions")` to avoid N+1 queries when rendering search results with published run previews. Note: `created_by` should not be included as it's not used in workflow rendering.
Applied to files:
widgets/saved_workflow.pywidgets/workflow_search.py
📚 Learning: 2025-09-24T07:06:56.185Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:06:56.185Z
Learning: In `daras_ai_v2/base.py`, the `_saved_tab()` method QuerySet at line 2199 needs `.select_related('saved_run', 'workspace', 'last_edited_by').prefetch_related('tags', 'versions')` to avoid N+1 queries when rendering published runs.
Applied to files:
widgets/saved_workflow.pydaras_ai_v2/base.py
📚 Learning: 2025-09-24T07:06:56.185Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:06:56.185Z
Learning: In `daras_ai_v2/base.py`, the `_examples_tab()` method QuerySet at line 2164 needs `.select_related('saved_run', 'workspace', 'last_edited_by').prefetch_related('tags', 'versions')` to optimize database queries for published run rendering.
Applied to files:
widgets/saved_workflow.pydaras_ai_v2/base.py
📚 Learning: 2025-09-24T07:28:15.005Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:28:15.005Z
Learning: The PublishedRunVersion model has `get_latest_by = "created_at"` defined in its Meta class, which makes `.latest()` queries efficient. However, `prefetch_related('versions')` still doesn't help with `versions.latest()` calls because Django generates a separate `ORDER BY ... LIMIT 1` query that bypasses the prefetch cache. Since `versions.latest()` is only used in 3 locations in the codebase, the N+1 impact is minimal compared to tags and foreign key access.
Applied to files:
widgets/saved_workflow.py
🧬 Code graph analysis (6)
daras_ai_v2/profiles.py (1)
widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
widgets/base_header.py (6)
daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)widgets/saved_workflow.py (1)
render_pill_with_link(157-170)widgets/author.py (1)
render_author_as_breadcrumb(12-49)widgets/workflow_search.py (2)
SearchFilters(68-83)get_query_params(82-83)bots/models/published_run.py (2)
PublishedRun(123-358)render(434-435)routers/root.py (1)
explore_page(248-260)
widgets/saved_workflow.py (6)
daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)widgets/demo_button.py (1)
get_demo_bots(41-50)widgets/workflow_search.py (2)
SearchFilters(68-83)get_query_params(82-83)bots/models/published_run.py (3)
PublishedRun(123-358)render(434-435)get_share_badge_html(318-332)bots/models/workflow.py (5)
page_cls(241-244)Workflow(189-258)short_slug(227-228)emoji(236-238)short_title(231-233)routers/root.py (1)
explore_page(248-260)
daras_ai_v2/base.py (5)
bots/models/published_run.py (5)
Tag(419-450)get_options(438-439)render(434-435)PublishedRun(123-358)approved_example_q(353-358)widgets/author.py (1)
render_author_from_workspace(52-82)widgets/base_header.py (2)
render_header_title(18-26)render_breadcrumbs_with_author(47-85)daras_ai_v2/grid_layout_widget.py (1)
grid_layout(25-38)widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
widgets/workflow_search.py (3)
bots/models/published_run.py (1)
PublishedRun(123-358)bots/models/workflow.py (1)
WorkflowAccessLevel(20-186)widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
bots/admin.py (1)
bots/models/published_run.py (1)
Tag(419-450)
🪛 Ruff (0.13.3)
bots/models/published_run.py
442-448: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
449-449: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
450-450: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
bots/migrations/0111_tag_publishedrun_tags_publishedrunversion_tags.py
9-11: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
13-80: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
daras_ai_v2/base.py
485-485: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
widgets/workflow_search.py
537-537: Avoid specifying long messages outside the exception class
(TRY003)
bots/admin.py
983-990: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
991-991: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
992-992: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
993-993: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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 (17)
widgets/workflow_search.py (5)
82-83: LGTM: Clean query param generation.The new
get_query_params()method properly leverages Pydantic'smodel_dump(exclude_defaults=True)to generate clean query parameters, avoiding empty/default values in URLs.
395-403: LGTM: Rendering parameters aligned with new signature.The updated
render_saved_workflow_previewcall correctly passes:
show_workflow_pill=Truefor search resultshide_access_level=True(renamed fromhide_visibility_pill)search_filtersfor tag pill navigation
418-445: LGTM: Improved sorting logic with better separation of concerns.The refactored
build_sort_filtercorrectly:
- Accumulates ordering fields into a tuple
- Uses the new
get_field_from_orderinghelper to extract field names fordistinct()- Maintains consistent logic across all sort options
511-521: LGTM: Tag names now searchable.Adding
"tags__name"to the search vector with weight "B" enables users to search workflows by their tags, which is a valuable UX enhancement. The weight is appropriate, matching workspace and author fields.
530-537: LGTM: Clean helper for ordering field extraction.The
get_field_from_orderingfunction correctly extracts field names from bothOrderByobjects and string values for use indistinct()clauses.Note: The TRY003 Ruff warning is a false positive—the error message is concise and appropriate for inline use.
widgets/saved_workflow.py (5)
15-15: LGTM: Clean import additions for routing and type hints.The imports support:
get_app_route_urlfor building navigable pill linksSearchFilterstype hints viaTYPE_CHECKINGto avoid circular importsAlso applies to: 22-22
25-36: LGTM: Simplified signature with internal page_cls derivation.The refactored signature:
- Accepts
published_rundirectly instead of separatepage_cls/workflowparameters- Derives
page_clsinternally (line 36), simplifying caller code- Adds parameters for pill rendering (
show_workflow_pill,search_filters)- Renames
hide_visibility_pill→hide_access_levelfor clarity
100-154: LGTM: Enhanced pill rendering with navigation.The refactored
render_title_pillscorrectly:
- Renders workflow pill when requested, linking to filtered explore page
- Renders tag pills linking to search queries
- Uses
tag.render()for consistent tag displayImportant: Ensure
published_run.tagsis prefetched viaprefetch_related("tags")at call sites to avoid N+1 queries (line 147).
157-170: LGTM: Clean reusable pill component.The
render_pill_with_linkhelper provides a well-structured abstraction for rendering clickable pills with:
- Proper className construction
- Configurable HTML escaping
- Flexible styling via
text_bgand**props
214-214: LGTM: Clearer parameter naming.Renaming
hide_visibility_pill→hide_access_levelbetter describes the parameter's purpose: controlling the visibility of the access level badge in the footer.Also applies to: 275-276
daras_ai_v2/base.py (7)
426-487: LGTM: Enhanced header layout for tags and breadcrumbs.The header refactoring correctly:
- Adds workspace author display for Saved/History tabs
- Restructures Run/Preview header with improved image and breadcrumb layout
- Accommodates tag pill rendering via
render_breadcrumbs_with_authorNote: The RUF021 warning at line 485 is a false positive—the precedence is clear from the logical flow.
945-954: LGTM: Tag comparison correctly uses PKs.The comparison logic correctly:
- Accepts
tags: list[Tag] | Noneparameter- Compares sets of primary keys instead of model instances
- Handles
Nonetags gracefully with(tags or [])This addresses the past review concern about unhashable model instances.
983-985: LGTM: Clearer labeling for duplication actions.The conditional label correctly distinguishes:
- "Duplicate" for the latest version (user wants a copy)
- "Save as New" for historical versions (user wants to fork from history)
1511-1514: LGTM: Prefetch optimization for example PR retrieval.Correctly prefetches
"tags"and includes necessaryselect_relatedfor efficient rendering. Consider removing"versions"if not needed.Based on learnings.
2175-2180: LGTM: Query optimization for examples tab.The QuerySet correctly:
- Prefetches
"tags"for tag pill rendering- Uses
select_related("saved_run", "workspace", "last_edited_by")for FK access- Excludes
"created_by"(not used in workflow rendering)Optional: Consider removing
"versions"fromprefetch_relatedsinceversions.latest()bypasses the prefetch cache (per learnings).Based on learnings.
2214-2219: LGTM: Query optimization for saved tab.The QuerySet correctly:
- Prefetches
"tags"to avoid N+1 queries when rendering tag pills- Uses
select_related("saved_run", "workspace", "last_edited_by")for foreign key access- Excludes
"created_by"(not used in rendering)Optional: Consider removing
"versions"fromprefetch_relatedsinceversions.latest()bypasses the prefetch cache (per learnings).Based on learnings.
1028-1028: LGTM: Complete tag flow integration.Tags are correctly propagated through all creation, duplication, and versioning flows:
- Save as new workflow (line 1028)
- Update root workflow as admin (line 1129)
- Create published run (lines 1545, 1557)
- Publish after login (line 1970)
Also applies to: 1129-1129, 1545-1557, 1970-1970
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
widgets/workflow_search.py (1)
551-558: LGTM! Field extraction helper is correctly implemented.The function properly handles both string and
OrderBytypes, correctly extracting field names for the distinct clause. The logic for stripping the "-" prefix and accessingOrderBy.expression.nameis correct.Optional: Consider a custom exception class.
The static analysis tool suggests using a custom exception class instead of specifying the message inline. While the current implementation is acceptable, you could optionally create a custom exception:
class InvalidOrderingValueError(ValueError): """Raised when an ordering value is neither a string nor OrderBy.""" def get_field_from_ordering(value: str | OrderBy) -> str: match value: case OrderBy(): return value.expression.name case str(): return value.lstrip("-") case _: raise InvalidOrderingValueError(f"Invalid value: {value}")This would address the Ruff TRY003 hint, though it's a minor improvement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
widgets/explore.py(2 hunks)widgets/workflow_search.py(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T07:23:39.522Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:23:39.522Z
Learning: In `widgets/workflow_search.py`, the `render_search_results()` function QuerySet at line 375 needs `.select_related("workspace", "saved_run", "last_edited_by").prefetch_related("tags", "versions")` to avoid N+1 queries when rendering search results with published run previews. Note: `created_by` should not be included as it's not used in workflow rendering.
Applied to files:
widgets/workflow_search.py
🧬 Code graph analysis (2)
widgets/workflow_search.py (5)
bots/models/published_run.py (4)
PublishedRun(123-358)Tag(419-450)get_options(438-439)render(434-435)daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)widgets/saved_workflow.py (2)
render_pill_with_link(157-170)render_saved_workflow_preview(25-97)routers/root.py (1)
explore_page(248-260)widgets/explore.py (1)
render(55-93)
widgets/explore.py (1)
widgets/workflow_search.py (2)
render_search_suggestions(276-294)get_query_params(82-83)
🪛 Ruff (0.13.3)
widgets/workflow_search.py
558-558: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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)
widgets/explore.py (1)
18-18: LGTM! Tag suggestions and query parameter refactor look good.The integration of
render_search_suggestionsprovides a clean UX for discovering tags when the search bar is empty, and the refactor to useget_query_params()improves maintainability by centralizing query parameter construction logic.Also applies to: 71-72, 79-79
widgets/workflow_search.py (7)
11-11: LGTM! Import updates align with tag functionality.The import changes correctly support the new tag-based features:
OrderByfor the ordering helper,Tagmodel for tag operations, and widget rendering functions for tag pills.Also applies to: 19-19, 24-24
82-83: LGTM! Good encapsulation of query parameter logic.The
get_query_params()method provides a clean abstraction for converting search filters to query parameters, improving maintainability and making the code more testable.
276-294: LGTM! Tag suggestions rendering is well-implemented.The function clearly renders featured tags as clickable pills that link to filtered searches. The use of
model_copy(update={...})ensures search filters are properly updated without mutation, and the styling classes provide good visual consistency.
406-408: LGTM! Query optimization correctly applied.The addition of
prefetch_related("tags", "versions")and inclusion of"last_edited_by"inselect_relatedproperly optimizes the QuerySet to avoid N+1 queries when rendering search results with tag information. Based on learnings.
418-418: LGTM! Parameter updates align with refactored signature.The updated parameters (
show_workflow_pill=True,hide_access_level=True, and passingsearch_filters) appropriately configure the workflow preview rendering for the search results context.Also applies to: 422-423
443-466: LGTM! Sort filter refactor improves consistency.The refactored implementation unifies all sort cases to follow the same pattern, accumulating fields into a tuple and applying the distinct clause uniformly. The use of
get_field_from_orderingcorrectly handles both string andOrderBytypes for the distinct clause.
535-535: LGTM! Search vector correctly expanded for tags.Adding
"tags__name"to the search vector with weight "B" enables users to search workflows by tag names, correctly implementing the tag search functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
widgets/workflow_search.py (1)
545-552: Remove unused functionget_field_from_ordering
The function in widgets/workflow_search.py (lines 545–552) isn’t referenced anywhere and can be deleted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bots/admin.py(2 hunks)widgets/workflow_search.py(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T07:23:39.522Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:23:39.522Z
Learning: In `widgets/workflow_search.py`, the `render_search_results()` function QuerySet at line 375 needs `.select_related("workspace", "saved_run", "last_edited_by").prefetch_related("tags", "versions")` to avoid N+1 queries when rendering search results with published run previews. Note: `created_by` should not be included as it's not used in workflow rendering.
Applied to files:
widgets/workflow_search.py
🧬 Code graph analysis (2)
widgets/workflow_search.py (3)
bots/models/published_run.py (4)
PublishedRun(123-358)Tag(419-450)get_options(438-439)render(434-435)daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
bots/admin.py (3)
bots/models/published_run.py (2)
Tag(419-450)PublishedRun(123-358)bots/admin_links.py (2)
change_obj_url(29-39)list_related_html_url(42-89)bots/models/saved_run.py (1)
SavedRun(51-285)
🪛 Ruff (0.13.3)
widgets/workflow_search.py
552-552: Avoid specifying long messages outside the exception class
(TRY003)
bots/admin.py
927-934: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
935-935: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
936-936: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
937-937: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
948-958: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
959-959: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
960-960: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
961-961: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
962-969: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
970-970: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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 (9)
bots/admin.py (2)
925-938: LGTM!The
TagAdminconfiguration is well-structured with appropriate display fields, search capabilities, and filtering options. The readonly timestamp fields follow Django best practices.
960-970: LGTM!Adding
"tags__name"tosearch_fieldsenables searching published runs by tag name, and theTagInlineallows managing tags directly from thePublishedRunadmin page. These changes enhance the admin interface's usability.widgets/workflow_search.py (7)
6-6: LGTM!The new imports (
ArrayAgg,OrderBy,Tag) are used appropriately throughout the file to support tag-based search functionality and ordering logic.Also applies to: 12-12, 20-20
83-84: LGTM!The
get_query_params()method provides a clean abstraction for converting search filters to query parameters, improving code readability where it's used (line 191).
277-290: LGTM!The
render_search_suggestions()function elegantly renders featured tags as clickable pills that filter search results. The implementation correctly usesTag.get_options()to retrieve featured tags and constructs filter URLs usingsearch_filters.model_copy().
401-403: LGTM!Adding
"tags"and"versions"toprefetch_relatedand"last_edited_by"toselect_relatedprevents N+1 queries when rendering search results, as confirmed by the learnings for this file.Based on learnings
411-419: LGTM!The updated parameters to
render_saved_workflow_preview()correctly enable workflow pills, hide access levels, and pass through search filters for proper rendering context.
434-458: LGTM!The refactored sort logic is cleaner and more maintainable. Using a tuple to accumulate fields and a single
order_by()call eliminates code duplication while preserving the correct sorting behavior for each option.
516-542: LGTM!The tag search integration is well-implemented:
ArrayAgg("tags__name", distinct=True)prevents duplicate results from the many-to-many relationship- Adding
"tag_names"to the search vector with weight A prioritizes tag matches appropriately- The annotation is correctly scoped within the search query conditional
bots/admin.py
Outdated
| class TagInline(admin.TabularInline): | ||
| model = PublishedRun.tags.through | ||
| extra = 0 | ||
| autocomplete_fields = TagAdmin.autocomplete_fields |
There was a problem hiding this comment.
Fix reference to non-existent attribute.
Line 943 references TagAdmin.autocomplete_fields, but TagAdmin doesn't define this attribute. This will evaluate to the default from ModelAdmin (an empty tuple), which may not be the intended behavior.
If Tag has foreign keys that should support autocomplete in the inline, define autocomplete_fields in TagAdmin:
@admin.register(Tag)
class TagAdmin(admin.ModelAdmin):
list_display = [
"name",
"category",
"icon",
"featured_priority",
"created_at",
"updated_at",
]
search_fields = ["name", "category"]
list_filter = ["category"]
readonly_fields = ["created_at", "updated_at"]
+ autocomplete_fields = [] # Add any ForeignKey fields here if neededOtherwise, remove the reference:
class TagInline(admin.TabularInline):
model = PublishedRun.tags.through
extra = 0
- autocomplete_fields = TagAdmin.autocomplete_fields📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class TagInline(admin.TabularInline): | |
| model = PublishedRun.tags.through | |
| extra = 0 | |
| autocomplete_fields = TagAdmin.autocomplete_fields | |
| @admin.register(Tag) | |
| class TagAdmin(admin.ModelAdmin): | |
| list_display = [ | |
| "name", | |
| "category", | |
| "icon", | |
| "featured_priority", | |
| "created_at", | |
| "updated_at", | |
| ] | |
| search_fields = ["name", "category"] | |
| list_filter = ["category"] | |
| readonly_fields = ["created_at", "updated_at"] | |
| autocomplete_fields = [] # Add any ForeignKey fields here if needed |
| class TagInline(admin.TabularInline): | |
| model = PublishedRun.tags.through | |
| extra = 0 | |
| autocomplete_fields = TagAdmin.autocomplete_fields | |
| class TagInline(admin.TabularInline): | |
| model = PublishedRun.tags.through | |
| extra = 0 |
🤖 Prompt for AI Agents
In bots/admin.py around lines 940 to 943, the TagInline sets autocomplete_fields
= TagAdmin.autocomplete_fields but TagAdmin does not define that attribute;
either add an explicit autocomplete_fields list to TagAdmin (e.g.
autocomplete_fields = ("<foreign_key_field>", ...) with the actual field names
on Tag that should use autocomplete) or remove the reference and set
autocomplete_fields directly on TagInline or omit it entirely; update
accordingly so the inline uses the intended autocomplete fields instead of
inheriting a non-existent attribute.
use ArrayAgg instead of m2m in vector query
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
widgets/workflow_search.py (2)
82-83: Good DRY refactor.Encapsulating the query parameter generation logic improves maintainability and provides a clear API for building URLs with search filters.
433-457: Improved maintainability with tuple accumulation pattern.The refactoring from multiple early returns to a tuple accumulation pattern makes the sorting logic more maintainable and easier to understand at a glance. The conditional addition of
"-rank"for the featured sort is correctly handled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
widgets/workflow_search.py(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T07:23:39.522Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:23:39.522Z
Learning: In `widgets/workflow_search.py`, the `render_search_results()` function QuerySet at line 375 needs `.select_related("workspace", "saved_run", "last_edited_by").prefetch_related("tags", "versions")` to avoid N+1 queries when rendering search results with published run previews. Note: `created_by` should not be included as it's not used in workflow rendering.
Applied to files:
widgets/workflow_search.py
🧬 Code graph analysis (1)
widgets/workflow_search.py (4)
bots/models/published_run.py (4)
PublishedRun(123-358)Tag(419-450)get_options(438-439)render(434-435)routers/root.py (1)
explore_page(248-260)daras_ai_v2/fastapi_tricks.py (1)
get_app_route_url(68-74)widgets/saved_workflow.py (1)
render_saved_workflow_preview(25-97)
⏰ 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 (5)
widgets/workflow_search.py (5)
276-289: Well-implemented tag suggestions feature.The function correctly preserves existing filters while adding tag-based search queries, providing a good user experience for discovering content by tags.
190-190: Consistent use of the new method.Using
get_query_params()aligns with the DRY refactor and improves code consistency.
400-402: Correct N+1 query prevention.The prefetch and select_related calls properly optimize the QuerySet to avoid N+1 queries when rendering search results. The exclusion of
created_byaligns with the guidance that it's not used in workflow rendering.Based on learnings.
410-418: Appropriate parameter choices for search result rendering.The parameters passed to
render_saved_workflow_previeware well-suited for the search results context: showing workflow type for clarity while hiding access level since access filtering is already applied, and passing search filters for context-aware rendering.
515-527: Verify ArrayAgg doesn't introduce duplicates or performance issues.The use of
ArrayAggwithdistinct=Trueshould correctly aggregate tag names perPublishedRunwithout duplicates. However, please verify:
- No duplicate results: Ensure the search results don't contain duplicate
PublishedRunentries when tags are present- Performance: Check query performance with a representative dataset, especially when many tags exist per workflow
- SQL correctness: Examine the generated SQL to confirm proper
GROUP BYhandlingThe implementation looks sound, but these checks will confirm the query optimization is working as expected.
You can verify by:
- Running test queries with
qs.queryor Django Debug Toolbar to inspect the SQL- Comparing result counts before/after the ArrayAgg annotation
- Testing with workflows that have multiple tags
2 participants
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.