feat: bulk runner UI improvements with responsive workflow selection #811
Conversation
📝 WalkthroughWalkthroughAdded BasePage.get_recipe_short_title() in daras_ai_v2/base.py returning the workflow emoji combined with the workflow short_title. Updated icon HTML classes in daras_ai_v2/workflow_url_input.py for edit and delete buttons from Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
🧹 Nitpick comments (5)
daras_ai_v2/workflow_url_input.py (1)
88-91: Muted delete icon may reduce affordance for destructive actionConsider adding a tooltip (“Remove”) or confirm on press since color no longer communicates danger.
def del_button(key: str): - gui.button( - '<i class="fa-regular fa-trash text-muted"></i>', - key=key, - type="tertiary", - ) + with gui.tooltip("Remove"): + gui.button( + '<i class="fa-regular fa-trash text-muted"></i>', + key=key, + type="tertiary", + )recipes/BulkRunner.py (4)
415-436: URL-only path split — guard against NoneLooks good. Minor nit: ensure
url_buttonnever receivesNone.- url = gui.text_input( + url = gui.text_input( "", key=key, - value=d.get("url"), + value=d.get("url") or "", placeholder="https://gooey.ai/.../?run_id=...", ) ... - gui.url_button(url) + gui.url_button(url or "")
437-456: Use primitive keys for selectbox state; sort options for UXEnum instances in session state can be brittle. Prefer ints; also sort labels for easier scanning.
- options = { - page_cls.workflow: page_cls.get_recipe_short_title() + options = { + int(page_cls.workflow): page_cls.get_recipe_short_title() for page_cls in all_home_pages } + # sort by label + options = dict(sorted(options.items(), key=lambda kv: kv[1].casefold())) ... - d["workflow"] = workflow + d["workflow"] = int(workflow)
473-492: Mobile header shows stale link until next render
url_button(d.get("url", ""))renders before the user picks a URL below, so it can be one render behind compared to desktop (which usesurl). If noticeable, consider moving the button after the selector or reusing the newly selected value.
519-535: Avoid validating empty URLs to prevent noisy errorsWrap
url_to_runscall so blank selections don’t emit errors during initial render.- try: - url_to_runs(url) - except Exception as e: - gui.error(repr(e)) + if url: + try: + url_to_runs(url) + except Exception as e: + gui.error(repr(e))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
daras_ai_v2/base.py(1 hunks)daras_ai_v2/workflow_url_input.py(1 hunks)recipes/BulkRunner.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
daras_ai_v2/base.py (1)
bots/models/workflow.py (2)
emoji(235-237)short_title(230-232)
recipes/BulkRunner.py (3)
daras_ai_v2/workflow_url_input.py (5)
edit_done_button(70-75)del_button(86-91)get_published_run_options(145-182)edit_button(78-83)init_workflow_selector(94-123)bots/models/workflow.py (2)
page_cls(240-243)Workflow(189-257)daras_ai_v2/base.py (1)
get_recipe_short_title(1144-1145)
⏰ 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)
daras_ai_v2/workflow_url_input.py (1)
80-83: Icon tone change to muted — OKVisual-only change looks fine.
Please sanity-check contrast in dark mode and hover/focus states.
recipes/BulkRunner.py (2)
457-472: URL selector — LGTMDeriving
page_clsfromWorkflow(...)and merging--added_workflowsis clean.
493-518: Desktop workflow mode — LGTMOrder guarantees
urlis defined before use; good split of columns.
| @classmethod | ||
| def get_recipe_short_title(cls) -> str: | ||
| return f"{cls.workflow.emoji} {cls.workflow.short_title}" | ||
|
|
There was a problem hiding this comment.
Fix call vs. attribute for emoji/short_title to avoid bound-method strings in UI
bots.models.workflow exposes emoji()/short_title() as methods (no @property shown). Using them as attributes renders bound-method text. Call them or handle both styles.
@classmethod
def get_recipe_short_title(cls) -> str:
- return f"{cls.workflow.emoji} {cls.workflow.short_title}"
+ # Support both method and property styles on Workflow
+ emoji = cls.workflow.emoji() if callable(getattr(cls.workflow, "emoji", None)) else cls.workflow.emoji
+ short = cls.workflow.short_title() if callable(getattr(cls.workflow, "short_title", None)) else cls.workflow.short_title
+ return f"{emoji} {short}".strip()📝 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.
| @classmethod | |
| def get_recipe_short_title(cls) -> str: | |
| return f"{cls.workflow.emoji} {cls.workflow.short_title}" | |
| @classmethod | |
| def get_recipe_short_title(cls) -> str: | |
| # Support both method and property styles on Workflow | |
| emoji = cls.workflow.emoji() if callable(getattr(cls.workflow, "emoji", None)) else cls.workflow.emoji | |
| short = cls.workflow.short_title() if callable(getattr(cls.workflow, "short_title", None)) else cls.workflow.short_title | |
| return f"{emoji} {short}".strip() |
🤖 Prompt for AI Agents
daras_ai_v2/base.py around lines 1143-1146: cls.workflow.emoji and
cls.workflow.short_title are methods on the workflow model, but the code treats
them as attributes which yields bound-method strings in the UI; change the
implementation to call them when callable (e.g., emoji = cls.workflow.emoji() if
callable(getattr(cls.workflow, "emoji", None)) else cls.workflow.emoji) and
similarly for short_title, with safe fallbacks (empty string or str(...)) before
constructing and returning the formatted title.
milovate
force-pushed
the
bulk-improvements
branch
from
69e7aec to
7721376
Compare
milovate
changed the title
milovate
force-pushed
the
bulk-improvements
branch
from
7721376 to
bf43305
Compare
milovate
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
recipes/BulkRunner.py (1)
438-438: Consider optimizing the import placement.The import
from daras_ai_v2.all_pages import all_home_pagesis performed inside the method, which could impact performance if this method is called frequently. Consider moving this to the top-level imports if it doesn't cause circular import issues.+from daras_ai_v2.all_pages import all_home_pages class BulkRunnerPage(BasePage): # ... existing code ... def _render_workflow_selector(self, key: str, d: dict): - from daras_ai_v2.all_pages import all_home_pages - options = { page_cls.workflow: page_cls.get_recipe_short_title() for page_cls in all_home_pages }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
daras_ai_v2/base.py(1 hunks)daras_ai_v2/workflow_url_input.py(1 hunks)recipes/BulkRunner.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- daras_ai_v2/workflow_url_input.py
- daras_ai_v2/base.py
🧰 Additional context used
🧬 Code graph analysis (1)
recipes/BulkRunner.py (3)
daras_ai_v2/workflow_url_input.py (5)
edit_done_button(70-75)del_button(86-91)get_published_run_options(145-182)edit_button(78-83)init_workflow_selector(94-123)bots/models/workflow.py (2)
page_cls(240-243)Workflow(189-257)daras_ai_v2/base.py (1)
get_recipe_short_title(1144-1145)
⏰ 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 (7)
recipes/BulkRunner.py (7)
415-435: LGTM! Well-structured mobile/desktop UI helper.The
_render_url_input_onlyhelper method correctly handles responsive column layouts and maintains consistent UI patterns with edit/delete buttons.
437-455: LGTM! Clean workflow selector implementation.The method properly uses
get_recipe_short_title()and implements session state persistence for the last selected workflow, improving user experience.
457-471: LGTM! URL selector with proper options handling.The method correctly retrieves published run options and merges additional workflows from the internal state.
473-491: LGTM! Mobile-optimized workflow mode UI.The mobile layout properly stacks workflow selector and URL selector with appropriate responsive styling.
493-517: LGTM! Desktop-optimized workflow mode UI.The desktop layout uses appropriate column proportions and maintains consistent button placement.
519-540: LGTM! Excellent refactoring with responsive design.The refactored
render_run_url_inputsmethod successfully delegates to the new private helpers while maintaining the same public API. The responsive design approach with separate mobile/desktop rendering paths is well-implemented.
441-441: get_recipe_short_title method is defined and available. The definition is present in daras_ai_v2/base.py (line 1144), so the call in recipes/BulkRunner.py is valid.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
recipes/BulkRunner.py (3)
457-471: Ensure current URL is selectable even if not in options.If
d['url']isn’t inurl_options(e.g., private run, freshly created), the selectbox can’t select it. Merge the current URL into options as a fallback.Apply this diff:
page_cls = Workflow(workflow).page_cls url_options = get_published_run_options( page_cls, current_user=self.request.user ) url_options.update(d.get("--added_workflows", {})) + # Ensure the currently selected URL remains selectable even if it's not in options + current_url = d.get("url") + if current_url and current_url not in url_options: + url_options = { + current_url: d.get("--added_workflows", {}).get(current_url, current_url) + } | url_options + url = gui.selectbox( "", key=key, options=url_options, value=d.get("url"), format_func=lambda x: url_options[x], ) return url
522-535: Avoid relying on “last return wins” when rendering both mobile and desktop variants.Both variants render with the same keys. Relying on the second call’s return value is brittle; derive
urlfrom session_state instead to avoid key-collision edge cases.Apply this diff:
- if not d.get("workflow") and d.get("url"): - with gui.div(className="d-block d-lg-none"): - url = self._render_url_input_only(key, del_key, d, is_mobile=True) - - with gui.div(className="d-none d-lg-block"): - url = self._render_url_input_only(key, del_key, d, is_mobile=False) - else: - with gui.div(className="d-block d-lg-none"): - url = self._render_workflow_mode_mobile(key, del_key, d) - - with gui.div(className="d-none d-lg-block"): - url = self._render_workflow_mode_desktop(key, del_key, d) + if not d.get("workflow") and d.get("url"): + with gui.div(className="d-block d-lg-none"): + self._render_url_input_only(key, del_key, d, is_mobile=True) + with gui.div(className="d-none d-lg-block"): + self._render_url_input_only(key, del_key, d, is_mobile=False) + else: + with gui.div(className="d-block d-lg-none"): + self._render_workflow_mode_mobile(key, del_key, d) + with gui.div(className="d-none d-lg-block"): + self._render_workflow_mode_desktop(key, del_key, d) + + # Single source of truth for the current value + url = gui.session_state.get(key, d.get("url"))Please confirm that
gooey_guitolerates rendering controls with identical keys in hidden containers; some frameworks error on duplicate-key widgets even when hidden.
536-541: Don’t validate empty URLs.Avoid emitting errors before the user enters a URL.
Apply this diff:
- try: - url_to_runs(url) - except Exception as e: - gui.error(repr(e)) + if url: + try: + url_to_runs(url) + except Exception as e: + gui.error(repr(e)) d["url"] = url
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
daras_ai_v2/workflow_url_input.py(1 hunks)recipes/BulkRunner.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- daras_ai_v2/workflow_url_input.py
🧰 Additional context used
🧬 Code graph analysis (1)
recipes/BulkRunner.py (3)
daras_ai_v2/workflow_url_input.py (5)
edit_done_button(70-75)del_button(86-91)get_published_run_options(145-182)edit_button(78-83)init_workflow_selector(94-123)bots/models/workflow.py (2)
page_cls(240-243)Workflow(189-257)daras_ai_v2/base.py (1)
get_recipe_short_title(1144-1145)
⏰ 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)
recipes/BulkRunner.py (3)
415-436: Nice split for URL-only path.Clear, minimal UI with responsive columns. No issues from my side.
437-456: Good: lazy import + persisted last-workflow selection.Pulling
all_home_pagesinside the function helps import-time. Remember to keep option order stable ifall_home_pagesorder matters for UX.Is
all_home_pagesalready in the intended display order? If not, consider sorting bypage_cls.get_recipe_short_title()to keep it deterministic.
493-518: Desktop workflow+URL layout looks good.Clear separation of workflow and URL controls; aligns with the mobile path. No blocking issues.
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.