Refactor (gradio part3) mode wiring decomposition#654
Merged
ChuxiJ merged 1 commit intoace-step:mainfrom Feb 21, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request extracts mode-related event wiring logic from the main event setup module into a dedicated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
1larity
force-pushed
the
sync/pr3-independent-20260220
branch
from
954a799 to
0bcdee3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
acestep/ui/gradio/events/__init__.py (1)
195-201: Consider iterable unpacking for output list construction.The list concatenation
[...] + mode_ui_outputsworks but iterable unpacking is more idiomatic.♻️ Optional: Use iterable unpacking
outputs=[ generation_section["src_audio"], generation_section["generation_mode"], generation_section["lyrics"], generation_section["captions"], - ] + mode_ui_outputs, + *mode_ui_outputs, + ],Apply the same pattern to line 217.
🤖 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 195 - 201, Change the outputs list construction to use iterable unpacking instead of list concatenation: build the outputs argument as a single list literal that unpacks mode_ui_outputs (e.g., outputs=[generation_section["src_audio"], generation_section["generation_mode"], generation_section["lyrics"], generation_section["captions"], *mode_ui_outputs]) and apply the same change at the other occurrence referenced around line 217; locate the call that passes outputs= and replace the existing "[...] + mode_ui_outputs" pattern with the unpacking form using generation_section and mode_ui_outputs.acestep/ui/gradio/events/wiring/generation_metadata_wiring.py (1)
85-103: Tuple concatenation on line 86 works but could use unpacking.The
+ (True,)appends theis_format_caption_statevalue. This works correctly sinceload_random_examplereturns an 8-tuple and the outputs list expects 9 values.♻️ Optional: Use tuple unpacking for clarity
generation_section["sample_btn"].click( - fn=lambda task: gen_h.load_random_example(task, llm_handler) + (True,), + fn=lambda task: (*gen_h.load_random_example(task, llm_handler), True), inputs=[generation_section["task_type"]],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/events/wiring/generation_metadata_wiring.py` around lines 85 - 103, The lambda passed to generation_section["sample_btn"].click appends (True,) to the tuple returned by gen_h.load_random_example using + (True,), which is correct but unclear; change the lambda to use tuple unpacking so it returns (*gen_h.load_random_example(task, llm_handler), True) instead (referencing generation_section["sample_btn"].click and gen_h.load_random_example) so the returned values clearly match the nine expected outputs including is_format_caption_state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@acestep/ui/gradio/events/__init__.py`:
- Around line 195-201: Change the outputs list construction to use iterable
unpacking instead of list concatenation: build the outputs argument as a single
list literal that unpacks mode_ui_outputs (e.g.,
outputs=[generation_section["src_audio"], generation_section["generation_mode"],
generation_section["lyrics"], generation_section["captions"], *mode_ui_outputs])
and apply the same change at the other occurrence referenced around line 217;
locate the call that passes outputs= and replace the existing "[...] +
mode_ui_outputs" pattern with the unpacking form using generation_section and
mode_ui_outputs.
In `@acestep/ui/gradio/events/wiring/generation_metadata_wiring.py`:
- Around line 85-103: The lambda passed to
generation_section["sample_btn"].click appends (True,) to the tuple returned by
gen_h.load_random_example using + (True,), which is correct but unclear; change
the lambda to use tuple unpacking so it returns
(*gen_h.load_random_example(task, llm_handler), True) instead (referencing
generation_section["sample_btn"].click and gen_h.load_random_example) so the
returned values clearly match the nine expected outputs including
is_format_caption_state.
Contributor
Author
|
@ChuxiJ Codex got it's knickers in a right twist with the commits on this, I've tried to repair it, but I wouldn't be surprised if there are conflicts for me to resolve once part 2 is applied. |
ChuxiJ
approved these changes
Feb 21, 2026
1larity
force-pushed
the
sync/pr3-independent-20260220
branch
from
0bcdee3 to
ea7b286
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
acestep/ui/gradio/events/wiring/generation_mode_wiring.py (1)
13-19: Consider expanding the docstring to include key inputs/outputs.The docstring is minimal. Per coding guidelines, docstrings should include "purpose plus key inputs/outputs." Consider briefly describing what each parameter represents.
📝 Suggested docstring expansion
def register_generation_mode_handlers( context: GenerationWiringContext, mode_ui_outputs: Sequence[Any], auto_checkbox_inputs: Sequence[Any], auto_checkbox_outputs: Sequence[Any], ) -> None: - """Register generation mode and simple-mode handlers.""" + """Register generation mode and simple-mode event handlers. + + Args: + context: Wiring context with handlers and component maps. + mode_ui_outputs: Output components for mode-change events. + auto_checkbox_inputs: Inputs for auto-checkbox unchecking logic. + auto_checkbox_outputs: Outputs for auto-checkbox unchecking logic. + """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/gradio/events/wiring/generation_mode_wiring.py` around lines 13 - 19, Update the minimal docstring for register_generation_mode_handlers to include its purpose and brief descriptions of each parameter: explain that context (GenerationWiringContext) provides wiring state and callbacks, mode_ui_outputs is the sequence of UI elements updated when generation mode changes, auto_checkbox_inputs are input widgets representing "auto" toggles, and auto_checkbox_outputs are the corresponding outputs or handlers triggered; also mention return value (None) and side effects (registers event handlers). Keep it short and consistent with other docstrings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@acestep/ui/gradio/events/wiring/generation_mode_wiring.py`:
- Around line 13-19: Update the minimal docstring for
register_generation_mode_handlers to include its purpose and brief descriptions
of each parameter: explain that context (GenerationWiringContext) provides
wiring state and callbacks, mode_ui_outputs is the sequence of UI elements
updated when generation mode changes, auto_checkbox_inputs are input widgets
representing "auto" toggles, and auto_checkbox_outputs are the corresponding
outputs or handlers triggered; also mention return value (None) and side effects
(registers event handlers). Keep it short and consistent with other docstrings.
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.
Summary
Scope
Added Acestep/ui/gradio/events/wiring/generation_mode_wiring.py with:
register_generation_mode_handlers(...)
Updated Acestep/ui/gradio/events/init.py to delegate mode/simple-mode registration.
Updated Acestep/ui/gradio/events/wiring/init.py exports.
Updated Acestep/ui/gradio/events/wiring/decomposition_contract_test.py to verify PR3 delegation and mode-output binding at AST level.
No handler behaviour changes.
No interface signature changes for:
No cross-path runtime/hardware changes.
Validation
Code review by GLM5 with no issues.
Unit tests introduced/updated
egister_generation_mode_handlers
Unit tests run
Manual UI tests passed
Conflict checks
Risk
Summary by CodeRabbit
Refactor
Tests