bulk progress display model#987
Open
milovate wants to merge 5 commits into
Open
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gooey-gui/app/components/BulkProgressCard.css`:
- Line 308: Replace deprecated CSS declarations "word-break: break-word" with
modern wrapping properties: use "overflow-wrap: anywhere;" and set "word-break:
normal;" where appropriate to preserve expected behavior. Update each occurrence
of the exact token "word-break: break-word" in BulkProgressCard.css (three
instances flagged) to "overflow-wrap: anywhere; word-break: normal;" so browsers
use the current spec for soft wrapping while avoiding the deprecated value.
- Around line 192-193: The font-family declarations in BulkProgressCard.css use
unquoted custom font names (e.g., avenir-lt-w01_85-heavy1475544 and
avenir-lt-w05_85-heavy) which stylelint flags; update each font-family
declaration in this file (including the other occurrences of the same names) to
wrap custom font names in quotes so they read as string identifiers, preserving
the comma-separated order and leaving generic family names (e.g., "Space
Grotesk", sans-serif) as-is.
In `@widgets/bulk_progress_state.py`:
- Around line 12-13: The module is missing the exported constant
BULK_RERUN_ALL_KEY that widgets.bulk_progress_display imports; add a top-level
definition and export for BULK_RERUN_ALL_KEY (e.g., a descriptive string
constant) in widgets/bulk_progress_state.py alongside BulkProgressPhase so the
import no longer fails at startup and other code can reference that symbol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d289377-2251-4c1e-b337-7297bada119d
📒 Files selected for processing (7)
gooey-gui/app/components/BulkProgressCard.cssgooey-gui/app/components/BulkProgressCard.tsxgooey-gui/app/components/index.tsrecipes/BulkRunner.pywidgets/bulk_progress_display.pywidgets/bulk_progress_state.pywidgets/bulk_runner_progress.py
Comment on lines
+192
to
+193
| font-family: avenir-lt-w01_85-heavy1475544, avenir-lt-w05_85-heavy, | ||
| "Space Grotesk", sans-serif; |
Contributor
There was a problem hiding this comment.
Quote custom font-family names to satisfy stylelint.
Unquoted custom font names are flagged; quote them in each declaration.
Proposed fix pattern
- font-family: avenir-lt-w01_85-heavy1475544, avenir-lt-w05_85-heavy,
+ font-family: "avenir-lt-w01_85-heavy1475544", "avenir-lt-w05_85-heavy",
"Space Grotesk", sans-serif;Also applies to: 260-261, 356-357, 472-473, 480-481
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 192-192: Expected quotes around "avenir-lt-w01_85-heavy1475544" (font-family-name-quotes)
(font-family-name-quotes)
[error] 192-192: Expected quotes around "avenir-lt-w05_85-heavy" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gooey-gui/app/components/BulkProgressCard.css` around lines 192 - 193, The
font-family declarations in BulkProgressCard.css use unquoted custom font names
(e.g., avenir-lt-w01_85-heavy1475544 and avenir-lt-w05_85-heavy) which stylelint
flags; update each font-family declaration in this file (including the other
occurrences of the same names) to wrap custom font names in quotes so they read
as string identifiers, preserving the comma-separated order and leaving generic
family names (e.g., "Space Grotesk", sans-serif) as-is.
milovate
force-pushed
the
feat/bulk-progress-display-model
branch
3 times, most recently
from
de747ab to
452f20d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 452f20d0f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
468
to
469
| sr.wait_for_celery_result(result) | ||
| response.eval_runs.append(sr.get_app_url()) |
There was a problem hiding this comment.
When eval_urls are present, these eval workflows are submitted as normal API calls with the default credit deduction path, but the bulk progress tracker only increments credits_used in workflow_completed for the primary run_urls. After sr.wait_for_celery_result(result), the eval run's sr.price is available but never added to the snapshot, so the evaluating and complete BulkProgressCard underreports Credits for any bulk run whose evaluator workflows consume credits.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
check if resolved
milovate
force-pushed
the
feat/bulk-progress-display-model
branch
from
3c20e98 to
8310209
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gooey-gui/app/components/bulkProgress/BulkProgressCard.css`:
- Line 3: Two CSS declarations ("background: `#fff`;") are missing the required
empty line before them and violate the stylelint rule
declaration-empty-line-before; open the BulkProgressCard.css and insert a single
blank line immediately above each offending declaration (the "background: `#fff`;"
at the top and the other occurrence around line 126) so there is an empty line
separating it from the previous declaration or comment, ensuring the file
conforms to the rule (alternatively, if intentional grouping is required, add a
one-line stylelint comment to explicitly disable declaration-empty-line-before
for that specific declaration).
In `@gooey-gui/app/components/bulkProgress/bulkProgressCardModel.ts`:
- Around line 313-316: The computed ring percentages can go out of bounds; clamp
the percentage values for both evals and unit-runs to the 0–100 range before
returning or rendering. Locate where ringPercent is computed from completedEvals
and evalCounts.total (and the analogous unit-run percent computed later, e.g.,
unitRunPercent/unitRunLabel) and wrap the raw percentage calculation with a
clamp (Math.min(Math.max(rawPercent, 0), 100) or a small clamp helper) so the
returned ringPercent and the unit-run percent are guaranteed to be between 0 and
100.
In `@widgets/bulk_progress_display.py`:
- Around line 143-169: The function bulk_snapshot_run_state treats a run as
"complete" whenever all unit runs finished (bulk_complete) even if
progress["phase"] is "evaluating" and evaluation work didn’t finish; update
bulk_snapshot_run_state so that before returning "complete" for non-running runs
you check progress["phase"] and if it equals "evaluating" return "evaluating"
(or keep "stopped"/"stopping" only when appropriate), i.e., move the phase check
for "evaluating" to cover the non-running path (use progress["phase"] and
bulk_complete together) and only return "complete" when phase is "complete" (or
bulk_complete and phase != "evaluating"); keep existing handling of
is_cancelled, is_running, and error_msg intact.
In `@widgets/bulk_progress_state.py`:
- Around line 293-296: The function bulk_progress_percent uses round(...) which
can bump percentages to 100 before completion; change the calculation to
truncate instead of round: compute percentage as integer truncation (e.g.,
int(completed_unit_runs * 100 / total_unit_runs)) and then clamp to 100 if
completed_unit_runs equals total_unit_runs (or use min(pct, 100)) to avoid
overshooting; update bulk_progress_percent to use completed_unit_runs and
total_unit_runs accordingly and ensure you handle zero/None total_unit_runs as
before.
- Around line 123-140: The eval_started method currently only returns a status
string so the updated response.eval_progress and response.bulk_progress (and
snapshot phase="evaluating") are never streamed; change eval_started to return
the response object after mutating it (i.e., set response.eval_progress and
response.bulk_progress as you already do and then return response instead of a
plain string) so the evaluation snapshot is emitted immediately; update callers
of eval_started if necessary to handle the response object returned from
eval_started.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 65b181db-a60a-4c91-9687-af2e1caf934a
📒 Files selected for processing (9)
gooey-gui/app/components/bulkProgress/BulkProgressCard.cssgooey-gui/app/components/bulkProgress/BulkProgressCard.tsxgooey-gui/app/components/bulkProgress/bulkProgress.types.tsgooey-gui/app/components/bulkProgress/bulkProgressCardModel.tsgooey-gui/app/components/bulkProgress/bulkProgressFormat.tsgooey-gui/app/components/index.tsrecipes/BulkRunner.pywidgets/bulk_progress_display.pywidgets/bulk_progress_state.py
✅ Files skipped from review due to trivial changes (1)
- gooey-gui/app/components/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- gooey-gui/app/components/bulkProgress/bulkProgress.types.ts
- gooey-gui/app/components/bulkProgress/bulkProgressFormat.ts
- gooey-gui/app/components/bulkProgress/BulkProgressCard.tsx
- recipes/BulkRunner.py
| @@ -0,0 +1,508 @@ | |||
| .bulk-progress-card { | |||
| --bulk-progress-detail-label-width: 7.5rem; | |||
| background: #fff; | |||
Contributor
There was a problem hiding this comment.
Fix stylelint declaration-empty-line-before violations.
Two declarations are missing the required empty line and can fail lint checks.
Suggested patch
.bulk-progress-card {
--bulk-progress-detail-label-width: 7.5rem;
+
background: `#fff`;
@@
.bulk-progress-copy {
@@
);
+
align-items: flex-start;Also applies to: 126-126
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 3-3: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gooey-gui/app/components/bulkProgress/BulkProgressCard.css` at line 3, Two
CSS declarations ("background: `#fff`;") are missing the required empty line
before them and violate the stylelint rule declaration-empty-line-before; open
the BulkProgressCard.css and insert a single blank line immediately above each
offending declaration (the "background: `#fff`;" at the top and the other
occurrence around line 126) so there is an empty line separating it from the
previous declaration or comment, ensuring the file conforms to the rule
(alternatively, if intentional grouping is required, add a one-line stylelint
comment to explicitly disable declaration-empty-line-before for that specific
declaration).
Comment on lines
+293
to
+296
| def bulk_progress_percent(progress: BulkProgressCounts | None) -> int: | ||
| if not progress or not progress["total_unit_runs"]: | ||
| return 0 | ||
| return round(progress["completed_unit_runs"] / progress["total_unit_runs"] * 100) |
Contributor
There was a problem hiding this comment.
Don't round incomplete progress up to 100%.
round(...) can report 100% Completed before the last unit run finishes. That makes the streamed status lie on large batches.
Proposed fix
def bulk_progress_percent(progress: BulkProgressCounts | None) -> int:
if not progress or not progress["total_unit_runs"]:
return 0
- return round(progress["completed_unit_runs"] / progress["total_unit_runs"] * 100)
+ completed = progress["completed_unit_runs"]
+ total = progress["total_unit_runs"]
+ if completed >= total:
+ return 100
+ return completed * 100 // total🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@widgets/bulk_progress_state.py` around lines 293 - 296, The function
bulk_progress_percent uses round(...) which can bump percentages to 100 before
completion; change the calculation to truncate instead of round: compute
percentage as integer truncation (e.g., int(completed_unit_runs * 100 /
total_unit_runs)) and then clamp to 100 if completed_unit_runs equals
total_unit_runs (or use min(pct, 100)) to avoid overshooting; update
bulk_progress_percent to use completed_unit_runs and total_unit_runs accordingly
and ensure you handle zero/None total_unit_runs as before.
This comment was marked as outdated.
This comment was marked as outdated.
milovate
force-pushed
the
feat/bulk-progress-display-model
branch
from
7f2b997 to
8f055f4
Compare
4 tasks
Replace deprecated bulk progress word wrapping, validate input audio links before rendering anchors, and derive the card run state inside the snapshot builder. Keep eval progress rings based on completed evals and inline redundant one-off helpers in the bulk progress display path.
Refactor the bulk progress card model to ensure ring percentages are clamped between 0 and 100. Update the bulk progress display to replace the deprecated Literal with an Enum for bulk runner states, enhancing clarity and maintainability. Adjust snapshot building to utilize the new state management approach.
- Trim workflow titles before rendering so whitespace-only titles do not create empty links.
Show per-workflow elapsed time beside the active workflow link during running/stopping, and reorder the complete-state summary to Total runs, Rows, Credits, Total time. Reuse workflow link-group layout for last completed meta and inline the live elapsed hook in WorkflowRow. Remove unused BulkRunner row-group helper. Co-authored-by: Cursor <cursoragent@cursor.com>
milovate
force-pushed
the
feat/bulk-progress-display-model
branch
from
8f055f4 to
389bb68
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extract progress orchestration and display building into bulk_progress_state and bulk_progress_display, drive eval UI from bulk_progress.phase, and render a layout-only BulkProgressCard from Python camelCase props.
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.