Open
Conversation
Implements ace-step#695
Contributor
📝 WalkthroughWalkthroughRefactors the Gradio UI: extracts inline CSS into files, adds a custom theme, reorganizes the interface into header/sidebar/content with tabbed flows, replaces accordion containers with section/column containers (renamed keys), and tweaks button styling and event wiring references. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
acestep/ui/gradio/events/__init__.py (1)
16-1238: 🛠️ Refactor suggestion | 🟠 Major
events/__init__.pyis 1238 LOC — significantly exceeds the 200 LOC hard cap.
setup_event_handlersalone wires generation, batch navigation, audio saving, LRC, scoring, and simple-mode handlers.setup_training_event_handlersadds another ~400 lines. Consider splitting by domain (e.g.events/generation/wiring.py,events/results/wiring.py,events/training/wiring.py).Based on learnings: "Enforce a module size guideline for Python files: only raise module-size concerns when a file exceeds 200 lines of code (LOC) ... when a file exceeds 200 LOC, suggest splitting into smaller modules to improve readability and maintainability."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/events/__init__.py` around lines 16 - 1238, The file is too large (1238 LOC) and should be split; extract wiring for major domains into smaller modules. Move the large setup_event_handlers logic into domain-specific wiring functions (e.g., generation wiring, results wiring, training wiring) by creating new modules like events/generation/wiring.py, events/results/wiring.py, events/training/wiring.py and refactor setup_event_handlers to delegate to helpers such as register_generation_handlers, register_results_handlers, register_lrc_handlers, register_batch_navigation_handlers, and setup_training_event_handlers; update imports and call sites so setup_event_handlers simply calls these smaller functions (preserving signatures for dit_handler, llm_handler, generation_section, results_section, etc.) to keep each module under ~200 LOC.acestep/ui/gradio/interfaces/training.py (1)
15-837: 🛠️ Refactor suggestion | 🟠 Major
training.pyis 837 LOC — far exceeds the 200 LOC hard cap.The file mixes dataset builder, LoRA training, and LoKr training concerns. Each tab's UI construction should be extracted into its own module (e.g.
training_dataset.py,training_lora.py,training_lokr.py) to stay within the 200 LOC limit and improve maintainability.Based on learnings: "Enforce a module size guideline for Python files: only raise module-size concerns when a file exceeds 200 lines of code (LOC) ... when a file exceeds 200 LOC, suggest splitting into smaller modules to improve readability and maintainability."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/interfaces/training.py` around lines 15 - 837, The create_training_section function is too large (>200 LOC); split its concerns by extracting the Dataset Builder, LoRA Training, and LoKr Training tab constructions into separate modules (e.g., training_dataset.py, training_lora.py, training_lokr.py) each exposing a builder function (e.g., build_dataset_tab, build_lora_tab, build_lokr_tab) that returns the Gradio components used in the final dict; then simplify create_training_section to import and call those builders to assemble the Tabs, merge their returned components into the single return dictionary (keeping unique keys like "load_json_btn", "start_training_btn", "start_lokr_training_btn", etc.), and ensure dataset_builder_state and training_state remain created and included — this will keep each file <200 LOC and preserve existing component names for event wiring.
🧹 Nitpick comments (3)
acestep/ui/gradio/interfaces/theme.py (1)
17-32: Movefonts.LocalFont(...)calls out of argument defaults (Ruff B008).Calling
fonts.LocalFont(...)in a default argument is evaluated once at class definition time and flagged by Ruff B008. The idiomatic fix is module-level constants:🔧 Proposed fix
+_DEFAULT_FONT: tuple[fonts.Font | str, ...] = ( + fonts.LocalFont("Montserrat"), + "ui-sans-serif", + "system-ui", + "sans-serif", +) +_DEFAULT_FONT_MONO: tuple[fonts.Font | str, ...] = ( + fonts.LocalFont("IBM Plex Mono"), + "ui-monospace", + "Consolas", + "monospace", +) + + class AceStepTheme(gr.themes.Base): def __init__( self, *, ... font: fonts.Font | str - | Iterable[fonts.Font | str] = ( - fonts.LocalFont("Montserrat"), - "ui-sans-serif", - "system-ui", - "sans-serif", - ), + | Iterable[fonts.Font | str] = _DEFAULT_FONT, font_mono: fonts.Font | str - | Iterable[fonts.Font | str] = ( - fonts.LocalFont("IBM Plex Mono"), - "ui-monospace", - "Consolas", - "monospace", - ), + | Iterable[fonts.Font | str] = _DEFAULT_FONT_MONO, ):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/interfaces/theme.py` around lines 17 - 32, The default arguments for the Theme (symbols: font and font_mono) call fonts.LocalFont(...) at definition time (Ruff B008); move those calls into module-level constants (e.g., MONTERRAT_LOCAL and IBM_PLEX_LOCAL) and then use those constants in the default tuples for font and font_mono so the LocalFont() constructors are executed at import time via explicit constants rather than as default argument expressions.acestep/ui/gradio/interfaces/css/main.css (1)
221-232:btn-primary-importantuses hardcoded hex colours instead of theme variables.
#5f00a6/#6d00bfare duplicated colour values that bypass theAceStepThemetoken system. If the theme palette is ever changed, this button will look inconsistent. Consider referencing a CSS variable defined by the theme or a shared utility variable.♻️ Suggested refactor
+/* Define once, ideally in utils.css or via the Gradio theme */ +:root { + --btn-important-bg: `#5f00a6`; + --btn-important-bg-hover: `#6d00bf`; +} + .btn-primary-important:not(:disabled) { - background: `#5f00a6`; + background: var(--btn-important-bg, `#5f00a6`); color: white; } .btn-primary-important:not(:disabled):hover { - background: `#6d00bf`; + background: var(--btn-important-bg-hover, `#6d00bf`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/interfaces/css/main.css` around lines 221 - 232, Replace the hardcoded hex values used by .btn-primary-important and its hover state with the theme/CSS variables from AceStepTheme (e.g. use the shared primary color variable and a darker variant) so the button follows the global palette; update .btn-primary-important:not(:disabled), .btn-primary-important:not(:disabled):hover and body.dark .btn-primary-important:disabled to reference those CSS vars (or add the vars to the theme if missing) instead of `#5f00a6` / `#6d00bf` to avoid duplication and keep styling consistent with the theme.acestep/ui/gradio/interfaces/generation.py (1)
1-953: Split this module and large UI builders into smaller units.This file and
create_generation_tab_sectionhave grown far past the repository size/focus guidance, which makes event wiring and future UI changes harder to reason about safely. Please split by responsibility (e.g., service config, advanced accordions, generation-mode blocks, optional params, action rows).As per coding guidelines "
**/*.py: Target module size: optimal <= 150 LOC, hard cap 200 LOC" and "Split functions by responsibility, not by convenience."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/interfaces/generation.py` around lines 1 - 953, This module is too large and should be split by responsibility; extract the big builders into smaller modules and keep only orchestration in this file. Create separate files (e.g., service_config.py, advanced_accordions.py, generation_mode.py, optional_params.py, action_rows.py) and move the corresponding UI builders: _create_service_config_content and create_service_config_section -> service_config module; the LoRA/DiT/LM/Output/Automation accordion blocks and create_advanced_settings_section logic -> advanced_accordions module; the create_generation_tab_section and its internal groups (generation_mode radio, simple_mode_group, custom_mode_group, src_audio_row, repainting_group, generate button row) -> generation_mode module; leave _compute_init_defaults in a small utils module to be imported by the above. Update create_advanced_settings_section and create_generation_tab_section to import these new builders and only orchestrate composition, and keep create_generation_section as a thin merger that imports the split functions; ensure public symbol names (create_advanced_settings_section, create_generation_tab_section, _create_service_config_content, _compute_init_defaults) remain available (or re-export) so event wiring and callers don't break, and run/adjust any event wiring/tests that reference the moved components.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@acestep/ui/gradio/events/generation/mode_ui.py`:
- Line 148: Replace component instantiation returns with gr.update property
updates: in the generation event handler where the tuple currently returns
gr.Column(visible=show_optional) (the optional_params_section output), change
that to gr.update(visible=show_optional); similarly, in init_service_wrapper()
where the code returns gr.Accordion(open=not is_model_initialized), replace it
with gr.update(open=not is_model_initialized). Locate the return tuples in the
event handler (the one returning optional_params_section) and in
init_service_wrapper() and swap component constructors for gr.update(...) using
the same property names (visible/open) and boolean expressions.
In `@acestep/ui/gradio/help_content.py`:
- Around line 194-195: The module currently reads the CSS file at import time
via css_file.read_text() and assigns it to HELP_MODAL_CSS which will raise
FileNotFoundError (or encoding errors) and break imports; change this to lazily
load or safely read the file with explicit encoding='utf-8' and a guarded
try/except that catches FileNotFoundError and UnicodeDecodeError, returning a
sensible fallback (e.g., empty string or bundled default CSS) and emitting a
warning; update references to css_file and HELP_MODAL_CSS so the module no
longer performs an unguarded read at import time and instead uses the safe
loader.
In `@acestep/ui/gradio/interfaces/__init__.py`:
- Around line 93-96: The Training UI and its callbacks are only hidden but not
disabled in service_mode; change the code so registration of training callbacks
is also conditional on service_mode: when creating the Tab with
create_training_section(dit_handler, llm_handler, init_params) only call
whatever registers training callbacks (e.g., methods on training_section or any
subsequent calls that attach handlers) if visible/if not service_mode—i.e., wrap
the callback registration logic the same way the Tab visibility is gated so that
in service_mode both the UI and the training handlers are not registered.
In `@acestep/ui/gradio/interfaces/css/help_modal.css`:
- Around line 38-42: Add keyboard focus-visible styles for the help button and
close button so keyboard users get a clear focus indicator: update the CSS for
.help-inline-btn and .help-modal-close to include :focus-visible rules
(mirroring the hover visual language) that set a visible outline or border-color
using var(--color-accent) and preserve/adjust transform (e.g., scale) and
include an accessible outline-offset; specifically add
.help-inline-btn:focus-visible and .help-modal-close:focus-visible selectors to
the stylesheet so both elements show a clear, consistent focus state.
In `@acestep/ui/gradio/interfaces/css/main.css`:
- Around line 37-41: The CSS has an orphaned comment terminator causing a parse
error: remove the stray "*/" after the .instrumental-row>div>button rule so the
selector block remains active (or alternatively wrap the whole block in /* ...
*/ if you want it commented out); locate the .instrumental-row>div>button rule
and either delete the trailing "*/" or enclose the entire selector and its
declarations in a proper comment.
In `@acestep/ui/gradio/interfaces/generation.py`:
- Around line 326-339: Replace the hardcoded English UI strings in the LoRA
accordion with localized calls; update the label/placeholder/info/value
arguments for lora_path, load_lora_btn, unload_lora_btn, use_lora_checkbox,
lora_scale_slider, and lora_status to use the translation helper t(...) (or the
project’s i18n function) instead of raw strings so these controls render in the
active language—ensure placeholders, button text, checkbox label, slider
label/info, and the default status value all call t(...) with appropriate keys.
In `@acestep/ui/gradio/interfaces/theme.py`:
- Around line 47-59: The theme assignment block contains mixed single and
double-quoted string literals (e.g., button_primary_background_fill_dark,
button_primary_text_color_dark, input_background_fill_dark,
block_background_fill_dark, block_border_color_dark, slider_color_dark,
color_accent); update all single-quoted string literals in this block to use
double quotes consistently (replace occurrences like '...'' with "...") so every
string in that theme configuration uses double quotes per the style guide.
- Around line 1-127: Add concise docstrings to the module (top of the file), the
AceStepTheme class, and its __init__ method: for the module include a one-line
summary of the theme utilities provided; for class AceStepTheme add a short
description of the theme's purpose; for __init__ document the parameters
(primary_hue, secondary_hue, neutral_hue, spacing_size, radius_size, text_size,
font, font_mono) and any important behavior (it calls super().__init__ and sets
custom theme values via super().set); keep all three minimal but present and
follow the project's docstring style (one-line summary plus brief Args when
appropriate).
---
Outside diff comments:
In `@acestep/ui/gradio/events/__init__.py`:
- Around line 16-1238: The file is too large (1238 LOC) and should be split;
extract wiring for major domains into smaller modules. Move the large
setup_event_handlers logic into domain-specific wiring functions (e.g.,
generation wiring, results wiring, training wiring) by creating new modules like
events/generation/wiring.py, events/results/wiring.py, events/training/wiring.py
and refactor setup_event_handlers to delegate to helpers such as
register_generation_handlers, register_results_handlers, register_lrc_handlers,
register_batch_navigation_handlers, and setup_training_event_handlers; update
imports and call sites so setup_event_handlers simply calls these smaller
functions (preserving signatures for dit_handler, llm_handler,
generation_section, results_section, etc.) to keep each module under ~200 LOC.
In `@acestep/ui/gradio/interfaces/training.py`:
- Around line 15-837: The create_training_section function is too large (>200
LOC); split its concerns by extracting the Dataset Builder, LoRA Training, and
LoKr Training tab constructions into separate modules (e.g.,
training_dataset.py, training_lora.py, training_lokr.py) each exposing a builder
function (e.g., build_dataset_tab, build_lora_tab, build_lokr_tab) that returns
the Gradio components used in the final dict; then simplify
create_training_section to import and call those builders to assemble the Tabs,
merge their returned components into the single return dictionary (keeping
unique keys like "load_json_btn", "start_training_btn",
"start_lokr_training_btn", etc.), and ensure dataset_builder_state and
training_state remain created and included — this will keep each file <200 LOC
and preserve existing component names for event wiring.
---
Nitpick comments:
In `@acestep/ui/gradio/interfaces/css/main.css`:
- Around line 221-232: Replace the hardcoded hex values used by
.btn-primary-important and its hover state with the theme/CSS variables from
AceStepTheme (e.g. use the shared primary color variable and a darker variant)
so the button follows the global palette; update
.btn-primary-important:not(:disabled),
.btn-primary-important:not(:disabled):hover and body.dark
.btn-primary-important:disabled to reference those CSS vars (or add the vars to
the theme if missing) instead of `#5f00a6` / `#6d00bf` to avoid duplication and keep
styling consistent with the theme.
In `@acestep/ui/gradio/interfaces/generation.py`:
- Around line 1-953: This module is too large and should be split by
responsibility; extract the big builders into smaller modules and keep only
orchestration in this file. Create separate files (e.g., service_config.py,
advanced_accordions.py, generation_mode.py, optional_params.py, action_rows.py)
and move the corresponding UI builders: _create_service_config_content and
create_service_config_section -> service_config module; the
LoRA/DiT/LM/Output/Automation accordion blocks and
create_advanced_settings_section logic -> advanced_accordions module; the
create_generation_tab_section and its internal groups (generation_mode radio,
simple_mode_group, custom_mode_group, src_audio_row, repainting_group, generate
button row) -> generation_mode module; leave _compute_init_defaults in a small
utils module to be imported by the above. Update
create_advanced_settings_section and create_generation_tab_section to import
these new builders and only orchestrate composition, and keep
create_generation_section as a thin merger that imports the split functions;
ensure public symbol names (create_advanced_settings_section,
create_generation_tab_section, _create_service_config_content,
_compute_init_defaults) remain available (or re-export) so event wiring and
callers don't break, and run/adjust any event wiring/tests that reference the
moved components.
In `@acestep/ui/gradio/interfaces/theme.py`:
- Around line 17-32: The default arguments for the Theme (symbols: font and
font_mono) call fonts.LocalFont(...) at definition time (Ruff B008); move those
calls into module-level constants (e.g., MONTERRAT_LOCAL and IBM_PLEX_LOCAL) and
then use those constants in the default tuples for font and font_mono so the
LocalFont() constructors are executed at import time via explicit constants
rather than as default argument expressions.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
acestep/ui/gradio/events/__init__.pyacestep/ui/gradio/events/generation/mode_ui.pyacestep/ui/gradio/help_content.pyacestep/ui/gradio/interfaces/__init__.pyacestep/ui/gradio/interfaces/css/help_modal.cssacestep/ui/gradio/interfaces/css/main.cssacestep/ui/gradio/interfaces/css/utils.cssacestep/ui/gradio/interfaces/generation.pyacestep/ui/gradio/interfaces/result.pyacestep/ui/gradio/interfaces/theme.pyacestep/ui/gradio/interfaces/training.py
| generate_btn_update, # 2: generate_btn | ||
| False, # 3: simple_sample_created | ||
| gr.Accordion(visible=show_optional, open=False), # 4: optional_params_accordion | ||
| gr.Column(visible=show_optional), # 4: optional_params_section |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd acestep/ui/gradio/events/generation && wc -l mode_ui.pyRepository: ace-step/ACE-Step-1.5
Length of output: 79
🏁 Script executed:
cd acestep/ui/gradio/events/generation && sed -n '130,160p' mode_ui.pyRepository: ace-step/ACE-Step-1.5
Length of output: 1957
🏁 Script executed:
rg -n "gr\.(Column|Row|Accordion|Tab|Tabs|Blocks|Group)\(" --type py acestep/ui/gradio/events/ -A 2 -B 2Repository: ace-step/ACE-Step-1.5
Length of output: 2088
🏁 Script executed:
sed -n '100,155p' acestep/ui/gradio/events/generation/mode_ui.pyRepository: ace-step/ACE-Step-1.5
Length of output: 2905
🏁 Script executed:
sed -n '1,50p' acestep/ui/gradio/events/generation/mode_ui.pyRepository: ace-step/ACE-Step-1.5
Length of output: 1921
🏁 Script executed:
rg -n "def.*mode_ui|def.*on_generation_mode" acestep/ui/gradio/events/generation/mode_ui.py -A 30Repository: ace-step/ACE-Step-1.5
Length of output: 1374
🏁 Script executed:
rg -n "compute_mode_ui_updates" acestep/ui/gradio/ -B 2 -A 2Repository: ace-step/ACE-Step-1.5
Length of output: 12542
🏁 Script executed:
rg -n "handle_generation_mode_change\|on_generation_mode" acestep/ui/gradio/ -B 5 -A 15 --type pyRepository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
cd acestep && find . -name "__init__.py" -path "*/events/__init__.py" -exec cat {} \; | head -100Repository: ace-step/ACE-Step-1.5
Length of output: 4405
🏁 Script executed:
sed -n '200,260p' acestep/ui/gradio/events/__init__.pyRepository: ace-step/ACE-Step-1.5
Length of output: 2830
🏁 Script executed:
rg -n "generation_mode.*change\|\.change\(" acestep/ui/gradio/events/__init__.py -A 50 | head -150Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
rg -n "handle_generation_mode_change" acestep/ui/gradio/events/generation/mode_ui.py -A 50 | head -100Repository: ace-step/ACE-Step-1.5
Length of output: 4200
🏁 Script executed:
rg -n "generation_mode.*change" acestep/ui/gradio/events/__init__.py -B 2 -A 20 | head -80Repository: ace-step/ACE-Step-1.5
Length of output: 2516
🏁 Script executed:
rg -n "optional_params_section" acestep/ui/gradio/ -B 2 -A 2Repository: ace-step/ACE-Step-1.5
Length of output: 2626
🏁 Script executed:
sed -n '155,170p' acestep/ui/gradio/events/generation/ui_helpers.pyRepository: ace-step/ACE-Step-1.5
Length of output: 685
🏁 Script executed:
sed -n '100,120p' acestep/ui/gradio/events/generation/service_init.pyRepository: ace-step/ACE-Step-1.5
Length of output: 778
🏁 Script executed:
sed -n '25,45p' acestep/ui/gradio/events/results/audio_transfer.pyRepository: ace-step/ACE-Step-1.5
Length of output: 726
🏁 Script executed:
rg -n "update_audio_uploads_accordion" acestep/ui/gradio/ -B 3 -A 3Repository: ace-step/ACE-Step-1.5
Length of output: 1936
🏁 Script executed:
sed -n '100,130p' acestep/ui/gradio/events/generation/service_init.pyRepository: ace-step/ACE-Step-1.5
Length of output: 1358
🏁 Script executed:
rg -n "send_audio_to_remix\|send_audio_to_repaint" acestep/ui/gradio/events/ -B 5 -A 30 | head -150Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
rg -n "update_audio_uploads_accordion\(" acestep/ui/gradio/events/__init__.py -B 2 -A 8Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
sed -n '75,130p' acestep/ui/gradio/events/generation/service_init.pyRepository: ace-step/ACE-Step-1.5
Length of output: 2532
🏁 Script executed:
rg -n "send_audio_to_remix\|send_audio_to_repaint" acestep/ui/gradio/events/results/audio_transfer.py -B 3 -A 20Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
rg -n "reference_audio.*src_audio" acestep/ui/gradio/events/__init__.py -B 5 -A 10Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
rg -n "audio_uploads_accordion" acestep/ui/gradio/events/ -B 3 -A 3Repository: ace-step/ACE-Step-1.5
Length of output: 2493
🏁 Script executed:
sed -n '130,180p' acestep/ui/gradio/events/generation/service_init.pyRepository: ace-step/ACE-Step-1.5
Length of output: 2086
🏁 Script executed:
rg -n "reference_audio\|audio_uploads_accordion" acestep/ui/gradio/events/__init__.py -B 5 -A 10 | head -100Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
rg -n "\.change\(" acestep/ui/gradio/events/__init__.py | grep -A 2 -B 2 "reference_audio\|lm_codes_audio"Repository: ace-step/ACE-Step-1.5
Length of output: 296
🏁 Script executed:
sed -n '150,200p' acestep/ui/gradio/events/__init__.pyRepository: ace-step/ACE-Step-1.5
Length of output: 2006
🏁 Script executed:
sed -n '265,280p' acestep/ui/gradio/events/__init__.pyRepository: ace-step/ACE-Step-1.5
Length of output: 758
🏁 Script executed:
rg -n "init_service_wrapper\|on_tier_change" acestep/ui/gradio/events/__init__.py -B 2 -A 50 | head -120Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
python3 << 'EOF'
# Check Gradio version and behavior for component instantiation
try:
import gradio as gr
print(f"Gradio version: {gr.__version__}")
# Try to understand the Accordion/Column instantiation pattern
# In Gradio, returning a component instance vs gr.update()
print("\n--- Gradio semantics ---")
print("Event handler outputs should be:")
print("1. gr.update(...) for property updates")
print("2. gr.skip() to skip updating")
print("3. Plain values for simple components")
print("4. NOT new component instances")
except ImportError:
print("Gradio not installed in sandbox")
EOFRepository: ace-step/ACE-Step-1.5
Length of output: 96
🏁 Script executed:
sed -n '140,195p' acestep/ui/gradio/events/generation/mode_ui.pyRepository: ace-step/ACE-Step-1.5
Length of output: 3966
🏁 Script executed:
rg -n "def init_service_wrapper" acestep/ui/gradio/events/generation/service_init.py -A 100 | sed -n '1,60p'Repository: ace-step/ACE-Step-1.5
Length of output: 2860
🏁 Script executed:
rg -n "def send_audio_to_file" acestep/ui/gradio/events/results/audio_transfer.py -B 5 -A 30Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
sed -n '125,170p' acestep/ui/gradio/events/generation/service_init.pyRepository: ace-step/ACE-Step-1.5
Length of output: 2070
🏁 Script executed:
rg -n "def send_to_remix\|def send_to_repaint" acestep/ui/gradio/events/results/audio_transfer.py -B 2 -A 40 | head -120Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
grep -n "accordion_state" acestep/ui/gradio/events/generation/service_init.pyRepository: ace-step/ACE-Step-1.5
Length of output: 163
🏁 Script executed:
sed -n '50,100p' acestep/ui/gradio/events/__init__.pyRepository: ace-step/ACE-Step-1.5
Length of output: 2616
🏁 Script executed:
rg -n "init_btn.*click\|init_status" acestep/ui/gradio/events/__init__.py -B 5 -A 30 | head -80Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
rg -n "send_audio_to\|send_to_remix" acestep/ui/gradio/events/__init__.py -B 3 -A 15Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
rg -n "send_to_remix\|send_to_repaint\|send_audio_to" acestep/ui/gradio/events/ -B 2 -A 15 | head -150Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
python3 << 'EOF'
# Analyze the findings
findings = {
"mode_ui.py:148": {
"code": "gr.Column(visible=show_optional)",
"context": "return tuple (index 4) in compute_mode_ui_updates",
"used_by": "generation_mode.change event handler (events/__init__.py:415-426)",
"is_event_output": True
},
"ui_helpers.py:164": {
"code": "return gr.Accordion(open=has_audio)",
"context": "update_audio_uploads_accordion function",
"used_by": "Unknown - need to check",
"is_event_output": None
},
"service_init.py:114": {
"code": "accordion_state = gr.Accordion(open=not is_model_initialized)",
"context": "init_service_wrapper function",
"used_by": "init_btn.click event handler (index 2 in outputs)",
"is_event_output": True
},
"audio_transfer.py:37": {
"code": "gr.Accordion(open=True), # audio_uploads_accordion",
"context": "return tuple (index 9) in _send_audio_to_file",
"used_by": "Unknown - need to check",
"is_event_output": None
}
}
print("=" * 60)
print("CRITICAL ISSUES IDENTIFIED")
print("=" * 60)
for loc, info in findings.items():
print(f"\n{loc}:")
print(f" Code: {info['code']}")
print(f" Context: {info['context']}")
print(f" Used by: {info['used_by']}")
print(f" Is event output: {info['is_event_output']}")
print("\n" + "=" * 60)
print("SUMMARY")
print("=" * 60)
print("Confirmed event handler outputs with component instantiation:")
print("1. mode_ui.py:148 - gr.Column(...) in compute_mode_ui_updates")
print("2. service_init.py:114 - gr.Accordion(...) in init_service_wrapper")
print("\nNeed to verify:")
print("3. audio_transfer.py:37 - gr.Accordion(...) in return tuple")
print("4. ui_helpers.py:164 - gr.Accordion(...) usage context")
EOFRepository: ace-step/ACE-Step-1.5
Length of output: 1478
🏁 Script executed:
sed -n '20,50p' acestep/ui/gradio/events/results/audio_transfer.pyRepository: ace-step/ACE-Step-1.5
Length of output: 1033
🏁 Script executed:
rg -n "def _send_audio_to_file" acestep/ui/gradio/events/results/audio_transfer.py -A 40Repository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
rg -n "\.then\(|send_audio_to_file" acestep/ui/gradio/events/__init__.py -B 3 -A 15Repository: ace-step/ACE-Step-1.5
Length of output: 15726
gr.Column(...) must be gr.update(...) — component instantiation in an event-handler output causes a runtime error.
Line 148 returns a new gr.Column instance as an event output, which violates Gradio semantics. Event handlers expect gr.update() objects for property updates, not component instances. Every other element in this 44-item tuple correctly uses gr.update().
The same bug also exists in acestep/ui/gradio/events/generation/service_init.py:114 where gr.Accordion(open=not is_model_initialized) is returned as an event output in init_service_wrapper() instead of gr.update(open=...).
🐛 Proposed fix
- gr.Column(visible=show_optional), # 4: optional_params_section
+ gr.update(visible=show_optional), # 4: optional_params_sectionAlso fix service_init.py:114:
- accordion_state = gr.Accordion(open=not is_model_initialized)
+ accordion_state = gr.update(open=not is_model_initialized)📝 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.
Suggested change
| gr.Column(visible=show_optional), # 4: optional_params_section | |
| gr.update(visible=show_optional), # 4: optional_params_section |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/gradio/events/generation/mode_ui.py` at line 148, Replace
component instantiation returns with gr.update property updates: in the
generation event handler where the tuple currently returns
gr.Column(visible=show_optional) (the optional_params_section output), change
that to gr.update(visible=show_optional); similarly, in init_service_wrapper()
where the code returns gr.Accordion(open=not is_model_initialized), replace it
with gr.update(open=not is_model_initialized). Locate the return tuples in the
event handler (the one returning optional_params_section) and in
init_service_wrapper() and swap component constructors for gr.update(...) using
the same property names (visible/open) and boolean expressions.
Comment on lines
+194
to
+195
| css_file = Path(__file__).parent / "interfaces" / "css" / "help_modal.css" | ||
| HELP_MODAL_CSS = css_file.read_text() |
Contributor
There was a problem hiding this comment.
Unhandled FileNotFoundError at module-level — missing CSS file crashes the entire import.
css_file.read_text() is evaluated at import time. If help_modal.css is absent (packaging issue, wrong working directory, etc.), this raises FileNotFoundError and makes the whole help_content module (and every importer) fail to load.
Also missing an explicit encoding parameter — on Windows with a non-UTF-8 locale this can silently mangle characters.
🛡️ Proposed fix
-css_file = Path(__file__).parent / "interfaces" / "css" / "help_modal.css"
-HELP_MODAL_CSS = css_file.read_text()
+_css_file = Path(__file__).parent / "interfaces" / "css" / "help_modal.css"
+try:
+ HELP_MODAL_CSS = _css_file.read_text(encoding="utf-8")
+except FileNotFoundError:
+ logger.warning(f"help_modal.css not found at {_css_file}; help modal will be unstyled.")
+ HELP_MODAL_CSS = ""As per coding guidelines: "Handle errors explicitly in Python; avoid bare except."
📝 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.
Suggested change
| css_file = Path(__file__).parent / "interfaces" / "css" / "help_modal.css" | |
| HELP_MODAL_CSS = css_file.read_text() | |
| _css_file = Path(__file__).parent / "interfaces" / "css" / "help_modal.css" | |
| try: | |
| HELP_MODAL_CSS = _css_file.read_text(encoding="utf-8") | |
| except FileNotFoundError: | |
| logger.warning(f"help_modal.css not found at {_css_file}; help modal will be unstyled.") | |
| HELP_MODAL_CSS = "" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/gradio/help_content.py` around lines 194 - 195, The module
currently reads the CSS file at import time via css_file.read_text() and assigns
it to HELP_MODAL_CSS which will raise FileNotFoundError (or encoding errors) and
break imports; change this to lazily load or safely read the file with explicit
encoding='utf-8' and a guarded try/except that catches FileNotFoundError and
UnicodeDecodeError, returning a sensible fallback (e.g., empty string or bundled
default CSS) and emitting a warning; update references to css_file and
HELP_MODAL_CSS so the module no longer performs an unguarded read at import time
and instead uses the safe loader.
Comment on lines
+93
to
+96
| with gr.Tab(t("training.tab_title"), visible=not service_mode): | ||
| training_section = create_training_section( | ||
| dit_handler, llm_handler, init_params=init_params | ||
| ) |
Contributor
There was a problem hiding this comment.
Gate training handler registration with service_mode as well.
Line 93 hides the Training tab, but Line 114 still registers all training callbacks. That keeps training pathways active even in service mode.
🔧 Suggested fix
- # --- Training Tab ---
- with gr.Tab(t("training.tab_title"), visible=not service_mode):
- training_section = create_training_section(
- dit_handler, llm_handler, init_params=init_params
- )
+ # --- Training Tab ---
+ if not service_mode:
+ with gr.Tab(t("training.tab_title"), visible=True):
+ training_section = create_training_section(
+ dit_handler, llm_handler, init_params=init_params
+ )
@@
- # Connect training event handlers
- setup_training_event_handlers(demo, dit_handler, llm_handler, training_section)
+ # Connect training event handlers
+ if not service_mode:
+ setup_training_event_handlers(demo, dit_handler, llm_handler, training_section)As per coding guidelines "Gate WIP or unstable UI/API paths behind explicit feature/release flags".
Also applies to: 113-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/gradio/interfaces/__init__.py` around lines 93 - 96, The Training
UI and its callbacks are only hidden but not disabled in service_mode; change
the code so registration of training callbacks is also conditional on
service_mode: when creating the Tab with create_training_section(dit_handler,
llm_handler, init_params) only call whatever registers training callbacks (e.g.,
methods on training_section or any subsequent calls that attach handlers) if
visible/if not service_mode—i.e., wrap the callback registration logic the same
way the Tab visibility is gated so that in service_mode both the UI and the
training handlers are not registered.
Comment on lines
+38
to
+42
| .help-inline-btn:hover { | ||
| color: var(--color-accent, #4a9eff); | ||
| border-color: var(--color-accent, #4a9eff); | ||
| transform: scale(1.1); | ||
| } |
Contributor
There was a problem hiding this comment.
Add :focus-visible styles to help button for keyboard accessibility.
.help-inline-btn and .help-modal-close both lack focus indicators. Users navigating via keyboard will have no visible focus state when tabbing to these elements.
♿ Proposed addition
.help-inline-btn:hover {
color: var(--color-accent, `#4a9eff`);
border-color: var(--color-accent, `#4a9eff`);
transform: scale(1.1);
}
+
+.help-inline-btn:focus-visible {
+ outline: 2px solid var(--color-accent, `#4a9eff`);
+ outline-offset: 2px;
+}And similarly for .help-modal-close:
.help-modal-close:hover {
opacity: 1;
}
+
+.help-modal-close:focus-visible {
+ outline: 2px solid var(--color-accent, `#4a9eff`);
+ outline-offset: 2px;
+ opacity: 1;
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/gradio/interfaces/css/help_modal.css` around lines 38 - 42, Add
keyboard focus-visible styles for the help button and close button so keyboard
users get a clear focus indicator: update the CSS for .help-inline-btn and
.help-modal-close to include :focus-visible rules (mirroring the hover visual
language) that set a visible outline or border-color using var(--color-accent)
and preserve/adjust transform (e.g., scale) and include an accessible
outline-offset; specifically add .help-inline-btn:focus-visible and
.help-modal-close:focus-visible selectors to the stylesheet so both elements
show a clear, consistent focus state.
Comment on lines
326
to
339
| with gr.Accordion("🔧 LoRA Adapter", open=False, elem_classes=["has-info-container"]): | ||
| with gr.Row(): | ||
| lora_path = gr.Textbox(label="LoRA Path", placeholder="./lora_output/final/adapter", info="Path to trained LoRA adapter directory", scale=3) | ||
| load_lora_btn = gr.Button("📥 Load LoRA", variant="secondary", scale=1) | ||
| unload_lora_btn = gr.Button("🗑️ Unload", variant="secondary", scale=1) | ||
| with gr.Column(): | ||
| lora_path = gr.Textbox(label="LoRA Path", placeholder="./lora_output/final/adapter", info="Path to trained LoRA adapter directory", scale=3, elem_classes=["has-info-container"]) | ||
| load_lora_btn = gr.Button("📥 Load LoRA", variant="secondary", scale=1) | ||
| unload_lora_btn = gr.Button("🗑️ Unload", variant="secondary", scale=1) | ||
| with gr.Row(): | ||
| with gr.Column(): | ||
| use_lora_checkbox = gr.Checkbox(label="Use LoRA", value=False, info="Enable LoRA adapter for inference", scale=1, elem_classes=["has-info-container"]) | ||
| lora_scale_slider = gr.Slider(minimum=0.0, maximum=1.0, value=1.0, step=0.05, label="LoRA Scale", info="LoRA influence strength (0=disabled, 1=full)", scale=2, elem_classes=["has-info-container"]) | ||
|
|
||
| with gr.Row(): | ||
| use_lora_checkbox = gr.Checkbox(label="Use LoRA", value=False, info="Enable LoRA adapter for inference", scale=1) | ||
| lora_scale_slider = gr.Slider(minimum=0.0, maximum=1.0, value=1.0, step=0.05, label="LoRA Scale", info="LoRA influence strength (0=disabled, 1=full)", scale=2) | ||
| lora_status = gr.Textbox(label="LoRA Status", value="No LoRA loaded", interactive=False, lines=1, elem_classes=["no-tooltip"]) | ||
| lora_status = gr.Textbox(label="LoRA Status", value="No LoRA loaded", interactive=False, lines=1, elem_classes=["no-tooltip"]) | ||
|
|
Contributor
There was a problem hiding this comment.
Localize new LoRA UI strings to avoid mixed-language rendering.
This block introduces hardcoded English labels/info ("LoRA Path", "Load LoRA", "LoRA Status", etc.) while the rest of the page uses t(...). In non-English mode this section will not translate.
🌐 Suggested fix pattern
- with gr.Accordion("🔧 LoRA Adapter", open=False, elem_classes=["has-info-container"]):
+ with gr.Accordion(t("generation.lora_section_title"), open=False, elem_classes=["has-info-container"]):
@@
- lora_path = gr.Textbox(label="LoRA Path", placeholder="./lora_output/final/adapter", info="Path to trained LoRA adapter directory", scale=3, elem_classes=["has-info-container"])
- load_lora_btn = gr.Button("📥 Load LoRA", variant="secondary", scale=1)
- unload_lora_btn = gr.Button("🗑️ Unload", variant="secondary", scale=1)
+ lora_path = gr.Textbox(
+ label=t("generation.lora_path_label"),
+ placeholder=t("generation.lora_path_placeholder"),
+ info=t("generation.lora_path_info"),
+ scale=3,
+ elem_classes=["has-info-container"],
+ )
+ load_lora_btn = gr.Button(t("generation.load_lora_btn"), variant="secondary", scale=1)
+ unload_lora_btn = gr.Button(t("generation.unload_lora_btn"), variant="secondary", scale=1)
@@
- use_lora_checkbox = gr.Checkbox(label="Use LoRA", value=False, info="Enable LoRA adapter for inference", scale=1, elem_classes=["has-info-container"])
- lora_scale_slider = gr.Slider(minimum=0.0, maximum=1.0, value=1.0, step=0.05, label="LoRA Scale", info="LoRA influence strength (0=disabled, 1=full)", scale=2, elem_classes=["has-info-container"])
+ use_lora_checkbox = gr.Checkbox(
+ label=t("generation.use_lora_label"),
+ value=False,
+ info=t("generation.use_lora_info"),
+ scale=1,
+ elem_classes=["has-info-container"],
+ )
+ lora_scale_slider = gr.Slider(
+ minimum=0.0,
+ maximum=1.0,
+ value=1.0,
+ step=0.05,
+ label=t("generation.lora_scale_label"),
+ info=t("generation.lora_scale_info"),
+ scale=2,
+ elem_classes=["has-info-container"],
+ )
@@
- lora_status = gr.Textbox(label="LoRA Status", value="No LoRA loaded", interactive=False, lines=1, elem_classes=["no-tooltip"])
+ lora_status = gr.Textbox(
+ label=t("generation.lora_status_label"),
+ value=t("generation.lora_status_empty"),
+ interactive=False,
+ lines=1,
+ elem_classes=["no-tooltip"],
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/gradio/interfaces/generation.py` around lines 326 - 339, Replace
the hardcoded English UI strings in the LoRA accordion with localized calls;
update the label/placeholder/info/value arguments for lora_path, load_lora_btn,
unload_lora_btn, use_lora_checkbox, lora_scale_slider, and lora_status to use
the translation helper t(...) (or the project’s i18n function) instead of raw
strings so these controls render in the active language—ensure placeholders,
button text, checkbox label, slider label/info, and the default status value all
call t(...) with appropriate keys.
Comment on lines
1
to
127
| from __future__ import annotations | ||
| from typing import Iterable | ||
| import gradio as gr | ||
| from gradio.themes.utils import colors, fonts, sizes | ||
|
|
||
|
|
||
| class AceStepTheme(gr.themes.Base): | ||
| def __init__( | ||
| self, | ||
| *, | ||
| primary_hue: colors.Color | str = colors.slate, | ||
| secondary_hue: colors.Color | str = colors.indigo, | ||
| neutral_hue: colors.Color | str = colors.gray, | ||
| spacing_size: sizes.Size | str = sizes.spacing_md, | ||
| radius_size: sizes.Size | str = sizes.radius_md, | ||
| text_size: sizes.Size | str = sizes.text_md, | ||
| font: fonts.Font | ||
| | str | ||
| | Iterable[fonts.Font | str] = ( | ||
| fonts.LocalFont("Montserrat"), | ||
| "ui-sans-serif", | ||
| "system-ui", | ||
| "sans-serif", | ||
| ), | ||
| font_mono: fonts.Font | ||
| | str | ||
| | Iterable[fonts.Font | str] = ( | ||
| fonts.LocalFont("IBM Plex Mono"), | ||
| "ui-monospace", | ||
| "Consolas", | ||
| "monospace", | ||
| ), | ||
| ): | ||
| super().__init__( | ||
| primary_hue=primary_hue, | ||
| secondary_hue=secondary_hue, | ||
| neutral_hue=neutral_hue, | ||
| spacing_size=spacing_size, | ||
| radius_size=radius_size, | ||
| text_size=text_size, | ||
| font=font, | ||
| font_mono=font_mono, | ||
| ) | ||
|
|
||
| super().set( | ||
| # Custom values | ||
| button_primary_background_fill="*neutral_950", | ||
| button_primary_background_fill_dark='*primary_100', | ||
| button_primary_text_color_dark='*neutral_950', | ||
| button_secondary_background_fill="*primary_200", | ||
| button_secondary_text_color="*neutral_950", | ||
| body_background_fill='#f3f4f5', | ||
| block_background_fill_dark='#0f1624', | ||
| input_background_fill_dark='*neutral_800', | ||
| block_border_color='white', | ||
| block_border_color_dark='#0f1624', | ||
| slider_color='*neutral_800', | ||
| slider_color_dark='*primary_200', | ||
| color_accent='white', | ||
|
|
||
| # The rest of the values were taken from Gradio's Soft theme | ||
| # Colors | ||
| background_fill_primary="*neutral_50", | ||
|
|
||
| # Shadows | ||
| shadow_drop="0 1px 4px 0 rgb(0 0 0 / 0.1)", | ||
| shadow_drop_lg="0 2px 5px 0 rgb(0 0 0 / 0.1)", | ||
|
|
||
| # Block Labels | ||
| block_background_fill="white", | ||
| block_label_padding="*spacing_sm *spacing_md", | ||
| block_label_radius="*radius_md", | ||
| block_label_text_size="*text_md", | ||
| block_label_text_weight="600", | ||
| block_label_text_color="*primary_500", | ||
| block_label_text_color_dark="white", | ||
|
|
||
| block_title_radius="*block_label_radius", | ||
| block_title_padding="*block_label_padding", | ||
| block_title_background_fill="*block_label_background_fill", | ||
| block_title_text_weight="600", | ||
| block_title_text_color="*primary_500", | ||
| block_title_text_color_dark="white", | ||
| block_label_margin="*spacing_md", | ||
|
|
||
| # Inputs | ||
| input_background_fill="white", | ||
| input_border_color="*neutral_50", | ||
| input_shadow="*shadow_drop", | ||
| input_shadow_focus="*shadow_drop_lg", | ||
| checkbox_shadow="none", | ||
|
|
||
| # Buttons | ||
| shadow_spread="6px", | ||
| button_primary_shadow="*shadow_drop_lg", | ||
| button_primary_shadow_hover="*shadow_drop_lg", | ||
| button_primary_shadow_active="*shadow_inset", | ||
| button_secondary_shadow="*shadow_drop_lg", | ||
| button_secondary_shadow_hover="*shadow_drop_lg", | ||
| button_secondary_shadow_active="*shadow_inset", | ||
| checkbox_label_shadow="*shadow_drop_lg", | ||
| button_primary_background_fill_hover_dark="*primary_500", | ||
| button_primary_text_color="white", | ||
| button_secondary_background_fill_hover_dark="*primary_500", | ||
| button_cancel_background_fill="*button_secondary_background_fill", | ||
| button_cancel_background_fill_hover="*button_secondary_background_fill_hover", | ||
| button_cancel_background_fill_hover_dark="*button_secondary_background_fill_hover", | ||
| button_cancel_text_color="*button_secondary_text_color", | ||
| checkbox_label_background_fill_selected="*primary_500", | ||
| checkbox_label_background_fill_selected_dark="*primary_600", | ||
| checkbox_border_width="1px", | ||
| checkbox_border_color="*neutral_100", | ||
| checkbox_border_color_dark="*neutral_600", | ||
| checkbox_background_color_selected="*primary_600", | ||
| checkbox_background_color_selected_dark="*primary_700", | ||
| checkbox_border_color_focus="*primary_500", | ||
| checkbox_border_color_focus_dark="*primary_600", | ||
| checkbox_border_color_selected="*primary_600", | ||
| checkbox_border_color_selected_dark="*primary_700", | ||
| checkbox_label_text_color_selected="white", | ||
|
|
||
| # Borders | ||
| block_border_width="0px", | ||
| panel_border_width="1px", | ||
| ) | ||
|
|
||
| theme = AceStepTheme() No newline at end of file |
Contributor
There was a problem hiding this comment.
Missing docstrings on module, class, and __init__.
All three are required by the coding guidelines but are absent. A minimal example:
+"""Gradio theme module for ACE-Step UI.
+
+Defines AceStepTheme, a custom subclass of gr.themes.Base,
+and exposes a default ``theme`` instance for use by the interface layer.
+"""
from __future__ import annotations class AceStepTheme(gr.themes.Base):
+ """Custom Gradio theme for ACE-Step with dark/light token overrides."""
+
def __init__( def __init__(
self,
*,
...
):
+ """Initialise theme with optional hue, size, radius, text, and font settings.
+
+ Args:
+ primary_hue: Primary colour hue or Color object.
+ secondary_hue: Secondary colour hue or Color object.
+ neutral_hue: Neutral colour hue or Color object.
+ spacing_size: Spacing size token.
+ radius_size: Border-radius size token.
+ text_size: Text size token.
+ font: Body font stack.
+ font_mono: Monospace font stack.
+ """As per coding guidelines: "Docstrings are mandatory for all new or modified Python modules, classes, and functions."
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 20-20: Do not perform function call fonts.LocalFont in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 28-28: Do not perform function call fonts.LocalFont in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/gradio/interfaces/theme.py` around lines 1 - 127, Add concise
docstrings to the module (top of the file), the AceStepTheme class, and its
__init__ method: for the module include a one-line summary of the theme
utilities provided; for class AceStepTheme add a short description of the
theme's purpose; for __init__ document the parameters (primary_hue,
secondary_hue, neutral_hue, spacing_size, radius_size, text_size, font,
font_mono) and any important behavior (it calls super().__init__ and sets custom
theme values via super().set); keep all three minimal but present and follow the
project's docstring style (one-line summary plus brief Args when appropriate).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
acestep/ui/gradio/interfaces/theme.py (1)
1-33:⚠️ Potential issue | 🟡 MinorAdd required docstrings for the module, class, and
__init__.These new/modified Python symbols are missing mandatory docstrings.
🔧 Minimal patch
+"""Custom Gradio theme definitions for the ACE-Step UI.""" from __future__ import annotations from typing import Iterable import gradio as gr from gradio.themes.utils import colors, fonts, sizes class AceStepTheme(gr.themes.Base): + """Centralized Gradio theme tokens for the ACE-Step interface.""" + def __init__( self, *, @@ - ): + ) -> None: + """Initialize hue, sizing, and font token configuration. + + Args: + primary_hue: Primary color hue token. + secondary_hue: Secondary color hue token. + neutral_hue: Neutral color hue token. + spacing_size: Spacing scale token. + radius_size: Radius scale token. + text_size: Text-size scale token. + font: Body font stack token(s). + font_mono: Monospace font stack token(s). + """ super().__init__(As per coding guidelines: "Docstrings are mandatory for all new or modified Python modules, classes, and functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/interfaces/theme.py` around lines 1 - 33, Add missing docstrings: add a module-level docstring at the top describing the theme module's purpose; add a class docstring for AceStepTheme that briefly explains what the theme configures and when to use it; add an __init__ docstring describing the constructor and documenting key parameters (primary_hue, secondary_hue, neutral_hue, spacing_size, radius_size, text_size, font, font_mono) including their types and default behavior. Ensure the docstrings follow project style (one-line summary + short description and parameter list) and attach them to the module, the AceStepTheme class, and the __init__ method respectively.
🧹 Nitpick comments (2)
acestep/ui/gradio/interfaces/css/main.css (1)
59-65::has()selector support is now broadly available; fallback is optional rather than critical.As of Feb 2026,
:has()is stable and well-supported: Chrome 105+, Firefox 121+, Safari 15.4+, Android WebView 105+, and iOS 15.4+. The selector reached "Baseline Newly Available" status in Dec 2023 and is expected to be "Baseline Widely Available" by June 2026.Lines 59–65 and 132 use
:has()to ensure parent containers allow tooltip overflow. On older browsers lacking:has()support, this gracefully degrades (tooltips may clip), but the core UI remains functional. Given the application's modern Gradio architecture and lack of documented legacy browser support requirements, a class-based fallback is recommended as an optional enhancement for extra robustness rather than a critical requirement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/interfaces/css/main.css` around lines 59 - 65, The current CSS relies on the parent :has(.has-info-container) selector (used with .row, .column, .form, .accordion, .tabs, .gr-block, .gr-box) to allow tooltip overflow; since :has() is broadly supported but may still be missing in some environments, add an optional class-based fallback by updating markup to add a parent class (e.g., .has-info-parent) when a .has-info-container is present and duplicate the CSS rules to target .row.has-info-parent, .column.has-info-parent, .form.has-info-parent, .accordion.has-info-parent, .tabs.has-info-parent, .gr-block.has-info-parent, .gr-box.has-info-parent so the same overflow behavior applies on browsers without :has(). Ensure the code that creates .has-info-container also toggles .has-info-parent on its nearest parent container (or add it during render) and keep the existing :has(...) rules intact.acestep/ui/gradio/interfaces/theme.py (1)
17-32: Extract tuple defaults to module-level constants to resolve B008 violations.The
fontandfont_monoparameters use tuples containingfonts.LocalFont(...)calls as defaults. These function calls execute at definition time, triggering B008. ExtractDEFAULT_FONT_STACKandDEFAULT_FONT_MONO_STACKas module-level constants, then useNonedefaults with explicit assignment in__init__.Suggested refactoring
+DEFAULT_FONT_STACK: tuple[fonts.Font | str, ...] = ( + fonts.LocalFont("Montserrat"), + "ui-sans-serif", + "system-ui", + "sans-serif", +) + +DEFAULT_FONT_MONO_STACK: tuple[fonts.Font | str, ...] = ( + fonts.LocalFont("IBM Plex Mono"), + "ui-monospace", + "Consolas", + "monospace", +) + class AceStepTheme(gr.themes.Base): def __init__( - font: fonts.Font - | str - | Iterable[fonts.Font | str] = ( - fonts.LocalFont("Montserrat"), - "ui-sans-serif", - "system-ui", - "sans-serif", - ), - font_mono: fonts.Font - | str - | Iterable[fonts.Font | str] = ( - fonts.LocalFont("IBM Plex Mono"), - "ui-monospace", - "Consolas", - "monospace", - ), + font: fonts.Font | str | Iterable[fonts.Font | str] | None = None, + font_mono: fonts.Font | str | Iterable[fonts.Font | str] | None = None, ) -> None: + if font is None: + font = DEFAULT_FONT_STACK + if font_mono is None: + font_mono = DEFAULT_FONT_MONO_STACK🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/interfaces/theme.py` around lines 17 - 32, Extract the tuple defaults that call fonts.LocalFont into module-level constants (e.g., DEFAULT_FONT_STACK and DEFAULT_FONT_MONO_STACK) containing the same tuple values, change the parameter defaults for font and font_mono to None in the class/constructor signature, and in __init__ assign self.font = font or DEFAULT_FONT_STACK and self.font_mono = font_mono or DEFAULT_FONT_MONO_STACK; update references to the symbols font, font_mono, DEFAULT_FONT_STACK, DEFAULT_FONT_MONO_STACK, and __init__ accordingly so the LocalFont calls are not executed at definition time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@acestep/ui/gradio/interfaces/css/main.css`:
- Around line 72-75: The CSS selectors like ".checkbox-container+div" and the
sibling rules alongside ".has-info-container
span[data-testid=\"block-info\"]+div" are too global and hide/affect sibling
divs outside the tooltip feature; scope them by prefixing with the tooltip
opt-in container class (e.g., ".has-info-container") so only rows inside
opted-in containers are targeted (update every occurrence including the
selectors shown and those at the other noted locations: the rules near lines
124-127, 132-133, and 153-155) — modify selectors such that
".checkbox-container+div" becomes ".has-info-container .checkbox-container+div"
(and similarly scope the other sibling selectors) so only elements inside the
tooltip-enabled container are affected.
- Around line 75-76: The tooltip rules at the CSS block around lines 75–76 (and
the similar block at 123–128) hide helper text with display:none which prevents
keyboard focus and screen-reader access; change the hide/show strategy to use
visibility: hidden / visibility: visible and opacity: 0 / opacity: 1 (with
pointer-events: none when hidden) and add reveal selectors for :focus,
:focus-visible and :focus-within in addition to :hover so keyboard users can
open tooltips; keep position: absolute, add a small transition for opacity, and
ensure the tooltip element itself is keyboard-focusable (tabindex) and
referenced via aria-describedby so the new focus-based rules work for assistive
tech.
---
Duplicate comments:
In `@acestep/ui/gradio/interfaces/theme.py`:
- Around line 1-33: Add missing docstrings: add a module-level docstring at the
top describing the theme module's purpose; add a class docstring for
AceStepTheme that briefly explains what the theme configures and when to use it;
add an __init__ docstring describing the constructor and documenting key
parameters (primary_hue, secondary_hue, neutral_hue, spacing_size, radius_size,
text_size, font, font_mono) including their types and default behavior. Ensure
the docstrings follow project style (one-line summary + short description and
parameter list) and attach them to the module, the AceStepTheme class, and the
__init__ method respectively.
---
Nitpick comments:
In `@acestep/ui/gradio/interfaces/css/main.css`:
- Around line 59-65: The current CSS relies on the parent
:has(.has-info-container) selector (used with .row, .column, .form, .accordion,
.tabs, .gr-block, .gr-box) to allow tooltip overflow; since :has() is broadly
supported but may still be missing in some environments, add an optional
class-based fallback by updating markup to add a parent class (e.g.,
.has-info-parent) when a .has-info-container is present and duplicate the CSS
rules to target .row.has-info-parent, .column.has-info-parent,
.form.has-info-parent, .accordion.has-info-parent, .tabs.has-info-parent,
.gr-block.has-info-parent, .gr-box.has-info-parent so the same overflow behavior
applies on browsers without :has(). Ensure the code that creates
.has-info-container also toggles .has-info-parent on its nearest parent
container (or add it during render) and keep the existing :has(...) rules
intact.
In `@acestep/ui/gradio/interfaces/theme.py`:
- Around line 17-32: Extract the tuple defaults that call fonts.LocalFont into
module-level constants (e.g., DEFAULT_FONT_STACK and DEFAULT_FONT_MONO_STACK)
containing the same tuple values, change the parameter defaults for font and
font_mono to None in the class/constructor signature, and in __init__ assign
self.font = font or DEFAULT_FONT_STACK and self.font_mono = font_mono or
DEFAULT_FONT_MONO_STACK; update references to the symbols font, font_mono,
DEFAULT_FONT_STACK, DEFAULT_FONT_MONO_STACK, and __init__ accordingly so the
LocalFont calls are not executed at definition time.
Comment on lines
+72
to
+75
| .has-info-container span[data-testid="block-info"]+div, | ||
| .has-info-container span[data-testid="block-info"]+span, | ||
| .checkbox-container+div { | ||
| display: none; |
Contributor
There was a problem hiding this comment.
Scope the checkbox tooltip selectors to opted-in containers.
On Line 74 (.checkbox-container+div) and related rules on Line 126/Line 132/Line 154, the selectors are global. This can unintentionally hide/style sibling <div> content for checkbox rows outside the tooltip feature.
🔧 Proposed fix
-.has-info-container span[data-testid="block-info"]+div,
-.has-info-container span[data-testid="block-info"]+span,
-.checkbox-container+div {
+.has-info-container span[data-testid="block-info"]+div,
+.has-info-container span[data-testid="block-info"]+span,
+.has-info-container .checkbox-container+div {
@@
-.has-info-container span[data-testid="block-info"]:hover+div,
-.has-info-container span[data-testid="block-info"]:hover+span,
-.checkbox-container:hover+div {
+.has-info-container span[data-testid="block-info"]:hover+div,
+.has-info-container span[data-testid="block-info"]:hover+span,
+.has-info-container .checkbox-container:hover+div {
@@
-.has-info-container span[data-testid="block-info"]::after,
-.checkbox-container:has(+ div) .label-text::after {
+.has-info-container span[data-testid="block-info"]::after,
+.has-info-container .checkbox-container:has(+ div) .label-text::after {
@@
-.has-info-container span[data-testid="block-info"]:hover::after,
-.checkbox-container:hover .label-text::after {
+.has-info-container span[data-testid="block-info"]:hover::after,
+.has-info-container .checkbox-container:hover .label-text::after {Also applies to: 124-127, 132-133, 153-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/gradio/interfaces/css/main.css` around lines 72 - 75, The CSS
selectors like ".checkbox-container+div" and the sibling rules alongside
".has-info-container span[data-testid=\"block-info\"]+div" are too global and
hide/affect sibling divs outside the tooltip feature; scope them by prefixing
with the tooltip opt-in container class (e.g., ".has-info-container") so only
rows inside opted-in containers are targeted (update every occurrence including
the selectors shown and those at the other noted locations: the rules near lines
124-127, 132-133, and 153-155) — modify selectors such that
".checkbox-container+div" becomes ".has-info-container .checkbox-container+div"
(and similarly scope the other sibling selectors) so only elements inside the
tooltip-enabled container are affected.
Comment on lines
+75
to
+76
| display: none; | ||
| position: absolute; |
Contributor
There was a problem hiding this comment.
Add keyboard-accessible tooltip reveal states.
Current tooltip visibility is hover-only (display: none → show on :hover). On Line 75 and Line 124-127, keyboard users may not be able to access helper text.
♿ Proposed fix
.has-info-container span[data-testid="block-info"]:hover+div,
.has-info-container span[data-testid="block-info"]:hover+span,
.has-info-container .checkbox-container:hover+div {
display: block !important;
}
+
+/* Keyboard accessibility */
+.has-info-container:focus-within span[data-testid="block-info"]+div,
+.has-info-container:focus-within span[data-testid="block-info"]+span,
+.has-info-container .checkbox-container:focus-within+div {
+ display: block !important;
+}Also applies to: 123-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/gradio/interfaces/css/main.css` around lines 75 - 76, The tooltip
rules at the CSS block around lines 75–76 (and the similar block at 123–128)
hide helper text with display:none which prevents keyboard focus and
screen-reader access; change the hide/show strategy to use visibility: hidden /
visibility: visible and opacity: 0 / opacity: 1 (with pointer-events: none when
hidden) and add reveal selectors for :focus, :focus-visible and :focus-within in
addition to :hover so keyboard users can open tooltips; keep position: absolute,
add a small transition for opacity, and ensure the tooltip element itself is
keyboard-focusable (tabindex) and referenced via aria-describedby so the new
focus-based rules work for assistive tech.
Contributor
|
I want the settings panel on the left to be collapsible. Is that possible? |
Author
|
Gradio has a Sidebar component. Maybe we could tweak it. This would be great for small screens. 2026-02-25.14-06-03.webm |
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
2 participants
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.
Implements #695
For some reason I wasn't able to fully change the slider's color for Chrome. It only worked for Firefox. The above screenshot is from Firefox.
Summary by CodeRabbit
Style
Refactor