refactor(gradio-events): extract generation run and batch navigation wiring modules (PR5)#655
Open
1larity wants to merge 5 commits intoace-step:mainfrom
Open
refactor(gradio-events): extract generation run and batch navigation wiring modules (PR5)#6551larity wants to merge 5 commits intoace-step:mainfrom
1larity wants to merge 5 commits intoace-step:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughRefactors Gradio event wiring into modular registration helpers (service, metadata, mode, run, batch navigation, text-format, results aux), replaces inline wiring in the main events module with these registrations, and adds AST-based tests to verify delegation contracts. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Gradio UI
participant GenW as GenerationWiringContext
participant ResH as ResultsHandler
participant DIT as DIT Handler
participant LLM as LLM Handler
UI->>GenW: click generate_btn (inputs...)
GenW->>ResH: clear_audio_outputs_for_new_generation()
GenW->>ResH: generate_with_batch_management(inputs..., DIT, LLM)
ResH-->>UI: update generated audio, captions, status, scores...
ResH->>DIT: schedule generate_next_batch_background(params...)
DIT-->>GenW: update batch_queue / next_batch_status
GenW-->>UI: update batch controls (next/prev/queue/status)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 1
🤖 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/wiring/results_aux_wiring.py`:
- Line 25: Replace the mojibake sequence "—" in the comment near
results_aux_wiring.py with a proper em dash (—); specifically update the comment
that references generation_mode.change (currently "# Mode-UI outputs shared with
generation_mode.change — applied atomically") to use a real em dash so it
reads "# Mode-UI outputs shared with generation_mode.change — applied
atomically".
…wiring modules (PR5)
1larity
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/__init__.py`:
- Around line 39-54: Update the setup_event_handlers function docstring to list
its purpose and the key inputs/outputs introduced: document the wiring_context
parameter and the outputs returned/registered (auto_checkbox_inputs,
auto_checkbox_outputs from register_generation_service_handlers and
mode_ui_outputs from build_mode_ui_outputs) and note that these are forwarded to
register_generation_metadata_handlers and register_generation_mode_handlers;
keep it concise and follow existing docstring style including types and brief
descriptions for wiring_context, auto_checkbox_inputs, auto_checkbox_outputs,
and mode_ui_outputs.
In `@acestep/ui/gradio/events/wiring/generation_batch_navigation_wiring.py`:
- Around line 131-133: Update the docstring for
register_generation_batch_navigation_handlers to include a one-line purpose plus
clear inputs/outputs: state that it registers previous/next batch navigation
handlers and pre-generation chain; document the input parameter "context" of
type GenerationWiringContext and describe any important fields used from it
(e.g., UI components or event buses); specify the return value (None) and any
exceptions it may raise (if any). Keep it concise, follow existing docstring
style in the file, and ensure the function signature name
register_generation_batch_navigation_handlers and type GenerationWiringContext
are mentioned for discoverability.
- Around line 13-128: Update the two helper docstrings to explicitly document
parameters, return values and key elements of the returned lists: for
_build_navigation_outputs(results_section: dict[str, Any], include_next_status:
bool) state that results_section is a mapping of output component keys (list the
main groups: generated_audio_1..8, generated_audio_batch, generation_info,
current_batch_index, batch_indicator, prev/next buttons, status_output and
optional next_batch_status, followed by score_display_1..8, codes_display_1..8,
lrc_display_1..8, details_accordion_1..8 and restore_params_btn), explain
include_next_status controls presence of next_batch_status, and specify the
return type list[Any]; for
_build_capture_current_params_inputs(generation_section: dict[str, Any])
document that generation_section is a mapping of generation control keys (list
representative keys like captions, lyrics, bpm, key_scale, inference_steps,
guidance_scale, random_seed_checkbox, seed, reference_audio, batch_size_input,
audio_format, LM/COT related keys, auto_score/auto_lrc/score_scale,
normalization flags, latent_shift/rescale, etc.) and that the function returns
an ordered list[Any] of those control values for restoring generation state.
Ensure concise “Args:” and “Returns:” sections follow project docstring style.
In `@acestep/ui/gradio/events/wiring/generation_run_wiring.py`:
- Around line 11-23: The docstrings for register_generation_run_handlers and the
inner generation_wrapper are missing concise input/output and yield details;
update register_generation_run_handlers to document the GenerationWiringContext
parameter and that it registers handlers for batched audio inference, and update
generation_wrapper to state its args (proxy passthrough to
res_h.generate_with_batch_management), what it yields (streamed generation
results/frames or iterator of result objects), and any exceptions propagated
from res_h.generate_with_batch_management; refer to
register_generation_run_handlers, GenerationWiringContext, generation_wrapper,
dit_handler, llm_handler, and res_h.generate_with_batch_management when adding
these inputs/outputs/yield descriptions.
In `@acestep/ui/gradio/events/wiring/results_aux_wiring.py`:
- Around line 13-99: The exported registration helper
register_results_aux_handlers and the inner builders make_score_handler and
make_lrc_handler lack docstrings that describe their purpose and the key
inputs/outputs; update register_results_aux_handlers' docstring to briefly state
its role (registers remix/repaint, scoring, LRC and codes handlers) and
enumerate the main parameters and UI components it wires
(GenerationWiringContext, mode_ui_outputs, generation_section outputs like
src_audio/lyrics/captions, results_section buttons and generated_audio_N,
lm_metadata_state, generation_mode, score_display_N, details_accordion_N,
batch_queue), and add concise docstrings to make_score_handler and
make_lrc_handler that say they return a callable bound to an index and list the
callback signature and what res_h.calculate_score_handler_with_selection and
res_h.generate_lrc_handler expect/return (e.g., inputs: scale, batch_idx, queue
for scoring; batch_idx, queue, vocal_lang, infer_steps for LRC, and outputs: UI
elements updated). Ensure wording is brief and follows docstring style
guidelines.
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.
Summary
This PR continues the event-wiring decomposition by extracting generation run and batch navigation wiring from
acestep/ui/gradio/events/__init__.pyinto focused wiring modules.Behavior is intended to be unchanged. This is a structural refactor to reduce facade size/risk and keep wiring contracts explicit.
What Changed
acestep/ui/gradio/events/wiring/generation_run_wiring.pygenerate_btnclick chain:acestep/ui/gradio/events/wiring/generation_batch_navigation_wiring.pyprev_batch_btnandnext_batch_btnchainsacestep/ui/gradio/events/wiring/__init__.pyexports for new wiring registrarsacestep/ui/gradio/events/__init__.pyto delegate to:register_generation_run_handlers(...)register_generation_batch_navigation_handlers(...)acestep/ui/gradio/events/wiring/decomposition_contract_test.pyfor new delegation/contractsacestep/ui/gradio/events/wiring/results_aux_wiring.pyValidation
python -m unittest discover -s acestep/ui/gradio/events/wiring -p "*_test.py"Ran 8 tests ... OKgeneration_run_wiring.py: 169 LOCgeneration_batch_navigation_wiring.py: 180 LOCManual UI Tests Passed
autogenoff)autogenon)Next Batchnavigation updates outputs/state correctlyPrevious Batchnavigation restores prior batch correctlyRestore Paramsrepopulates generation inputs correctlyRisk
Low. This is wiring extraction with no intended behavior changes; ordering contracts were preserved and validated.
Summary by CodeRabbit
Refactor
New Features
Tests