feat: add Streamlit-based ACE Studio UI#693
Conversation
Alternative UI to the existing Gradio interface, built with Streamlit. Key features: - Dashboard with project overview and quick-start actions - Text-to-music generation wizard with full parameter controls - Interactive audio editor with repaint, cover, and complete tasks - Waveform visualization with region selection for editing - Audio file browser (projects + gradio_outputs + upload) - Batch generation support (up to 8 songs) - Settings panel with model init controls (DiT + LLM) - Auto-initialization of models on first launch - Project management with metadata persistence Architecture: - main.py: Entry point with sidebar navigation and auto-init - components/: Modular UI components (editor split into 5 modules) - utils/cache.py: @st.cache_resource handler singletons + init helpers - utils/project_manager.py: Project CRUD with JSON metadata - config.py: Centralized constants and path management All generation calls use the existing acestep.inference.generate_music() API - no changes to the core ACE-Step codebase.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a complete Streamlit-based ACE Studio UI: project/dashboard, generation wizard, editor (waveform, tasks, runner), batch generator, settings, utilities (project manager, audio utils, handler cache), configs, docs, and startup scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Generation UI
participant Cache as Handler Cache
participant Handlers as DiT/LLM<br/>Handlers
participant ACEStep as ACE-Step<br/>Pipeline
participant ProjMgr as Project<br/>Manager
participant Disk as File System
User->>UI: Enter song parameters
UI->>Cache: get_dit_handler()
Cache-->>UI: DiT handler instance
UI->>Cache: is_dit_ready()
Cache->>Handlers: Check model loaded
Handlers-->>Cache: Status
Cache-->>UI: Ready/Not Ready
alt Model not loaded
UI->>Cache: initialize_dit()
Cache->>Handlers: Load weights
Handlers->>Disk: Read model checkpoint
Disk-->>Handlers: Model data
Handlers-->>Cache: Initialization complete
end
UI->>Handlers: generate_music(GenerationParams)
Handlers->>ACEStep: Execute pipeline
ACEStep-->>Handlers: Audio output
Handlers-->>UI: Generated audio
UI->>ProjMgr: create_project(name)
ProjMgr->>Disk: Create project dir + metadata
Disk-->>ProjMgr: Success
UI->>ProjMgr: save_audio(audio_bytes)
ProjMgr->>Disk: Write WAV file
Disk-->>ProjMgr: File path
UI->>User: Display results + playback
sequenceDiagram
participant User
participant Editor as Editor UI
participant AudioPicker as Audio Picker
participant Waveform as Waveform
participant TaskUI as Task Panel
participant Runner as Editor Runner
participant Cache as Handler Cache
participant Handlers as DiT Handler
participant ProjMgr as Project Manager
participant Disk as File System
User->>Editor: Open Editor
Editor->>AudioPicker: show sources
User->>AudioPicker: Select audio
AudioPicker-->>Editor: audio_path
Editor->>Waveform: show_waveform_and_player()
Waveform->>Disk: Read audio
Waveform->>Waveform: Compute duration & waveform
Waveform-->>Editor: duration_sec
User->>Editor: Select task & params
Editor->>TaskUI: Render task UI
User->>TaskUI: Submit run
TaskUI->>Runner: run_edit_task(params)
Runner->>Cache: get_dit_handler()
Runner->>Handlers: generate_music(edit params)
Handlers-->>Runner: Generated audio
Runner->>User: Display results
User->>Runner: Save to project
Runner->>ProjMgr: create_project + save_audio
ProjMgr->>Disk: Persist files
Disk-->>ProjMgr: Success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (23)
ace_studio_streamlit/README.md-169-177 (1)
169-177:⚠️ Potential issue | 🟡 MinorRoadmap contradicts already-implemented features.
Line 169 lists "Waveform visualization with interactive timeline" as a future item, but the PR description states waveform visualization with selectable regions is already shipped. Remove it from the roadmap to avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/README.md` around lines 169 - 177, The roadmap entry "Waveform visualization with interactive timeline" conflicts with the PR which already shipped waveform visualization with selectable regions; remove that checklist line from the README.md roadmap section so the roadmap no longer lists an already-implemented feature and keep the remaining items unchanged.ace_studio_streamlit/.streamlit/config.toml-10-10 (1)
10-10:⚠️ Potential issue | 🟡 Minor
showErrorDetails = truemay leak stack traces to end users.In any shared/public deployment this exposes internal file paths and module details in the browser. Consider setting it to
falsefor production and documenting the override for local development (e.g., via CLI flag or a dev-specific config overlay).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/.streamlit/config.toml` at line 10, The config key showErrorDetails is currently true and may leak stack traces; change showErrorDetails to false in .streamlit/config.toml for production and add a note or mechanism to allow enabling it only for local/dev use (e.g., a dev-specific config overlay or a CLI/dev flag) so developers can override showErrorDetails when needed.ace_studio_streamlit/INSTALL.md-45-48 (1)
45-48:⚠️ Potential issue | 🟡 Minor"4GB VRAM (CPU only)" is a contradiction; also verify the ROCm env var syntax.
- Line 46: CPUs do not have VRAM. The intent is probably "4GB RAM minimum (CPU-only inference)".
- Line 68:
export PYTORCH_HIP_ALLOC_CONF=":256:8"— the value format looks malformed. The typical ROCm/HIP allocation config uses key-value pairs like"max_split_size_mb:512". Confirm the intended value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/INSTALL.md` around lines 45 - 48, Replace the contradictory "4GB VRAM (CPU only)" line with a clear minimum RAM requirement such as "4GB RAM (CPU-only inference)" to indicate CPU-based usage, and correct the ROCm env var usage by fixing the export of PYTORCH_HIP_ALLOC_CONF to the proper key-value syntax used by ROCm/HIP (e.g., using keys like max_split_size_mb with an MB value) so the value is well-formed and matches the intended allocation size.ace_studio_streamlit/INSTALL.md-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorPython version requirement contradicts the coding guidelines.
"Python 3.8+ (tested with 3.11)" conflicts with the project requirement of Python 3.11–3.12. Users who install with 3.8–3.10 will encounter runtime issues with type hints and library constraints used elsewhere in the codebase. Update to "Python 3.11–3.12 required".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/INSTALL.md` at line 9, The README entry currently states "Python 3.8+ (tested with 3.11)" which contradicts the project's required Python range; update the INSTALL.md line that reads "Python 3.8+ (tested with 3.11)" to explicitly state "Python 3.11–3.12 required" so the supported version range matches the coding guidelines and avoids runtime/type hint issues.ace_studio_streamlit/run.sh-11-16 (1)
11-16:⚠️ Potential issue | 🟡 Minor
pythonmay resolve to Python 2 on many Linux systems; usepython3.Lines 11 and 16 both call
python, which is not guaranteed to point to Python 3 on all distributions (e.g., Debian/Ubuntu ship Python 2 as thepythonbinary). Replace withpython3.🐛 Proposed fix
-python --version +python3 --version ... - python -m venv ../.venv + python3 -m venv ../.venv🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/run.sh` around lines 11 - 16, The script uses the python binary which can be Python 2 on some systems; update the two invocations ("python --version" and "python -m venv ../.venv" inside the if [ ! -d "../.venv" ] block) to call python3 instead (i.e., use "python3 --version" and "python3 -m venv ../.venv") so the virtual environment and version check run with Python 3.ace_studio_streamlit/.gitignore-1-5 (1)
1-5:⚠️ Potential issue | 🟡 MinorAdd
.streamlit/secrets.tomland.envto.gitignore.The current
.gitignoredoesn't cover.streamlit/secrets.tomlor.env.INSTALL.mddocuments an optional.envfile with secrets (DEVICE, model paths, etc.), andsecrets.tomlis Streamlit's designated secrets store. If either file ever contains real credentials, they'll be silently committed.🛡️ Proposed additions
__pycache__/ *.pyc .cache/ projects/ streamlit.log +.env +.streamlit/secrets.toml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/.gitignore` around lines 1 - 5, Update the .gitignore to prevent accidental commits of secrets by adding entries for Streamlit secrets and environment files: add ".streamlit/secrets.toml" and ".env" to the existing ignore list (the current file contains __pycache__/, *.pyc, .cache/, projects/, streamlit.log) so that secrets.toml and any .env files are not tracked.ace_studio_streamlit/run.sh-1-36 (1)
1-36:⚠️ Potential issue | 🟡 MinorScript has no working-directory guard and uses a parent-directory venv that can shadow the ACE-Step venv.
Two related issues:
- No
cdguard — if the script is invoked from any directory other thanace_studio_streamlit/,requirements.txtandmain.py(Line 25 and 36) won't be found. Addcd "$(dirname "$0")"at the top.- Venv at
../.venv— creating/activating the virtual environment one level up risks overwriting or interfering with the parent ACE-Step project's own venv. Use a local.venvinsideace_studio_streamlit/instead.♻️ Proposed fix
#!/bin/bash # Quick start script for ACE Studio Streamlit UI -set -e +set -euo pipefail +cd "$(dirname "$0")" ... # Check if venv exists -if [ ! -d "../.venv" ]; then +if [ ! -d ".venv" ]; then echo "Creating virtual environment..." - python -m venv ../.venv + python3 -m venv .venv fi # Activate venv echo "Activating virtual environment..." -source ../.venv/bin/activate +source .venv/bin/activate🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/run.sh` around lines 1 - 36, Add a working-directory guard at the top of run.sh by changing to the script's directory (use cd "$(dirname "$0")") so requirements.txt and main.py are resolved relative to the script, and stop creating/activating a parent-level virtualenv: replace occurrences of "../.venv" with a local ".venv" and update the activation command (source .venv/bin/activate) so the script creates/uses a repository-local virtual environment instead of shadowing the parent project's venv.ace_studio_streamlit/utils/audio_utils.py-41-51 (1)
41-51:⚠️ Potential issue | 🟡 MinorUse
path=parameter instead of deprecatedfilename=in librosa.get_duration().The
filenameparameter was deprecated in librosa 0.10 and renamed topath; it will be removed in 1.0. Update the call to avoid FutureWarning and ensure compatibility with future librosa versions. Note: other parts of the codebase already use the correctpath=parameter (e.g.,editor_waveform.py).♻️ Proposed fix
- duration = librosa.get_duration(filename=file_path) + duration = librosa.get_duration(path=file_path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/utils/audio_utils.py` around lines 41 - 51, The get_audio_duration function calls librosa.get_duration using the deprecated filename= parameter; update the call in get_audio_duration to use path=file_path (i.e., librosa.get_duration(path=file_path)) to match current librosa API and avoid FutureWarnings, keeping the existing try/except and logger.error behavior unchanged.ace_studio_streamlit/QUICKSTART.md-37-42 (1)
37-42:⚠️ Potential issue | 🟡 MinorReplace absolute local paths with repo‑relative instructions.
The/Users/...path is machine‑specific; usecd ace_studio_streamlitor<repo>/ace_studio_streamlitinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/QUICKSTART.md` around lines 37 - 42, Replace the machine-specific absolute path in the QUICKSTART.md code block with a repo-relative path; change the first line from "cd /Users/..." to "cd ace_studio_streamlit" (or "<repo>/ace_studio_streamlit" as an alternative) and keep the subsequent run instructions referencing ./run.sh and run.bat so readers can run run.sh (macOS/Linux) or run.bat (Windows) from the repository directory.ace_studio_streamlit/PROJECT_SUMMARY.md-5-8 (1)
5-8:⚠️ Potential issue | 🟡 MinorReplace developer-specific absolute paths with repo-relative placeholders.
Paths like/Users/.../ace_studio_streamlitwon’t work for other users. Use a placeholder such as<repo>/ace_studio_streamlitorcd ace_studio_streamlitinstead.Also applies to: 77-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/PROJECT_SUMMARY.md` around lines 5 - 8, Replace any developer-specific absolute filesystem paths in PROJECT_SUMMARY.md (e.g., the string "/Users/p25301/Projects/ACE-Step-1.5/ace_studio_streamlit/") with repo-relative placeholders such as "<repo>/ace_studio_streamlit" or a simple instruction like "cd ace_studio_streamlit"; update the same pattern in the other occurrences noted (lines ~77-88) so all hard-coded absolute paths are removed and the README uses repo-relative or neutral instructions instead.ace_studio_streamlit/components/dashboard.py-22-32 (1)
22-32:⚠️ Potential issue | 🟡 MinorQuick actions set
editor_mode, but editor selectsedit_task.
This prevents the “Create Cover” / “Edit Song” buttons from pre-selecting the intended task. Align the key to whatshow_editorreads.Suggested change
with col2: if st.button("🎤 Create Cover", key="quick_cover", use_container_width=True): st.session_state.tab = "editor" st.session_state.editor_mode = "cover" + st.session_state.edit_task = "cover" st.rerun() @@ with col3: if st.button("🎨 Edit Song", key="quick_edit", use_container_width=True): st.session_state.tab = "editor" st.session_state.editor_mode = "repaint" + st.session_state.edit_task = "repaint" st.rerun()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/dashboard.py` around lines 22 - 32, The quick-action buttons set st.session_state.editor_mode but the editor reads st.session_state.edit_task (so the actions don't pre-select the task); update the handlers in col2 and col3 (the st.button calls with keys "quick_cover" and "quick_edit") to set st.session_state.edit_task = "cover" and st.session_state.edit_task = "repaint" respectively (you may keep editor_mode for compatibility if desired), then keep setting st.session_state.tab = "editor" and st.rerun() so show_editor picks the correct task.ace_studio_streamlit/components/editor.py-28-37 (1)
28-37:⚠️ Potential issue | 🟡 MinorConsider stopping the editor flow early when DiT isn't ready for better UX.
Downstream guards exist in
run_edit_task()(editor_runner.py:47–49) that prevent actual task execution if DiT is not loaded. However, the current flow still renders the full editor UI (audio picker, waveform, task selector) before the error is shown when users click a button. Addingst.stop()after the warning would provide clearer, earlier feedback instead of waiting for user interaction.Suggested change
if not is_dit_ready(): st.warning( "DiT model is **not loaded**. " "Load it in **⚙️ Settings → Models** first." ) + st.stop()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/editor.py` around lines 28 - 37, The warning for DiT not being loaded currently displays but the editor UI (e.g., pick_audio_source(), waveform, task selector) still renders; update the early-exit logic in the editor flow by calling st.stop() immediately after the is_dit_ready() warning so the page halts rendering further components (instead of relying on downstream guards in run_edit_task()) and prevents the rest of the editor UI from appearing when DiT is unavailable.ace_studio_streamlit/config.py-46-50 (1)
46-50:⚠️ Potential issue | 🟡 Minor
DEFAULT_GUIDANCEandDEFAULT_STEPSare defined here but not used bygeneration_wizard.py— the wizard hardcodes different values.
Constant Config value Wizard hardcode DEFAULT_STEPS32 8 (line 177) DEFAULT_GUIDANCE7.5 7.0 (line 187) This will confuse maintainers. Either import and use these constants in the wizard (and batch generator), or remove the unused constants to avoid drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/config.py` around lines 46 - 50, generation_wizard.py currently hardcodes step and guidance defaults that drift from the constants DEFAULT_STEPS and DEFAULT_GUIDANCE in config.py; update the wizard (and the batch generator) to import DEFAULT_STEPS and DEFAULT_GUIDANCE from ace_studio_streamlit.config and replace the hardcoded values (8 and 7.0) where defaults are applied (e.g., in the functions or variables around the wizard defaults), or if those constants are truly unused project-wide, remove DEFAULT_STEPS and DEFAULT_GUIDANCE from config.py to avoid confusion — choose one approach and apply it consistently across generation_wizard.py and the batch generator so defaults are centralized.ace_studio_streamlit/components/generation_wizard.py-32-41 (1)
32-41:⚠️ Potential issue | 🟡 MinorRedundant
import sys as _sys—sysis already imported at the module level (line 7).Proposed fix
def _quick_init_dit() -> None: """One-click DiT init from the Generate page.""" - import sys as _sys - with st.spinner("Loading DiT model..."): _status, _ok = initialize_dit( config_path="acestep-v15-turbo", device="auto", - offload_to_cpu=(_sys.platform != "darwin"), + offload_to_cpu=(sys.platform != "darwin"), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/generation_wizard.py` around lines 32 - 41, The local redundant import "import sys as _sys" inside _quick_init_dit should be removed and the module-level sys should be used instead; edit the _quick_init_dit function to delete the inner import line and leave the call that uses _sys.platform unchanged (or rename usages to sys.platform if you prefer), ensuring initialize_dit is called with offload_to_cpu=(sys.platform != "darwin") and no shadowing import remains.ace_studio_streamlit/components/batch_generator.py-150-155 (1)
150-155:⚠️ Potential issue | 🟡 MinorProgress bar shows completion before each song is processed.
progress = (idx + 1) / total_songsis calculated at the top of the loop, so the bar reaches 100% before the last song's generation attempt.Proposed fix
for idx, song in enumerate(queue): - # Update progress - progress = (idx + 1) / total_songs - progress_placeholder.progress(progress) + progress_placeholder.progress(idx / total_songs) status_placeholder.text(f"Generating song {idx + 1}/{total_songs}: {song['caption'][:40]}...") try: ... + finally: + progress_placeholder.progress((idx + 1) / total_songs)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/batch_generator.py` around lines 150 - 155, The progress is updated before processing each song in the try loop, so progress_placeholder.progress((idx + 1) / total_songs) and status_placeholder.text(...) should be moved to reflect completed work; either compute progress = idx / total_songs at the top of the loop (so it shows work completed so far) or, preferably, move the progress_placeholder.progress and status_placeholder.text calls to after the song has been processed so they use (idx + 1) / total_songs; update the loop in the try block that iterates over queue to call progress_placeholder.progress and status_placeholder.text only after the song generation step completes (referencing queue, total_songs, progress_placeholder.progress, and status_placeholder.text).ace_studio_streamlit/components/audio_player.py-28-28 (1)
28-28:⚠️ Potential issue | 🟡 MinorHardcoded
format="audio/wav"won't match non-WAV files.The widget accepts any audio path, but the MIME type is always set to
audio/wav. If the file is.mp3or.flac, this is incorrect. The same issue exists on line 72.Proposed fix
+_MIME_MAP = { + ".wav": "audio/wav", + ".mp3": "audio/mpeg", + ".flac": "audio/flac", + ".m4a": "audio/mp4", +} + def audio_player_widget(audio_path: str, label: str = "Audio", show_download: bool = True): ... - st.audio(audio_bytes, format="audio/wav") + mime = _MIME_MAP.get(audio_file.suffix.lower(), "audio/wav") + st.audio(audio_bytes, format=mime)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/audio_player.py` at line 28, The hardcoded format="audio/wav" should be replaced with a computed MIME type so non-WAV files render correctly: import Python's mimetypes and call mimetypes.guess_type(...) using the audio source filename (or a provided filename parameter) to get mime_type (fallback to "audio/wav" or None if unknown), then pass that mime_type to st.audio(audio_bytes, format=mime_type); update both occurrences of st.audio(...) (the calls that currently pass format="audio/wav") in audio_player.py accordingly.ace_studio_streamlit/components/settings_panel.py-260-297 (1)
260-297:⚠️ Potential issue | 🟡 MinorSilently swallowing exceptions — log before discarding.
Both
_list_dit_modelsand_list_lm_modelscatchExceptionandpass, making failures invisible.loguruis already imported — use it.Proposed fix
try: models = handler.get_available_acestep_v15_models() if models: return models - except Exception: - pass + except Exception as exc: + logger.warning(f"Failed to query DiT models from handler: {exc}")Apply the same pattern to
_list_lm_models.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/settings_panel.py` around lines 260 - 297, Both _list_dit_models and _list_lm_models currently swallow exceptions silently; modify the except blocks in get_dit_handler() usage (inside _list_dit_models) and get_llm_handler() usage (inside _list_lm_models) to log the caught exception via loguru (e.g., logger.exception or logger.error with exception info) with a clear contextual message like "error getting available models from handler" before continuing to the fallback scan, so failures are visible while preserving the fallback behavior.ace_studio_streamlit/components/editor_waveform.py-89-101 (1)
89-101:⚠️ Potential issue | 🟡 MinorSilent 120 s fallback when duration detection fails — may cause incorrect region selection.
If both
soundfileandlibrosaare unavailable (or error), the user sees a 120 s timeline with no indication it's a guess. This directly affects repaint/complete region boundaries. Consider displaying a warning to the user, and logging the failures.Proposed fix
def _get_duration(audio_path: Path) -> float: """Return audio duration in seconds.""" if sf is not None: try: return sf.info(str(audio_path)).duration - except Exception: - pass + except Exception as exc: + logger.debug(f"soundfile duration failed: {exc}") if librosa is not None: try: return librosa.get_duration(path=str(audio_path)) - except Exception: - pass - return 120.0 # fallback + except Exception as exc: + logger.debug(f"librosa duration failed: {exc}") + logger.warning(f"Could not determine duration for {audio_path}; defaulting to 120s") + return 120.0You'll also need to add
from loguru import loggerat the top of the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/editor_waveform.py` around lines 89 - 101, The helper _get_duration currently returns a silent 120.0 fallback when sf and librosa fail; modify _get_duration to catch and log the exceptions from sf.info and librosa.get_duration using logger from loguru (add "from loguru import logger" at top), and when both methods fail call logger.warning with the collected error details and also surface a user-visible warning (e.g., via streamlit's st.warning) explaining that the duration was estimated and defaulting to 120s to avoid misleading the waveform/regions; keep the return value of 120.0 as the final fallback.ace_studio_streamlit/components/generation_wizard.py-338-344 (1)
338-344:⚠️ Potential issue | 🟡 MinorFragile project name sanitization — only replaces spaces and slashes.
Characters like
\,:,*,?,",<,>,|(and null bytes) are invalid on Windows. Consider a more robust sanitization or use a slug utility.Proposed fix
+import re + safe_name = ( - caption[:30] - .replace(" ", "_") - .replace("/", "_") + re.sub(r'[^\w\-]', '_', caption[:30]).strip('_') )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/generation_wizard.py` around lines 338 - 344, The current safe_name generation only replaces spaces and "/" and is fragile on Windows; update the sanitization where safe_name is created (in the block that instantiates ProjectManager and computes safe_name from caption) to perform robust slugification: remove or replace all disallowed filesystem characters (e.g., \ : * ? " < > | and null bytes) by using a regex that keeps only a safe whitelist (letters, numbers, hyphen, underscore) or replaces any other character with "_" , trim leading/trailing dots/spaces, enforce a max length (e.g., 30) and ensure the result is not empty (fallback to a timestamp or "project_<id>"). Also preserve reference to ProjectManager usage so created project directory uses this sanitized name.ace_studio_streamlit/utils/project_manager.py-36-38 (1)
36-38:⚠️ Potential issue | 🟡 Minor
mkdirmissingparents=True— will fail if parent directories don't exist.If
PROJECTS_DIRrefers to a nested path where intermediate directories are absent, the constructor will raiseFileNotFoundError.Suggested fix
def __init__(self, projects_dir: Path = PROJECTS_DIR): self.projects_dir = projects_dir - self.projects_dir.mkdir(exist_ok=True) + self.projects_dir.mkdir(parents=True, exist_ok=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/utils/project_manager.py` around lines 36 - 38, The constructor (__init__) sets self.projects_dir to PROJECTS_DIR but calls self.projects_dir.mkdir(exist_ok=True) which will raise FileNotFoundError for nested paths; change the mkdir call in the ProjectManager.__init__ to self.projects_dir.mkdir(parents=True, exist_ok=True) so intermediate directories are created when PROJECTS_DIR is a nested path.ace_studio_streamlit/components/editor_runner.py-143-143 (1)
143-143:⚠️ Potential issue | 🟡 MinorFilename sanitization is too narrow — path-unsafe characters can slip through.
Only spaces and
/are replaced, but characters like\,?,*,:,<,>,|, and"are also invalid on common filesystems (especially Windows). This can causeshutil.copy2or project directory creation to fail.Suggested fix
- safe = caption[:25].replace(" ", "_").replace("/", "_") + import re + safe = re.sub(r"[^\w\-.]", "_", caption[:25])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/editor_runner.py` at line 143, The current filename sanitization only replaces spaces and "/" (safe = caption[:25].replace(" ", "_").replace("/", "_")), letting characters like \ ? * : < > | " through; update the sanitization to use a whitelist regex on the caption (e.g., allow only alphanumerics, hyphen, underscore, dot) and replace all other characters with "_" before truncating, also trim leading/trailing dots/spaces and ensure the result is non-empty; change the logic around the safe variable (and any downstream use in shutil.copy2 or project directory creation) to use this cleaned value so filesystem-unsafe characters are removed consistently.ace_studio_streamlit/utils/project_manager.py-129-139 (1)
129-139:⚠️ Potential issue | 🟡 Minor
_load_metadatawill crash on unknown keys in JSON.
ProjectMetadata(**data)raisesTypeErrorif the JSON file contains keys not present in the dataclass (e.g., added by a future version). Theexcept Exceptionon line 137 catches this but silently discards the entire project metadata, making the project invisible inlist_projects.Filter to known fields before construction:
Suggested fix
try: with open(metadata_path, "r") as f: data = json.load(f) - return ProjectMetadata(**data) + known_fields = {f.name for f in ProjectMetadata.__dataclass_fields__.values()} + filtered = {k: v for k, v in data.items() if k in known_fields} + return ProjectMetadata(**filtered) except Exception as e: logger.error(f"Failed to load metadata from {metadata_path}: {e}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/utils/project_manager.py` around lines 129 - 139, The current _load_metadata method can crash (or silently drop metadata) when ProjectMetadata(**data) receives unknown keys from metadata.json; fix it by filtering the loaded JSON to only the dataclass fields before constructing ProjectMetadata: read data, compute allowed_keys = set(ProjectMetadata.__dataclass_fields__.keys()), build filtered = {k: v for k, v in data.items() if k in allowed_keys}, optionally log a warning about any dropped keys, then return ProjectMetadata(**filtered); preserve the existing exception logging for genuine IO/parse errors so list_projects still sees valid projects.ace_studio_streamlit/utils/cache.py-145-147 (1)
145-147:⚠️ Potential issue | 🟡 Minor
clear_handlers()clears all@st.cache_resourceentries, not just handler caches.
st.cache_resource.clear()is a global wipe — it invalidates every@st.cache_resource-decorated function's cache across the entire app. While the codebase currently has only these three handler functions as cache_resource entries, this approach becomes a maintainability risk if future code adds other cached resources.Call
.clear()directly on each handler function instead:Suggested fix
def clear_handlers() -> None: """Clear all cached handlers (forces re-creation).""" - st.cache_resource.clear() + get_dit_handler.clear() + get_llm_handler.clear() + get_dataset_handler.clear()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/utils/cache.py` around lines 145 - 147, clear_handlers currently calls st.cache_resource.clear(), which invalidates every `@st.cache_resource` entry globally; instead locate each handler function in this module that is decorated with `@st.cache_resource` (they are the cached handler factory functions referenced by clear_handlers) and replace the global call with explicit per-function clears (e.g., MyHandlerFunction.clear(), OtherHandlerFunction.clear(), ...) so only those handler caches are invalidated; update clear_handlers to call .clear() on each identified handler function and remove the st.cache_resource.clear() call.
🧹 Nitpick comments (13)
ace_studio_streamlit/components/dashboard.py (1)
10-12: Add a return type hint and complete docstring forshow_dashboard.
The function should declare-> Noneand document outputs explicitly.Suggested change
-def show_dashboard(): - """Display dashboard with recent projects and quick start options""" +def show_dashboard() -> None: + """Display dashboard with recent projects and quick start options. + + Returns: + None + """As per coding guidelines, “Docstrings are mandatory for all new or modified Python modules, classes, and functions”, “Docstrings must be concise and include purpose plus key inputs/outputs (and raised exceptions when relevant)”, and “Add type hints for new/modified functions when practical in Python”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/dashboard.py` around lines 10 - 12, Add an explicit return type hint and expand the docstring for show_dashboard: change the signature to declare -> None and update the triple-quoted docstring to succinctly state the function’s purpose, side effects (writes UI to Streamlit via st.markdown and other st.* calls), any outputs (no return value), and note that it raises no exceptions under normal operation; keep the docstring concise and place it immediately under the def show_dashboard(...) line.ace_studio_streamlit/main.py (2)
69-85: Centralize platform/hardware decisions viagpu_config.py.
Backend/offload selection is based onsys.platformhere; please route this throughgpu_config.pyso hardware logic stays consistent across the codebase.
As per coding guidelines, “Usegpu_config.pyfor hardware detection and configuration; DO NOT alter non-target platform paths unless explicitly required”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/main.py` around lines 69 - 85, The platform-based hardware decisions in the DiT init and LLM load (offload_to_cpu=(sys.platform != "darwin") and _backend = "mlx" if sys.platform == "darwin" else "vllm") must be moved to gpu_config.py; replace the inline checks by calling gpu_config.py helpers (e.g., use gpu_config.get_offload_to_cpu() for the DiT offload_to_cpu argument and gpu_config.select_llm_backend() or similar for the _backend value) so all hardware/platform logic is centralized and reused when calling initialize_llm and the DiT loader.
1-216: Splitmain.pyto stay under the 200‑LOC cap.
At ~216 LOC, consider extracting the sidebar, auto-init, or footer into helpers/modules to meet the size guideline.
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). Do not raise for files in the 150–200 LOC range, and accept files below 200 LOC without justification to split.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/main.py` around lines 1 - 216, main.py exceeds the 200-LOC guideline; refactor to split responsibilities by extracting the sidebar rendering, auto-initialisation block, and footer into helper functions/modules. Create functions like render_sidebar() (move the entire with st.sidebar: block and references to ProjectManager/is_llm_ready), auto_init_models() (move the _models_auto_init_done guard and calls to is_dit_ready/initialize_dit/initialize_llm), and render_footer() (move the final divider/markdown), then replace those blocks in main.py with calls to these functions and import them (or a single ui_helpers module). Ensure session state keys (st.session_state.tab, editor_mode, selected_project, _models_auto_init_done) and function names show_dashboard, show_generation_wizard, show_editor, show_batch_generator, show_settings_panel remain referenced unchanged so routing still works.ace_studio_streamlit/run.bat (1)
14-36: Make the script location-agnostic by switching to its directory.
If the script is launched from a different CWD,requirements.txtandmain.pywon’t resolve.Suggested change
`@echo` off REM Quick start script for ACE Studio Streamlit UI (Windows) setlocal enabledelayedexpansion +pushd "%~dp0" @@ streamlit run main.py +popd endlocal🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/run.bat` around lines 14 - 36, The script fails when run from another CWD; make it location-agnostic by changing to the script's directory at start and restoring it at the end: add a pushd "%~dp0" (or pushd "%~dp0.." if you need the parent dir for ..\.venv paths) before any venv/installation/streamlit commands so that call ..\.venv\Scripts\activate.bat, pip install -q -r requirements.txt and streamlit run main.py resolve reliably, and add a matching popd at the end to return to the original directory.ace_studio_streamlit/config.py (1)
16-23: Side effects at import time:sys.pathmutation andmkdircalls.Both
sys.path.insertandmkdirexecute when any module importsconfig. This is a common pattern for monorepo config modules and acceptable here, but worth documenting in the module docstring so future maintainers understand the import-time behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/config.py` around lines 16 - 23, The module performs import-time side effects (mutating sys.path via sys.path.insert using ACESTEP_ROOT and creating directories via PROJECTS_DIR.mkdir, CACHE_DIR.mkdir, OUTPUT_DIR.mkdir); add a clear module-level docstring at the top of ace_studio_streamlit/config.py that documents these behaviors, naming the specific side effects (sys.path modification and the three mkdir calls) and why they occur so future maintainers understand the import-time effects and can avoid surprising behavior.ace_studio_streamlit/components/settings_panel.py (1)
133-137: LLMdevicefalls back to DiT widget value — confusing cross-section coupling.Line 136 reads
dit_device(defined in the DiT section) as the device for the LLM. Users may expect an independent device selector for the LLM. Consider adding a dedicated LLM device selectbox, or at least rename/document this coupling to make the intent clear.- device=dit_device if "dit_device_select" in st.session_state else "auto", + device="auto", # TODO: add a dedicated LLM device selector🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/settings_panel.py` around lines 133 - 137, The LLM device is incorrectly coupled to the DiT widget via dit_device and dit_device_select when calling initialize_llm; add an explicit, independent LLM device selector (e.g., llm_device_select) in the settings UI and propagate its value to the initialize_llm call (use llm_device value instead of dit_device), or if you want to keep coupling, rename variables and update labels to make the dependency explicit; update any references to dit_device/dit_device_select around the initialize_llm invocation so initialize_llm receives the new llm_device variable and the UI shows a clear label like "LLM device" to avoid confusion.ace_studio_streamlit/components/generation_wizard.py (1)
267-277: Progress bar and status text created insidest.spinnercontext may cause layout issues.Streamlit's
st.spinnerrenders an overlay. Creatingst.progressandst.emptyinside it can lead to UI flicker or elements hidden behind the spinner overlay. Consider placing the progress placeholders before the spinner, or dropping the spinner in favor of the progress bar itself.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/generation_wizard.py` around lines 267 - 277, The progress placeholders (progress_bar, status_text) are being created inside the st.spinner context which can cause layout flicker or hiding; move the creation of progress_bar = st.progress(0) and status_text = st.empty() (and any initial status_text.text("⏳ Preparing generation...") / progress_bar.progress(5)) to before the with st.spinner(...) block (or remove the spinner and rely on the progress UI), then call get_dit_handler() and get_llm_handler() inside the spinner and update progress_bar/status_text from there so the placeholders persist outside the spinner overlay.ace_studio_streamlit/components/editor_tasks.py (1)
68-68:duration_secis unused incover_ui.The other two task UIs use it for
region_selector, but cover doesn't need a region. If this is intentional for interface consistency, prefix with underscore to signal intent:_duration_sec. Otherwise, consider removing it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/editor_tasks.py` at line 68, The parameter duration_sec in the function cover_ui is unused; either remove it or rename it to _duration_sec to indicate it's intentionally unused. Update the cover_ui signature and any callers if you remove the parameter, or simply rename the parameter to _duration_sec in cover_ui so linters and readers know it's intentionally unused; reference function cover_ui to locate the change and adjust calls that pass duration_sec only if you remove the parameter entirely.ace_studio_streamlit/components/audio_player.py (1)
9-9:labelparameter is accepted but never used in either function.Either wire it into the UI (e.g. as a header/caption) or remove it to avoid a misleading API surface.
Also applies to: 62-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/audio_player.py` at line 9, The label parameter on audio_player_widget is accepted but never used; either remove the unused label parameter from audio_player_widget (and the other audio widget function that also accepts label) to keep the API clean, or wire it into the UI by rendering it (e.g., call st.subheader(label) or st.caption(label) or include it as the aria-label/alt text before calling st.audio) and update the function docstrings and any callers accordingly so the label is actually displayed and not just a dead parameter.ace_studio_streamlit/utils/project_manager.py (1)
46-51: Two separatedatetime.now()calls produce slightly different timestamps.Nit: capture the timestamp once for consistency.
Suggested fix
+ now = datetime.now().isoformat() metadata = ProjectMetadata( name=name, - created_at=datetime.now().isoformat(), - modified_at=datetime.now().isoformat(), + created_at=now, + modified_at=now, description=description, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/utils/project_manager.py` around lines 46 - 51, The created_at and modified_at fields use two calls to datetime.now() producing inconsistent timestamps; capture a single timestamp once (e.g., ts = datetime.now().isoformat()) before creating ProjectMetadata and use that ts for both created_at and modified_at when constructing the metadata object (reference: ProjectMetadata construction assigning created_at and modified_at to ensure they are identical).ace_studio_streamlit/components/editor_runner.py (2)
82-93: Add return type annotation to_generate.The docstring mentions
GenerationResultbut the function signature lacks a return type hint. As per coding guidelines, type hints should be added for new functions when practical.Suggested fix
Since the return type comes from a deferred import, you can use a string annotation or
TYPE_CHECKING:+from __future__ import annotations +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from acestep.inference import GenerationResult + def _generate( task_type: str, src_audio: str, caption: str, lyrics: str, repainting_start: float, repainting_end: float, audio_cover_strength: float, cover_noise_strength: float, inference_steps: int, seed: int, -): +) -> GenerationResult:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/editor_runner.py` around lines 82 - 93, Add a return type annotation to the _generate function to indicate it returns a GenerationResult; because GenerationResult is imported deferred, annotate the return type as a string ("GenerationResult") or import it under TYPE_CHECKING. Update the signature of _generate to include -> "GenerationResult" (or use TYPE_CHECKING to import the symbol) and ensure any imports or TYPE_CHECKING guard in the module reference GenerationResult so static checkers recognize the type.
129-150:_show_resultsmixes UI rendering with project persistence — consider splitting.The save-to-project block (lines 138–149) handles project creation, name sanitization, and file I/O inside a rendering loop. If a second output is generated with the same caption and task type,
create_projectsilently reuses the same directory (exist_ok=True), andshutil.copy2overwrites any file with the same name without warning the user.This is a minor UX risk — flagging for awareness rather than an immediate fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/components/editor_runner.py` around lines 129 - 150, _show_results mixes rendering and persistence: extract the "Save to project" behavior into a separate function (e.g., save_audio_to_project) that accepts ProjectManager, caption, out_path, task_type, and idx; sanitize caption as you already do, but ensure project names are made unique (avoid silently reusing directories when calling ProjectManager.create_project) by checking for existing project and appending a counter or timestamp to the name, and before calling shutil.copy2 check if dst exists and if so generate a unique filename (append _1, _2, etc.) to avoid overwriting; update the st.button handler in _show_results to call the new function so UI rendering stays separate from ProjectManager/create_project and shutil.copy2 file I/O.ace_studio_streamlit/utils/cache.py (1)
25-58: Add return type annotations to the cached handler factories.Per coding guidelines, type hints should be added for new functions when practical. These return
Optional[AceStepHandler],Optional[LLMHandler], andOptional[DatasetHandler]respectively. Since the types come from deferred imports, you can useTYPE_CHECKING:from typing import TYPE_CHECKING if TYPE_CHECKING: from acestep.handler import AceStepHandler from acestep.llm_inference import LLMHandler🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ace_studio_streamlit/utils/cache.py` around lines 25 - 58, Add explicit return type annotations to the three cached factory functions: get_dit_handler, get_llm_handler, and get_dataset_handler should return Optional[AceStepHandler], Optional[LLMHandler], and Optional[DatasetHandler] respectively; to avoid runtime imports, add "from typing import Optional, TYPE_CHECKING" and inside a TYPE_CHECKING block import AceStepHandler, LLMHandler, and DatasetHandler from their modules, then annotate the functions' signatures accordingly while keeping the deferred imports inside the try blocks unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
ace_studio_streamlit/.gitignoreace_studio_streamlit/.streamlit/config.tomlace_studio_streamlit/.streamlit/secrets.tomlace_studio_streamlit/INSTALL.mdace_studio_streamlit/PROJECT_SUMMARY.mdace_studio_streamlit/QUICKSTART.mdace_studio_streamlit/README.mdace_studio_streamlit/components/__init__.pyace_studio_streamlit/components/audio_player.pyace_studio_streamlit/components/batch_generator.pyace_studio_streamlit/components/dashboard.pyace_studio_streamlit/components/editor.pyace_studio_streamlit/components/editor_audio_picker.pyace_studio_streamlit/components/editor_runner.pyace_studio_streamlit/components/editor_tasks.pyace_studio_streamlit/components/editor_waveform.pyace_studio_streamlit/components/generation_wizard.pyace_studio_streamlit/components/settings_panel.pyace_studio_streamlit/config.pyace_studio_streamlit/main.pyace_studio_streamlit/requirements.txtace_studio_streamlit/run.batace_studio_streamlit/run.shace_studio_streamlit/utils/__init__.pyace_studio_streamlit/utils/audio_utils.pyace_studio_streamlit/utils/cache.pyace_studio_streamlit/utils/project_manager.py
| try: | ||
| import librosa | ||
| duration = librosa.get_duration(filename=audio_path) | ||
| minutes = int(duration // 60) | ||
| seconds = int(duration % 60) | ||
| st.metric("Duration", f"{minutes}m {seconds}s") | ||
| except: | ||
| st.metric("Duration", "Unknown") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "(audio_player|editor_waveform)" | head -20Repository: ace-step/ACE-Step-1.5
Length of output: 163
🏁 Script executed:
cat -n ace_studio_streamlit/components/audio_player.pyRepository: ace-step/ACE-Step-1.5
Length of output: 2510
🏁 Script executed:
if [ -f ace_studio_streamlit/components/editor_waveform.py ]; then
sed -n '95,100p' ace_studio_streamlit/components/editor_waveform.py
fiRepository: ace-step/ACE-Step-1.5
Length of output: 227
🌐 Web query:
librosa 0.10 get_duration parameter renamed filename to path API change
💡 Result:
In librosa 0.10.0, librosa.get_duration renamed the keyword argument filename → path as an API change. The old filename= keyword is kept only as a deprecated alias (equivalent to path) and is scheduled to be removed in librosa 1.0. [1][2][3]
Update your code:
dur = librosa.get_duration(path="audio.wav") # new (0.10+)
# dur = librosa.get_duration(filename="audio.wav") # deprecatedSources: librosa changelog entry for the rename [1], current docs showing path and filename=<DEPRECATED parameter> [2], and the implementation using rename_kw(... old_name="filename", new_name="path", version_deprecated="0.10.0", version_removed="1.0") [3].
Update librosa.get_duration() call to use the correct path parameter and replace bare except with explicit exception handling.
The filename kwarg was renamed to path in librosa 0.10.0 and is deprecated; it will be removed in librosa 1.0. The code currently works in 0.10.2 but uses a deprecated API. Also, replace the bare except with explicit exception handling per coding guidelines. Note that editor_waveform.py (line 98) already uses the correct path= kwarg.
Proposed fix
try:
import librosa
- duration = librosa.get_duration(filename=audio_path)
+ duration = librosa.get_duration(path=audio_path)
minutes = int(duration // 60)
seconds = int(duration % 60)
st.metric("Duration", f"{minutes}m {seconds}s")
- except:
+ except Exception:
st.metric("Duration", "Unknown")📝 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.
| try: | |
| import librosa | |
| duration = librosa.get_duration(filename=audio_path) | |
| minutes = int(duration // 60) | |
| seconds = int(duration % 60) | |
| st.metric("Duration", f"{minutes}m {seconds}s") | |
| except: | |
| st.metric("Duration", "Unknown") | |
| try: | |
| import librosa | |
| duration = librosa.get_duration(path=audio_path) | |
| minutes = int(duration // 60) | |
| seconds = int(duration % 60) | |
| st.metric("Duration", f"{minutes}m {seconds}s") | |
| except Exception: | |
| st.metric("Duration", "Unknown") |
🧰 Tools
🪛 Ruff (0.15.2)
[error] 45-45: Do not use bare except
(E722)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/components/audio_player.py` around lines 39 - 46, Change
the deprecated librosa.get_duration call and the bare except in audio_player.py:
replace get_duration(filename=audio_path) with get_duration(path=audio_path)
(consistent with editor_waveform.py) and avoid a bare except by catching
specific exceptions (e.g., librosa.util.exceptions.ParameterError or a general
Exception fallback) so you handle expected librosa/file errors and still set
st.metric("Duration", "Unknown") on failure; ensure any caught exception is not
silently swallowed (log or include minimal context if available).
| def generate_batch(queue: list, parallel_count: int, steps: int, guidance: float): | ||
| """Generate all songs in the batch queue""" | ||
| pm = ProjectManager(PROJECTS_DIR) | ||
| dit_handler = get_dit_handler() | ||
|
|
||
| if not dit_handler: | ||
| st.error("❌ Failed to load generation model") | ||
| return | ||
|
|
||
| # Create progress tracking | ||
| progress_placeholder = st.empty() | ||
| status_placeholder = st.empty() | ||
| results_placeholder = st.empty() | ||
|
|
||
| total_songs = len(queue) | ||
| completed = 0 | ||
| failed = 0 | ||
| results = [] | ||
|
|
||
| with st.spinner(f"🎵 Generating {total_songs} songs..."): | ||
| try: | ||
| for idx, song in enumerate(queue): | ||
| # Update progress | ||
| progress = (idx + 1) / total_songs | ||
| progress_placeholder.progress(progress) | ||
| status_placeholder.text(f"Generating song {idx + 1}/{total_songs}: {song['caption'][:40]}...") | ||
|
|
||
| try: | ||
| # Create project | ||
| project_name = f"Batch_{song['caption'][:20].replace(' ', '_')}" | ||
| project_path = pm.create_project(project_name, description=song['caption']) | ||
|
|
||
| # TODO: Actual generation | ||
| # result = dit_handler.generate(song['caption'], duration=song['duration']) | ||
| # pm.save_audio(project_path, result['audio'], "output.wav") | ||
|
|
||
| results.append({ | ||
| "song": song['caption'], | ||
| "project": project_name, | ||
| "status": "✅ Success" | ||
| }) | ||
| completed += 1 | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Batch generation error for song {idx + 1}: {e}") | ||
| results.append({ | ||
| "song": song['caption'], | ||
| "project": "", | ||
| "status": f"❌ Failed: {str(e)[:50]}" | ||
| }) | ||
| failed += 1 |
There was a problem hiding this comment.
generate_batch is a stub that reports false success — every queued song shows "✅ Success" without any actual generation.
The TODO on line 162 confirms generation is not implemented, yet the function appends "✅ Success" to results unconditionally. This will mislead users into thinking songs were generated. At minimum, mark items as "
Additionally, line 133 uses get_dit_handler() (which only checks handler creation) instead of is_dit_ready() (which verifies model loading). Other components consistently use is_dit_ready().
Proposed fix for readiness check
+from utils import ProjectManager, get_dit_handler, is_dit_ready
+
def generate_batch(queue: list, parallel_count: int, steps: int, guidance: float):
"""Generate all songs in the batch queue"""
pm = ProjectManager(PROJECTS_DIR)
- dit_handler = get_dit_handler()
-
- if not dit_handler:
- st.error("❌ Failed to load generation model")
+
+ if not is_dit_ready():
+ st.error("❌ DiT model not loaded. Go to Settings → Models.")
returnDo you want me to generate the actual generation implementation using the generate_music API (similar to generation_wizard.py), or open an issue to track this?
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 130-130: Unused function argument: parallel_count
(ARG001)
[warning] 130-130: Unused function argument: steps
(ARG001)
[warning] 130-130: Unused function argument: guidance
(ARG001)
[error] 142-142: Local variable results_placeholder is assigned to but never used
Remove assignment to unused variable results_placeholder
(F841)
[error] 160-160: Local variable project_path is assigned to but never used
Remove assignment to unused variable project_path
(F841)
[warning] 173-173: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/components/batch_generator.py` around lines 130 - 180,
generate_batch currently uses get_dit_handler() (which only checks creation)
instead of is_dit_ready(), and it unconditionally appends "✅ Success" despite
the TODO not calling the real generation API; change the readiness check to call
is_dit_ready() (or verify the handler via is_dit_ready()) before proceeding, and
either implement actual generation using the existing generate_music/generate
API (as used in generation_wizard.py)—calling dit_handler.generate_music or the
equivalent, saving audio via pm.save_audio(project_path, audio_bytes,
"output.wav"), and setting result status to "✅ Success" only on successful
save—or, if you cannot implement generation now, replace the unconditional "✅
Success" with "⚠️ Not implemented" (or disable the batch action behind a feature
flag) and ensure exceptions still set "❌ Failed: ..." as before.
| tab_proj, tab_out, tab_upload = st.tabs( | ||
| ["📁 Projects", "📂 All outputs", "⬆️ Upload file"] | ||
| ) | ||
|
|
||
| audio_path: Optional[Path] = None | ||
|
|
||
| with tab_proj: | ||
| audio_path = _pick_from_projects(audio_path) | ||
|
|
||
| with tab_out: | ||
| audio_path = _pick_from_outputs(audio_path) | ||
|
|
||
| with tab_upload: | ||
| audio_path = _pick_from_upload(audio_path) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Streamlit tabs inactive widgets execution behavior do widgets run
💡 Result:
In Streamlit, yes—widgets (and all other code) inside inactive st.tabs still run on every script rerun.
- The official docs state: “All content within every tab is computed and sent to the frontend, regardless of which tab is selected.” Tabs currently don’t support conditional/lazy rendering. [1]
- So any widget definitions, expensive computations, API calls, etc. placed inside any tab will execute even when that tab isn’t selected. [1]
If you need “only run when selected”, you must implement your own conditional rendering pattern (the docs suggest using something like st.segmented_control to gate what gets rendered). [1]
Sources:
[1] Streamlit st.tabs documentation (note about all tab content being computed) https://docs.streamlit.io/develop/api-reference/layout/st.tabs
🏁 Script executed:
cat -n ace_studio_streamlit/components/editor_audio_picker.py | head -50Repository: ace-step/ACE-Step-1.5
Length of output: 1804
🏁 Script executed:
cat -n ace_studio_streamlit/components/editor_audio_picker.py | sed -n '44,120p'Repository: ace-step/ACE-Step-1.5
Length of output: 2735
Inactive tabs can overwrite the chosen audio source.
All tabs execute every run, so each helper reassigns audio_path. If the user selects from "Projects," then switches to "Outputs," the projects selection runs first, then outputs runs and may return its selection or the fallback (the project file), then upload runs and may overwrite again. The final audio_path depends on execution order and which helpers have selections, making the user's intent unclear. Use an explicit source selector (radio/segmented control) or confirm button to ensure only the active source sets audio_path.
Suggested change (radio-based source selection)
- tab_proj, tab_out, tab_upload = st.tabs(
- ["📁 Projects", "📂 All outputs", "⬆️ Upload file"]
- )
-
- audio_path: Optional[Path] = None
-
- with tab_proj:
- audio_path = _pick_from_projects(audio_path)
-
- with tab_out:
- audio_path = _pick_from_outputs(audio_path)
-
- with tab_upload:
- audio_path = _pick_from_upload(audio_path)
-
- return audio_path
+ source = st.radio(
+ "Audio source",
+ ["📁 Projects", "📂 All outputs", "⬆️ Upload file"],
+ horizontal=True,
+ )
+
+ if source == "📁 Projects":
+ return _pick_from_projects(None)
+ if source == "📂 All outputs":
+ return _pick_from_outputs(None)
+ return _pick_from_upload(None)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/components/editor_audio_picker.py` around lines 22 - 36,
The current tabs implementation calls all helpers (_pick_from_projects,
_pick_from_outputs, _pick_from_upload) every run and they each reassign
audio_path, so inactive tabs can overwrite the user's chosen source; change to
an explicit source selector (e.g., st.radio or segmented control) and only
invoke the selected helper based on that value (or require a confirm button
before assigning audio_path) so that only the active helper updates audio_path;
reference the tab variables (tab_proj/tab_out/tab_upload) and the helper names
to locate and replace the logic.
| def _pick_from_upload(fallback: Optional[Path]) -> Optional[Path]: | ||
| """Upload tab content – persist to temp file.""" | ||
| uploaded = st.file_uploader( | ||
| "Upload an audio file", | ||
| type=["wav", "mp3", "flac", "m4a"], | ||
| key="ed_upload", | ||
| ) | ||
| if uploaded is None: | ||
| return fallback | ||
| tmp = Path(tempfile.gettempdir()) / uploaded.name | ||
| tmp.write_bytes(uploaded.getvalue()) | ||
| return tmp |
There was a problem hiding this comment.
Sanitize uploaded filenames and avoid temp‑file overwrite.
uploaded.name can include path separators or absolute paths; writing it directly under the temp dir can overwrite arbitrary files. Use a safe basename and a unique temp file.
Suggested change
if uploaded is None:
return fallback
- tmp = Path(tempfile.gettempdir()) / uploaded.name
- tmp.write_bytes(uploaded.getvalue())
- return tmp
+ safe_name = Path(uploaded.name).name
+ suffix = Path(safe_name).suffix
+ with tempfile.NamedTemporaryFile(delete=False, suffix=suffix) as tmp:
+ tmp.write(uploaded.getvalue())
+ return Path(tmp.name)🧰 Tools
🪛 Ruff (0.15.2)
[warning] 92-92: Docstring contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF002)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/components/editor_audio_picker.py` around lines 91 -
102, In _pick_from_upload, sanitize uploaded.name and avoid deterministic
temp-paths: extract a safe basename from uploaded.name (drop any directories)
and create a unique temp file in the system temp dir (e.g., using
tempfile.NamedTemporaryFile or mkstemp) preserving the uploaded file extension;
write uploaded.getvalue() into that unique file and return its Path instead of
writing directly to Path(tempfile.gettempdir()) / uploaded.name to prevent path
traversal and overwrite of existing files.
|
|
||
| progress_bar = st.progress(0) | ||
| status_text = st.empty() | ||
|
|
||
| status_text.text("⏳ Preparing generation...") | ||
| progress_bar.progress(5) | ||
|
|
||
| # Use the high-level inference API | ||
| from acestep.inference import ( | ||
| GenerationParams, | ||
| GenerationConfig, | ||
| generate_music, | ||
| ) | ||
|
|
||
| params = GenerationParams( | ||
| task_type="text2music", | ||
| caption=caption, | ||
| lyrics=lyrics or "[Instrumental]", | ||
| duration=float(duration), | ||
| bpm=bpm, | ||
| keyscale=key, | ||
| inference_steps=inference_steps, | ||
| guidance_scale=guidance_scale, | ||
| seed=seed, | ||
| thinking=use_cot, | ||
| use_cot_metas=use_cot, | ||
| use_cot_caption=use_cot, | ||
| use_cot_language=use_cot, | ||
| ) | ||
|
|
||
| config = GenerationConfig( | ||
| batch_size=batch_size, | ||
| use_random_seed=(seed < 0), | ||
| seeds=[seed] if seed >= 0 else None, | ||
| ) | ||
|
|
||
| status_text.text("🎨 Running ACE-Step pipeline...") | ||
| progress_bar.progress(20) | ||
|
|
||
| result = generate_music( | ||
| dit_handler=dit_handler, | ||
| llm_handler=llm_handler, | ||
| params=params, | ||
| config=config, | ||
| save_dir=str(OUTPUT_DIR), | ||
| ) | ||
|
|
||
| progress_bar.progress(100) | ||
|
|
||
| if not result.success: | ||
| st.error(f"Generation failed: {result.error}") | ||
| return | ||
|
|
||
| status_text.text("✅ Generation complete!") | ||
|
|
||
| # Display generated audio files | ||
| if result.audios: | ||
| st.markdown("### 🎧 Results") | ||
| for idx, audio_info in enumerate(result.audios): | ||
| audio_path = audio_info.get("path", "") | ||
| if audio_path and Path(audio_path).exists(): | ||
| st.audio(audio_path) | ||
| st.caption( | ||
| f"Song {idx + 1} — " | ||
| f"{Path(audio_path).name}" | ||
| ) | ||
|
|
||
| # Save as project | ||
| pm = ProjectManager(PROJECTS_DIR) | ||
| safe_name = ( | ||
| caption[:30] | ||
| .replace(" ", "_") | ||
| .replace("/", "_") | ||
| ) | ||
| project_path = pm.create_project( | ||
| safe_name, description=caption | ||
| ) | ||
| pm.save_metadata( | ||
| project_path, | ||
| genre=caption, | ||
| mood="Generated", | ||
| duration=duration, | ||
| bpm=bpm, | ||
| ) | ||
|
|
||
| # Copy audio files into project | ||
| for audio_info in result.audios: | ||
| src = Path(audio_info.get("path", "")) | ||
| if src.exists(): | ||
| import shutil | ||
| dst = project_path / src.name | ||
| shutil.copy2(str(src), str(dst)) | ||
|
|
||
| st.success( | ||
| f"🎉 Saved as project '{safe_name}'" | ||
| ) | ||
|
|
||
| except Exception as exc: | ||
| logger.error(f"Generation error: {exc}") | ||
| st.error(f"❌ Generation failed: {exc}") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Module exceeds the 200 LOC hard cap (~370 lines).
Extract generate_song (and its helpers) into a dedicated module (e.g. generation_runner.py), keeping only the UI form in generation_wizard.py. As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC."
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 68-68: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
[warning] 101-101: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
[warning] 167-167: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
[warning] 221-221: Unpacked variable col_l is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[warning] 221-221: Unpacked variable col_r is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[warning] 368-368: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/components/generation_wizard.py` around lines 1 - 370,
The file is too large — extract the generation logic into a new module (e.g.
generation_runner.py) by moving the generate_song function and any
tightly-coupled helper logic (the block starting at def generate_song(...)
including use of get_dit_handler, get_llm_handler, is_dit_ready, OUTPUT_DIR,
ProjectManager, and the import/use of GenerationParams, GenerationConfig,
generate_music) out of generation_wizard.py; in generation_runner.py re-export
generate_song and keep logging/exception handling there, and in
generation_wizard.py replace the moved function with a single import from
generation_runner (e.g. from generation_runner import generate_song) and ensure
any symbols used by the UI (ProjectManager only if still needed) are imported or
removed; update imports for OUTPUT_DIR and any utilities referenced by the moved
code so generation_runner.py is self-contained and tests still pass.
| streamlit==1.40.2 | ||
| streamlit-player==0.1.5 | ||
| streamlit-audio-recorder==0.0.8 | ||
| plotly==5.24.1 | ||
| librosa==0.10.2 | ||
| numpy==1.26.4 | ||
| scipy==1.14.1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
soundfile is missing from dependencies.
librosa 0.10.x removed librosa.output and now uses soundfile for all writes (see save_audio_file issue below). soundfile is a runtime dependency of librosa but is not pinned here, which is a risk for reproducible installs.
➕ Proposed addition
librosa==0.10.2
numpy==1.26.4
scipy==1.14.1
+soundfile>=0.12.1📝 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.
| streamlit==1.40.2 | |
| streamlit-player==0.1.5 | |
| streamlit-audio-recorder==0.0.8 | |
| plotly==5.24.1 | |
| librosa==0.10.2 | |
| numpy==1.26.4 | |
| scipy==1.14.1 | |
| streamlit==1.40.2 | |
| streamlit-player==0.1.5 | |
| streamlit-audio-recorder==0.0.8 | |
| plotly==5.24.1 | |
| librosa==0.10.2 | |
| numpy==1.26.4 | |
| scipy==1.14.1 | |
| soundfile>=0.12.1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/requirements.txt` around lines 1 - 7, Add the missing
runtime dependency soundfile to requirements by pinning a compatible version
(e.g., add "soundfile==0.12.1") alongside librosa to ensure reproducible
installs and to satisfy librosa's audio write functionality; update the
requirements.txt entry list (which currently contains "librosa==0.10.2") to
include the new "soundfile" line so installs reliably include the needed
package.
| @echo off | ||
| REM Quick start script for ACE Studio Streamlit UI (Windows) | ||
|
|
||
| setlocal enabledelayedexpansion | ||
|
|
||
| echo 🎹 ACE Studio - Quick Start | ||
| echo ================================== | ||
|
|
||
| REM Check Python | ||
| echo Checking Python... | ||
| python --version | ||
|
|
||
| REM Check if venv exists | ||
| if not exist "..\\.venv" ( | ||
| echo Creating virtual environment... | ||
| python -m venv ..\\.venv | ||
| ) | ||
|
|
||
| REM Activate venv | ||
| echo Activating virtual environment... | ||
| call ..\\.venv\\Scripts\\activate.bat | ||
|
|
||
| REM Install dependencies | ||
| echo Installing Streamlit dependencies... | ||
| pip install -q -r requirements.txt | ||
|
|
||
| REM Run the app | ||
| echo. | ||
| echo ================================== | ||
| echo ✅ Setup complete! | ||
| echo 🚀 Starting ACE Studio... | ||
| echo 📱 Open: http://localhost:8501 | ||
| echo ================================== | ||
| echo. | ||
|
|
||
| streamlit run main.py | ||
|
|
||
| endlocal |
There was a problem hiding this comment.
Convert the batch file to CRLF line endings for Windows compatibility.
LF-only endings can cause cmd.exe parsing issues in batch scripts; please commit with CRLF before release.
🧰 Tools
🪛 Blinter (1.0.112)
[error] 1-1: Unix line endings detected. Explanation: Batch file uses Unix line endings (LF-only) which can cause GOTO/CALL label parsing failures and script malfunction due to Windows batch parser 512-byte boundary bugs. Recommendation: Convert file to Windows line endings (CRLF). Use tools like dos2unix, notepad++, or configure git with 'git config core.autocrlf true'. Context: File uses Unix line endings (LF-only) - 38 LF sequences found
(E018)
[error] 21-21: UNC path without UAC elevation check. Explanation: UNC path operations may fail under UAC without proper elevation checks. Recommendation: Check for administrator privileges before UNC operations using NET SESSION. Context: UNC path operation may fail under UAC without elevation check
(SEC020)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/run.bat` around lines 1 - 38, The batch script run.bat
currently uses LF-only line endings which can break cmd.exe parsing; update the
file to use CRLF line endings (Windows style) and re-save/commit run.bat with
CRLF line endings, ensuring the file (run.bat) is converted (e.g., via your
editor, git core.autocrlf or dos2unix/unix2dos conversion) before pushing the
change so the activation/call and other batch commands run correctly on Windows.
| def save_audio_file(audio_data: np.ndarray, file_path: str, sr: int = 16000) -> None: | ||
| """Save audio data to file""" | ||
| if not librosa: | ||
| raise ImportError("librosa is required for audio saving") | ||
|
|
||
| try: | ||
| librosa.output.write_wav(file_path, audio_data, sr=sr) | ||
| logger.info(f"Saved audio to {file_path}") | ||
| except Exception as e: | ||
| logger.error(f"Failed to save audio to {file_path}: {e}") | ||
| raise |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the audio_utils.py file
cat -n ace_studio_streamlit/utils/audio_utils.pyRepository: ace-step/ACE-Step-1.5
Length of output: 3312
🏁 Script executed:
# Check requirements.txt for librosa version
cat requirements.txt | grep -i librosaRepository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
# Find requirements.txt
fd requirements.txtRepository: ace-step/ACE-Step-1.5
Length of output: 119
🌐 Web query:
librosa 0.10.2 write_wav API removed output module
💡 Result:
In librosa 0.10.2, the old librosa.output submodule (including librosa.output.write_wav) is already removed—it was deprecated in 0.7.0 and removed in 0.8.0. Use soundfile.write instead. [1], [2]
Replacement:
import soundfile as sf
# y: np.ndarray audio samples, sr: sample rate
sf.write("out.wav", y, sr)Notes:
librosa.output.write_wav(..., norm=...)used to optionally normalize; if you relied on that, normalizeyyourself before callingsf.write. [2]
Sources:
[1] librosa changelog (notes removal of deprecated output module)
[2] librosa 0.7.2 docs for librosa.output.write_wav (deprecation + “replace by soundfile.write”)
🏁 Script executed:
# Check both requirements.txt files for librosa version
echo "=== root requirements.txt ===" && cat requirements.txt | grep -i librosa
echo "=== ace_studio_streamlit requirements.txt ===" && cat ace_studio_streamlit/requirements.txt | grep -i librosaRepository: ace-step/ACE-Step-1.5
Length of output: 156
🏁 Script executed:
# Also check for soundfile in requirements
echo "=== soundfile in root ===" && cat requirements.txt | grep -i soundfile
echo "=== soundfile in ace_studio ===" && cat ace_studio_streamlit/requirements.txt | grep -i soundfileRepository: ace-step/ACE-Step-1.5
Length of output: 140
librosa.output.write_wav was removed in librosa 0.8.0 — this will crash at runtime.
The deprecated output module was removed in the librosa 0.8 series. With librosa==0.10.2 pinned in requirements, calling librosa.output.write_wav raises AttributeError: module 'librosa' has no attribute 'output' on every invocation of save_audio_file. Replace with soundfile.write, which is available in dependencies.
Proposed fix
+import soundfile as sf
...
def save_audio_file(audio_data: np.ndarray, file_path: str, sr: int = 16000) -> None:
"""Save audio data to file"""
- if not librosa:
- raise ImportError("librosa is required for audio saving")
-
try:
- librosa.output.write_wav(file_path, audio_data, sr=sr)
+ sf.write(file_path, audio_data, sr)
logger.info(f"Saved audio to {file_path}")
except Exception as e:
logger.error(f"Failed to save audio to {file_path}: {e}")
raise🧰 Tools
🪛 Ruff (0.15.2)
[warning] 31-31: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/utils/audio_utils.py` around lines 28 - 38, The function
save_audio_file uses the removed API librosa.output.write_wav; update
save_audio_file to call soundfile.write (e.g., sf.write) instead: import
soundfile (alias sf) at top and replace the librosa.output.write_wav(file_path,
audio_data, sr=sr) call with sf.write(file_path, audio_data, sr). Keep the
existing try/except and logger messages, and ensure any conditional check for
librosa presence is adjusted if you rely on soundfile rather than librosa for
writing.
| def initialize_llm( | ||
| lm_model_path: str = "acestep-5Hz-lm-1.7B", | ||
| backend: str = "mlx", | ||
| device: str = "auto", | ||
| offload_to_cpu: bool = False, | ||
| ) -> Tuple[str, bool]: |
There was a problem hiding this comment.
Default backend="mlx" is Apple Silicon–only — will fail on Linux/Windows.
MLX is only available on macOS with Apple Silicon. Per the coding guidelines, the backend should be platform-aware. On CUDA systems the default should be something like "transformers" or "vllm", and on MPS/Apple Silicon, "mlx".
Suggested fix
+import platform
+
+def _default_llm_backend() -> str:
+ """Return the appropriate default LLM backend for the current platform."""
+ if platform.system() == "Darwin" and platform.machine() == "arm64":
+ return "mlx"
+ return "transformers"
+
+
def initialize_llm(
lm_model_path: str = "acestep-5Hz-lm-1.7B",
- backend: str = "mlx",
+ backend: str = _default_llm_backend(),
device: str = "auto",
offload_to_cpu: bool = False,
) -> Tuple[str, bool]:Note: using a mutable default via function call is fine here since strings are immutable and the function is pure.
As per coding guidelines: "Use gpu_config.py for hardware detection and configuration; DO NOT alter non-target platform paths unless explicitly required."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/utils/cache.py` around lines 106 - 111, The default
backend in initialize_llm currently hardcodes "mlx" which only works on Apple
Silicon; change it to select a platform-appropriate default by delegating to the
platform detection utilities in gpu_config.py (e.g., detect CUDA vs
MPS/AppleSilicon) and set backend accordingly (e.g., "vllm"/"transformers" for
CUDA, "mlx" for MPS). Update the function signature to use a neutral default
(e.g., backend: str = None) and inside initialize_llm call the gpu_config
detection helper to assign the correct backend before proceeding; reference the
initialize_llm function and gpu_config module when making the change.
| def create_project(self, name: str, description: str = "") -> Path: | ||
| """Create a new project folder""" | ||
| project_path = self.projects_dir / name | ||
| project_path.mkdir(exist_ok=True) |
There was a problem hiding this comment.
Path traversal — unsanitized name can escape projects_dir.
name is user-controlled (derived from captions, UI input, etc.) and is directly joined to projects_dir. A name like ../../etc/cron.d/evil would create directories outside the intended project root. The same risk applies to get_project (line 57), delete_project (line 114), and indirectly to the editor_runner's save flow.
Suggested fix
Add a sanitization/validation step:
+ `@staticmethod`
+ def _safe_name(name: str) -> str:
+ """Sanitize project name to prevent path traversal."""
+ import re
+ safe = re.sub(r"[^\w\-.]", "_", name)
+ safe = safe.strip("._ ")
+ if not safe:
+ safe = "unnamed_project"
+ return safe
+
def create_project(self, name: str, description: str = "") -> Path:
"""Create a new project folder"""
- project_path = self.projects_dir / name
+ project_path = self.projects_dir / self._safe_name(name)
project_path.mkdir(exist_ok=True)Apply the same sanitization in get_project, delete_project, or validate that the resolved path is under projects_dir:
resolved = (self.projects_dir / name).resolve()
if not resolved.is_relative_to(self.projects_dir.resolve()):
raise ValueError(f"Invalid project name: {name}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ace_studio_streamlit/utils/project_manager.py` around lines 40 - 43, The
create_project method (and similarly get_project and delete_project) currently
joins the user-controlled name to projects_dir without validation, allowing path
traversal; fix this by resolving the candidate path and ensuring it is a
descendant of self.projects_dir before performing mkdir/remove/return.
Concretely, in create_project (and mirror in get_project/delete_project) compute
resolved = (self.projects_dir / name).resolve() and validate using
resolved.is_relative_to(self.projects_dir.resolve()) (or an equivalent
parent-check for older Python) and raise a ValueError for invalid names; only
then proceed to create or return the path.
|
place your folder in acestep/ui/ |
- Relocated ace_studio_streamlit/ to acestep/ui/ace_studio_streamlit/ - Updated config.py ACESTEP_ROOT path traversal (1 -> 3 parents) - Updated utils/cache.py sys.path traversal (3 -> 5 parents) - Updated run.sh and run.bat .venv relative paths - Updated markdown docs with new directory paths
There was a problem hiding this comment.
Actionable comments posted: 19
♻️ Duplicate comments (15)
acestep/ui/ace_studio_streamlit/INSTALL.md (1)
99-114: Docker image is still missing the parentacesteppackage.The
COPY . .instruction only copies theace_studio_streamlit/subtree into the image; the parentacesteppackage that the app imports at runtime is absent, causingModuleNotFoundErroron startup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/INSTALL.md` around lines 99 - 114, The Dockerfile currently copies only the streamlit subtree (COPY . .) so the parent Python package acestep is missing at runtime; fix this by building from the repository root (set docker build context to repo root) or by changing the Dockerfile to explicitly include the parent package (e.g., COPY the acestep package into /app) so that imports of acestep succeed at container startup; update the docker build command and/or the Dockerfile COPY directives accordingly.acestep/ui/ace_studio_streamlit/requirements.txt (1)
1-7:⚠️ Potential issue | 🟠 MajorAdd
soundfileto lock in librosa’s audio I/O dependency.
librosa==0.10.2relies onsoundfilefor audio writes; without an explicit pin, installs can miss or drift, breaking output. Please add a compatible pin.➕ Suggested pin
librosa==0.10.2 numpy==1.26.4 scipy==1.14.1 +soundfile==0.12.1librosa 0.10 soundfile dependency required for audio write recommended version🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/requirements.txt` around lines 1 - 7, The requirements pin is missing soundfile which librosa==0.10.2 needs for audio I/O; update the ace_studio_streamlit requirements.txt to explicitly add a compatible soundfile pin (e.g., add soundfile==0.12.1) alongside the existing librosa==0.10.2 entry so installs don't miss or drift the dependency.acestep/ui/ace_studio_streamlit/components/editor_audio_picker.py (2)
22-37:⚠️ Potential issue | 🟠 MajorInactive tabs can overwrite the selected audio source.
st.tabsexecutes all tab content every run, so each helper reassignsaudio_path, and the last tab wins. Use a source selector (radio/segmented control) to render only the chosen source.🔁 Suggested change
- tab_proj, tab_out, tab_upload = st.tabs( - ["📁 Projects", "📂 All outputs", "⬆️ Upload file"] - ) - - audio_path: Optional[Path] = None - - with tab_proj: - audio_path = _pick_from_projects(audio_path) - - with tab_out: - audio_path = _pick_from_outputs(audio_path) - - with tab_upload: - audio_path = _pick_from_upload(audio_path) - - return audio_path + source = st.radio( + "Audio source", + ["📁 Projects", "📂 All outputs", "⬆️ Upload file"], + horizontal=True, + ) + if source == "📁 Projects": + return _pick_from_projects(None) + if source == "📂 All outputs": + return _pick_from_outputs(None) + return _pick_from_upload(None)Streamlit st.tabs inactive tabs still executed all content computed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/editor_audio_picker.py` around lines 22 - 37, The tab blocks (tab_proj/tab_out/tab_upload) all run each rerun so each helper (_pick_from_projects, _pick_from_outputs, _pick_from_upload) keeps reassigning audio_path and the last tab wins; change to a single source selector (radio or st.selectbox/segmented control) and call only the corresponding helper based on the selected source, e.g. present options like "projects", "outputs", "upload" then if source == "projects" call _pick_from_projects(audio_path) else if "outputs" call _pick_from_outputs(audio_path) else call _pick_from_upload(audio_path) so inactive sources do not overwrite audio_path.
91-102:⚠️ Potential issue | 🟠 MajorSanitize uploaded filenames and avoid temp-file overwrite.
uploaded.namecan include path separators, and writing directly into the temp dir can overwrite existing files. Use a safe basename and a unique temp file.🧷 Suggested fix
if uploaded is None: return fallback - tmp = Path(tempfile.gettempdir()) / uploaded.name - tmp.write_bytes(uploaded.getvalue()) - return tmp + safe_name = Path(uploaded.name).name + suffix = Path(safe_name).suffix + with tempfile.NamedTemporaryFile(delete=False, suffix=suffix) as tmp: + tmp.write(uploaded.getvalue()) + return Path(tmp.name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/editor_audio_picker.py` around lines 91 - 102, In _pick_from_upload, uploaded.name must be sanitized and a unique temp file must be used to avoid path-traversal and overwrites; replace writing to Path(tempfile.gettempdir()) / uploaded.name with creating a secure temp file (e.g., via tempfile.NamedTemporaryFile or mkstemp) using the sanitized basename (Path(uploaded.name).name) to derive a safe suffix/extension, write uploaded.getvalue() into that unique temp path, and return that temp Path instead of using the raw uploaded.name.acestep/ui/ace_studio_streamlit/run.bat (1)
1-38:⚠️ Potential issue | 🟠 MajorConvert this batch file to CRLF line endings.
LF-only endings can break cmd.exe parsing; please re-save with CRLF before release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/run.bat` around lines 1 - 38, The run.bat file currently uses LF-only line endings which can break cmd.exe parsing; re-save the file with CRLF line endings (Windows style) so cmd can parse it correctly—open the file (run.bat) in an editor or use a tool/command to convert line endings and ensure the entire script (including the shebang-less header, calls to ..\..\..\.venv\Scripts\activate.bat, pip install -r requirements.txt and the final streamlit run main.py invocation) is written with CRLF before committing.acestep/ui/ace_studio_streamlit/components/batch_generator.py (2)
130-137:⚠️ Potential issue | 🟠 MajorUse a readiness check instead of handler creation.
get_dit_handler()only ensures the handler instance exists; it doesn’t confirm the model is loaded. Please switch to an explicit readiness check before proceeding.✅ Suggested update
-from utils import ProjectManager, get_dit_handler +from utils import ProjectManager, get_dit_handler, is_dit_ready @@ - dit_handler = get_dit_handler() - - if not dit_handler: - st.error("❌ Failed to load generation model") + if not is_dit_ready(): + st.error("❌ DiT model not loaded. Go to Settings → Models.") return + dit_handler = get_dit_handler()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/batch_generator.py` around lines 130 - 137, The current generate_batch function only calls get_dit_handler() which returns a handler instance but does not confirm the model is loaded; replace the simple presence check with an explicit readiness check on the handler (e.g., call a readiness method like is_ready(), ready(), or model_loaded on the returned object) and only proceed if that check returns true — otherwise show the error and return; if the handler exposes an async/long-load behavior consider waiting/polling with a short timeout or retry loop before failing to improve UX. Ensure you update the check inside generate_batch and reference the dit_handler variable for this readiness call.
149-171:⚠️ Potential issue | 🟠 MajorBatch generation is still stubbed but reports success.
The TODO block skips actual generation, yet each item is marked “✅ Success”. This misleads users and ignores
parallel_count,steps, andguidance. Either wire this up to the realgenerate_musiccall or gate the feature behind an explicit flag and surface “not implemented”.🧯 Minimal non-misleading fallback
- results.append({ - "song": song['caption'], - "project": project_name, - "status": "✅ Success" - }) - completed += 1 + results.append({ + "song": song["caption"], + "project": project_name, + "status": "⚠️ Not implemented" + }) + failed += 1As per coding guidelines: Gate WIP or unstable UI/API paths behind explicit feature/release flags.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/batch_generator.py` around lines 149 - 171, The batch generator currently skips actual generation (the commented dit_handler.generate / pm.save_audio) but still appends "✅ Success"; fix by either wiring in the real generation call or gating the stub behind a feature flag. Locate the loop in batch_generator.py where project_name is created, then (A) implement the call to dit_handler.generate(song['caption'], duration=song['duration'], steps=steps, guidance=guidance, parallel_count=parallel_count), save the audio via pm.save_audio(project_path, result['audio'], "output.wav"), and set results status based on success/failure, or (B) introduce an explicit flag (e.g., enable_batch_generation) and when false set results status to "Not implemented" (or similar) and do not increment completed; ensure progress/status placeholders reflect the non-implemented state and that parallel_count, steps, and guidance are respected when generation is enabled.acestep/ui/ace_studio_streamlit/utils/project_manager.py (1)
40-62:⚠️ Potential issue | 🟠 MajorValidate project names to prevent path traversal.
nameis user-controlled and is joined directly toprojects_dir, allowing paths like../../...to escape the root. Please sanitize or validate that the resolved path stays underprojects_dir(apply increate_project,get_project, anddelete_project).🛡️ Suggested fix (create_project)
def create_project(self, name: str, description: str = "") -> Path: """Create a new project folder""" - project_path = self.projects_dir / name + project_root = self.projects_dir.resolve() + project_path = (self.projects_dir / name).resolve() + if not project_path.is_relative_to(project_root): + raise ValueError(f"Invalid project name: {name}") project_path.mkdir(exist_ok=True)Also applies to: 114-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/utils/project_manager.py` around lines 40 - 62, User-controlled project names can cause path traversal; validate/sanitize in create_project, get_project and delete_project by ensuring the final resolved project path remains under self.projects_dir. Construct project_path, call project_path.resolve() and compare against self.projects_dir.resolve() (or use Path.is_relative_to) and reject names that contain path separators or '..' (raise/return an error) before creating, returning, or deleting; this prevents escapes from the projects_dir and ensures safe use in _save_metadata and other code that uses project_path.acestep/ui/ace_studio_streamlit/components/audio_player.py (1)
39-46:⚠️ Potential issue | 🟠 MajorUpdate the
librosa.get_durationcall and avoid bareexcept.
filename=is deprecated in librosa 0.10+, and the bareexceptviolates the error-handling guideline. Please switch topath=and catch explicit exceptions.✅ Suggested fix
try: import librosa - duration = librosa.get_duration(filename=audio_path) + duration = librosa.get_duration(path=audio_path) minutes = int(duration // 60) seconds = int(duration % 60) st.metric("Duration", f"{minutes}m {seconds}s") - except: + except ImportError: + st.metric("Duration", "Unknown") + except (OSError, ValueError, librosa.util.exceptions.ParameterError): st.metric("Duration", "Unknown")librosa get_duration filename parameter deprecated path renamed 0.10As per coding guidelines: Handle errors explicitly in Python; avoid bare
except.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/audio_player.py` around lines 39 - 46, Change the deprecated call librosa.get_duration(filename=audio_path) to use the new parameter name path (librosa.get_duration(path=audio_path)), and replace the bare except with an explicit exception handler (e.g., except Exception as e) so you can surface the error; update the except block to call st.metric("Duration", "Unknown") and also log or display the exception (using e) to aid debugging; target the librosa.get_duration call, the audio_path variable, and the st.metric usage in audio_player.py.acestep/ui/ace_studio_streamlit/main.py (1)
129-138: Silentexcept Exception: passswallows failures silently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/main.py` around lines 129 - 138, The try/except block around importing ProjectManager/PROJECTS_DIR and calling ProjectManager(PROJECTS_DIR).list_projects() swallows all errors—replace the bare "except Exception: pass" with specific error handling: catch ImportError and any file/IO errors (e.g., OSError) and log/display the error (use st.error or the module logger) so failures are visible, and for unexpected exceptions either re-raise or log them with traceback; keep the st.metric("💾 Projects", len(projects)) flow only when projects is successfully retrieved.acestep/ui/ace_studio_streamlit/components/generation_wizard.py (1)
1-370: Module exceeds the 200 LOC hard cap (370 lines).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/generation_wizard.py` around lines 1 - 370, This module is too large; split UI and generation logic into smaller files: keep the Streamlit UI (show_generation_wizard and _quick_init_dit plus UI helpers like caption/default handling and any st.* calls) in a trimmed component (e.g., generation_ui) and move the heavy inference logic (generate_song and the GenerationParams/GenerationConfig construction, result handling, ProjectManager I/O and file-copying) into a separate generation_logic module; update imports so generation_ui calls generate_song from generation_logic and ensure constants (OUTPUT_DIR, PROJECTS_DIR, GENRES, MOODS, DEFAULT_DURATION, DEFAULT_BPM) remain imported where needed and preserve function names show_generation_wizard, _quick_init_dit, and generate_song to make the refactor local and minimal.acestep/ui/ace_studio_streamlit/utils/cache.py (1)
106-111: Defaultbackend="mlx"is Apple Silicon–only — previously flagged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/utils/cache.py` around lines 106 - 111, The default backend for initialize_llm is set to "mlx" which is Apple Silicon–only; change the default backend parameter (backend) to a cross-platform default such as "auto" (or "ggml"/"cpu" depending on supported backends) and implement platform detection inside initialize_llm to select "mlx" only when running on Apple Silicon (e.g., check platform.machine() or torch.backends.mps availability) so non-Apple users don't hit an unsupported default; update any related docstring/comments to reflect the new default behavior and ensure callers relying on "mlx" still work by explicitly passing backend="mlx".acestep/ui/ace_studio_streamlit/utils/audio_utils.py (1)
28-38:librosa.output.write_wavwas removed in librosa 0.8.0 — already flagged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/utils/audio_utils.py` around lines 28 - 38, The save_audio_file function uses the removed librosa.output.write_wav API; update save_audio_file to use soundfile.write (import soundfile as sf) or librosa's recommended alternative (sf.write(file_path, audio_data, sr)) instead of librosa.output.write_wav, ensure audio_data dtype/shape is compatible (e.g., float32, (n,) or (n, channels)), keep the same logging and exception handling, and update the import/error message to reflect that soundfile (pysoundfile) is required if librosa no longer provides write_wav; reference the save_audio_file function and the removed librosa.output.write_wav call when making the change.acestep/ui/ace_studio_streamlit/components/settings_panel.py (2)
1-297: Module exceeds the 200 LOC hard cap (~297 lines) — previously flagged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/settings_panel.py` around lines 1 - 297, The file exceeds the 200-line cap — split the large settings component into smaller modules and re-export a single entry point: keep show_settings_panel in this file but move _show_model_settings, _show_hardware_settings, _show_storage_settings, _show_about_section into a new ui_settings_tabs.py (or similar) and move helpers _list_dit_models and _list_lm_models into a new settings_helpers.py (or a handlers module) that imports get_dit_handler/get_llm_handler; then update this module to import those functions (e.g., from ui_settings_tabs import _show_model_settings, _show_hardware_settings, _show_storage_settings, _show_about_section and from settings_helpers import _list_dit_models, _list_lm_models) so the public API still exposes show_settings_panel but the file length is reduced below 200 lines.
260-297: 🛠️ Refactor suggestion | 🟠 MajorSilent
except Exception: passswallows errors — log the failure.Both
_list_dit_modelsand_list_lm_modelscatch all exceptions and silently pass. When a handler exists but its method fails (wrong API, permission error, etc.), the UI silently falls back with no trace in logs, making debugging painful.Proposed fix
if handler is not None: try: models = handler.get_available_acestep_v15_models() if models: return models except Exception: - pass + logger.warning("Failed to list DiT models from handler", exc_info=True)Apply the same pattern to
_list_lm_models.As per coding guidelines: "Handle errors explicitly in Python; avoid bare
except" and "Uselogurulogger instead ofprint()for logging."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/settings_panel.py` around lines 260 - 297, Both _list_dit_models and _list_lm_models currently swallow all exceptions; update them to catch Exception as e and log the failure using the loguru logger (e.g., logger.exception or logger.error with the exception info) when handler.get_available_acestep_v15_models() or handler.get_available_5hz_lm_models() raises; keep the existing fallback disk-scan behavior but ensure the log includes context (function name and handler type) and the exception details to aid debugging.
🧹 Nitpick comments (15)
acestep/ui/ace_studio_streamlit/INSTALL.md (1)
1-3: Python docstring delimiters at the top of a Markdown file.Lines 1 and 3 contain
""", which appear as literal triple-quote characters in rendered Markdown. This is leftover Python-file convention and should be removed.✏️ Suggested fix
-""" -ACE Studio Streamlit - Installation & Setup Guide -""" - -# Installation Instructions +# ACE Studio Streamlit — Installation & Setup Guide🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/INSTALL.md` around lines 1 - 3, Remove the Python-style triple-quote docstring delimiters (the leading and trailing """ characters) at the top of the INSTALL.md file so the Markdown header renders correctly; edit the top of the file (the current block containing """ACE Studio Streamlit - Installation & Setup Guide""") to leave only the plain Markdown title text without the surrounding triple quotes.acestep/ui/ace_studio_streamlit/README.md (1)
40-57: Fenced code blocks without language specifier (MD040) — three occurrences.Lines 40, 96, and 173 all open bare
```fences for directory/project structure blocks. Addingtext(orplaintext) removes the markdownlint warnings.✏️ Suggested fix (apply to all three blocks)
-``` +```text acestep/ui/ace_studio_streamlit/ ├── main.py ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/README.md` around lines 40 - 57, The README.md contains three bare fenced code blocks showing directory/project trees that trigger MD040; update each opening fence from ``` to ```text (or ```plaintext) so the blocks are explicitly plain text—apply this change to the three directory tree blocks in the README (the directory listing blocks under the ace_studio_streamlit section) to remove the markdownlint warnings.acestep/ui/ace_studio_streamlit/QUICKSTART.md (1)
8-31: Fenced code blocks lack a language specifier (MD040).Both the project tree (Line 8) and the architecture diagram (Line 135) use bare
```fences. Addingtext(orplaintext) silences the markdownlint warning and improves rendering on some renderers.✏️ Suggested fix (apply to both blocks)
-``` +```text acestep/ui/ace_studio_streamlit/ ├── main.py ...Also applies to: 135-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/QUICKSTART.md` around lines 8 - 31, The fenced code blocks in QUICKSTART.md (the project tree block under the project listing and the architecture diagram block) are missing a language specifier which triggers MD040; update both triple-backtick fences to include a plain-text language tag (e.g., ```text or ```plaintext) so they render correctly and silence the markdownlint warning—locate the blocks by the project tree content (acestep/ui/ace_studio_streamlit/ ├── main.py ...) and the architecture diagram text and add the specifier to each opening fence.acestep/ui/ace_studio_streamlit/components/audio_player.py (1)
9-10: Either uselabelor remove it from the API.Both functions accept
labelbut never render it, which is confusing for callers. Consider showing it (e.g.,st.caption(label)) or removing the parameter.Also applies to: 62-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/audio_player.py` around lines 9 - 10, The functions audio_player_widget (and the other audio-related function that currently accepts label but doesn't render it) expose a label parameter that's never shown; either remove the label parameter from the function signatures and docs or render it in the UI—preferably call streamlit.caption/st.caption-like (e.g., st.caption(label)) immediately before rendering the player in audio_player_widget and the other function, update the docstrings to reflect the change, and remove unused parameter imports/usages if you choose to delete it to keep the API consistent.acestep/ui/ace_studio_streamlit/components/generation_wizard.py (3)
356-362:import shutilinside a loop.Move the import to the top of
generate_song(or module level).🔧 Proposed fix
+import shutil + def generate_song(...) -> None:Then remove the
import shutilat Line 360.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/generation_wizard.py` around lines 356 - 362, The import shutil is done inside the loop when copying audio files (see loop over result.audios and variables src and dst) — move the import statement out of the loop to the top of the generate_song function or module-level imports in generation_wizard.py, then remove the in-loop `import shutil` so that shutil.copy2 is called from an already-imported module; this touches the block that references result.audios, src, dst, project_path and the generate_song function.
32-46:_quick_init_ditre-importssysunnecessarily.
sysis already imported at Line 7; theimport sys as _syson Line 34 is redundant.🔧 Proposed fix
def _quick_init_dit() -> None: """One-click DiT init from the Generate page.""" - import sys as _sys - with st.spinner("Loading DiT model..."): _status, _ok = initialize_dit( config_path="acestep-v15-turbo", device="auto", - offload_to_cpu=(_sys.platform != "darwin"), + offload_to_cpu=(sys.platform != "darwin"), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/generation_wizard.py` around lines 32 - 46, The function _quick_init_dit unnecessarily re-imports sys as _sys; remove the local import line and use the module-level sys import already present, i.e., change offload_to_cpu=(_sys.platform != "darwin") to offload_to_cpu=(sys.platform != "darwin") so the call to initialize_dit remains the same and no duplicate import is introduced.
221-221: Unused layout variablescol_landcol_r(Ruff RUF059).🔧 Proposed fix
- col_l, col_c, col_r = st.columns([1, 2, 1]) + _, col_c, _ = st.columns([1, 2, 1])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/generation_wizard.py` at line 221, The variables col_l and col_r from the tuple assignment in generation_wizard.py (col_l, col_c, col_r = st.columns([1, 2, 1])) are unused and trigger RUF059; replace them with throwaway names so only the middle column is kept (e.g. use _, col_c, _ = st.columns([1, 2, 1])) or grab the middle element directly (e.g. col_c = st.columns([1, 2, 1])[1]) to remove the unused variables while preserving layout.acestep/ui/ace_studio_streamlit/main.py (1)
147-147: Deferredfrom utils import is_llm_readyinside sidebar block.Move to the top-level imports with the other
utilsimports at Line 57 for consistency and clarity. As per coding guidelines, keep imports at module level.🔧 Proposed fix
-from utils import is_dit_ready, initialize_dit, initialize_llm +from utils import is_dit_ready, is_llm_ready, initialize_dit, initialize_llmThen remove the duplicate import at Line 147.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/main.py` at line 147, Move the deferred import "from utils import is_llm_ready" into the module-level imports alongside the existing utils imports (so is_llm_ready is imported at top with other utils symbols) and remove the duplicate import currently inside the sidebar block; update any local references to use the top-level is_llm_ready import instead of doing an in-block import.acestep/ui/ace_studio_streamlit/components/editor_waveform.py (2)
64-64: Redundantint()cast aroundmath.ceil()(Ruff RUF046).
math.ceilreturnsintin Python 3; the outerint()is a no-op.🔧 Proposed fix
- dur_int = max(1, int(math.ceil(duration_sec))) + dur_int = max(1, math.ceil(duration_sec))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/editor_waveform.py` at line 64, The assignment to dur_int wraps math.ceil in an unnecessary int() call; since math.ceil returns an int in Python 3, remove the outer int() and set dur_int = max(1, math.ceil(duration_sec)) (locate the dur_int assignment near math.ceil in editor_waveform.py).
89-101: Silent exception swallowing in_get_durationobscures failures.Both fallback paths silently discard exceptions, causing the 120 s fallback to be used without any diagnostic. Add a
logger.debugcall (at minimum) so the cause is visible when the region selector or task behaves oddly.🔧 Proposed fix
+from loguru import logger ... def _get_duration(audio_path: Path) -> float: """Return audio duration in seconds.""" if sf is not None: try: return sf.info(str(audio_path)).duration - except Exception: - pass + except Exception as exc: + logger.debug(f"soundfile could not read duration for {audio_path}: {exc}") if librosa is not None: try: return librosa.get_duration(path=str(audio_path)) - except Exception: - pass + except Exception as exc: + logger.debug(f"librosa could not read duration for {audio_path}: {exc}") return 120.0 # fallback🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/editor_waveform.py` around lines 89 - 101, The helper _get_duration currently swallows exceptions from both soundfile (sf.info) and librosa.get_duration causing silent fallbacks; update _get_duration to catch exceptions and call the module logger.debug (or an appropriate logger) with context and the exception (including traceback or str(e)) for each failed attempt (i.e., when calling sf.info in _get_duration and when calling librosa.get_duration in _get_duration), then continue to the next fallback and finally return the 120.0 fallback if both fail.acestep/ui/ace_studio_streamlit/components/editor_runner.py (2)
12-18: Same fragile bare imports as noted insettings_panel.py.
from utils import …andfrom config import …rely onsys.pathmanipulation. Prefer relative imports (from ..utils import …,from ..config import …).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/editor_runner.py` around lines 12 - 18, The current fragile absolute imports in editor_runner.py (importing get_dit_handler, get_llm_handler, is_dit_ready, ProjectManager from utils and OUTPUT_DIR, PROJECTS_DIR from config) should be changed to relative imports to avoid relying on sys.path hacks; update the import statements to use package-relative form (e.g., from ..utils import get_dit_handler, get_llm_handler, is_dit_ready, ProjectManager and from ..config import OUTPUT_DIR, PROJECTS_DIR) so the module resolution is robust within the package.
82-126:_generateis missing a return type annotation.The function returns whatever
generate_music()returns but the signature says-> ???. Adding the return type improves readability and IDE support.Suggested fix
+from typing import Any + def _generate( task_type: str, ... seed: int, -): +) -> Any: """Build params, invoke generate_music, return GenerationResult."""Ideally, use the concrete type (e.g.
GenerationResult) if it's importable at annotation time or useTYPE_CHECKING:from __future__ import annotations from typing import TYPE_CHECKING if TYPE_CHECKING: from acestep.inference import GenerationResultAs per coding guidelines: "Add type hints for new/modified functions when practical in Python."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/editor_runner.py` around lines 82 - 126, The _generate function lacks a return type annotation; add a proper annotation of GenerationResult to its signature (e.g., -> "GenerationResult") and ensure GenerationResult is available for annotations by either using from __future__ import annotations and importing GenerationResult under a TYPE_CHECKING block (from typing import TYPE_CHECKING; if TYPE_CHECKING: from acestep.inference import GenerationResult) or importing it directly if safe at runtime; leave the call to generate_music unchanged and keep get_dit_handler/get_llm_handler references as-is.acestep/ui/ace_studio_streamlit/components/editor_tasks.py (1)
13-65: Consider extracting the shared "prompt + lyrics + advanced controls" pattern.
repaint_ui,cover_ui, andcomplete_uieach duplicate the prompt text area, lyrics text area, advanced expander with diffusion-steps slider and seed input. A small helper (e.g._common_prompt_controls(prefix) -> Tuple[str, str, int, int]) could DRY this up and ensure consistent defaults across all three panels.Also applies to: 129-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/editor_tasks.py` around lines 13 - 65, The UI functions repaint_ui, cover_ui and complete_ui duplicate the same "prompt + lyrics + Advanced (steps, seed)" controls; extract them into a helper like _common_prompt_controls(prefix: str) -> Tuple[str, str, int, int] that renders the prompt text_area (key f"{prefix}_prompt"), lyrics text_area (key f"{prefix}_lyrics"), and the expander with steps slider (key f"{prefix}_steps") and seed number_input (key f"{prefix}_seed"), then update repaint_ui, cover_ui and complete_ui to call this helper and use the returned (prompt, lyrics, steps, seed) values when calling run_edit_task so default keys and behavior remain consistent across all three UIs.acestep/ui/ace_studio_streamlit/utils/cache.py (1)
15-18:sys.pathmanipulation is the root cause of fragile imports across the package.Injecting the project root into
sys.pathat import time enables the barefrom config import …/from utils import …seen throughout the package, but this is fragile — it breaks underpip install, different working directories, or if another package shadows those names. Prefer structuring the Streamlit app as a proper Python package with relative imports, or use apyproject.toml/setup.pyentry point that sets up the package correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/utils/cache.py` around lines 15 - 18, The direct manipulation of sys.path via _project_root = Path(__file__).parent.parent.parent.parent.parent and sys.path.insert(...) is causing fragile imports; remove this sys.path hack and convert the Streamlit app to use package-relative imports (or add a proper packaging entry point). Specifically, delete the _project_root/sys.path insertion in ace_studio_streamlit/utils/cache.py and update any bare imports like "from config import ..." or "from utils import ..." to explicit relative imports (e.g., from .config or from ..utils where appropriate) or ensure the project is installed via pyproject.toml/setup.py so top-level imports resolve reliably. Ensure references to _project_root and sys.path.insert are removed and tests/run scripts use the installed package import paths.acestep/ui/ace_studio_streamlit/components/settings_panel.py (1)
10-18: Barefrom config import …/from utils import …relies onsys.pathmanipulation.These absolute-looking imports only work because
cache.pyinjects the project root intosys.path. This is fragile and can break if the working directory changes or if the package is installed as a proper Python package. Prefer relative imports within the package.Suggested fix
-from config import PROJECTS_DIR, CACHE_DIR, CHECKPOINTS_DIR -from utils import ( +from ..config import PROJECTS_DIR, CACHE_DIR, CHECKPOINTS_DIR +from ..utils import ( get_dit_handler, get_llm_handler, is_dit_ready,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/ace_studio_streamlit/components/settings_panel.py` around lines 10 - 18, The file uses absolute imports ("from config import PROJECTS_DIR, CACHE_DIR, CHECKPOINTS_DIR" and "from utils import get_dit_handler, get_llm_handler, is_dit_ready, is_llm_ready, initialize_dit, initialize_llm") which rely on sys.path hacks; change them to package-relative imports so the module resolves correctly when installed or run from other CWDs—replace those imports with relative ones (e.g., from ...config import PROJECTS_DIR, CACHE_DIR, CHECKPOINTS_DIR and from .utils or from ..<appropriate_package>.utils import the listed functions) ensuring the import level matches the package layout and updating the import targets to the exact module names containing get_dit_handler, get_llm_handler, is_dit_ready, is_llm_ready, initialize_dit, and initialize_llm.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
acestep/ui/ace_studio_streamlit/.gitignoreacestep/ui/ace_studio_streamlit/.streamlit/config.tomlacestep/ui/ace_studio_streamlit/.streamlit/secrets.tomlacestep/ui/ace_studio_streamlit/INSTALL.mdacestep/ui/ace_studio_streamlit/PROJECT_SUMMARY.mdacestep/ui/ace_studio_streamlit/QUICKSTART.mdacestep/ui/ace_studio_streamlit/README.mdacestep/ui/ace_studio_streamlit/components/__init__.pyacestep/ui/ace_studio_streamlit/components/audio_player.pyacestep/ui/ace_studio_streamlit/components/batch_generator.pyacestep/ui/ace_studio_streamlit/components/dashboard.pyacestep/ui/ace_studio_streamlit/components/editor.pyacestep/ui/ace_studio_streamlit/components/editor_audio_picker.pyacestep/ui/ace_studio_streamlit/components/editor_runner.pyacestep/ui/ace_studio_streamlit/components/editor_tasks.pyacestep/ui/ace_studio_streamlit/components/editor_waveform.pyacestep/ui/ace_studio_streamlit/components/generation_wizard.pyacestep/ui/ace_studio_streamlit/components/settings_panel.pyacestep/ui/ace_studio_streamlit/config.pyacestep/ui/ace_studio_streamlit/main.pyacestep/ui/ace_studio_streamlit/requirements.txtacestep/ui/ace_studio_streamlit/run.batacestep/ui/ace_studio_streamlit/run.shacestep/ui/ace_studio_streamlit/utils/__init__.pyacestep/ui/ace_studio_streamlit/utils/audio_utils.pyacestep/ui/ace_studio_streamlit/utils/cache.pyacestep/ui/ace_studio_streamlit/utils/project_manager.py
✅ Files skipped from review due to trivial changes (3)
- acestep/ui/ace_studio_streamlit/.gitignore
- acestep/ui/ace_studio_streamlit/.streamlit/secrets.toml
- acestep/ui/ace_studio_streamlit/PROJECT_SUMMARY.md
| import streamlit as st | ||
| from datetime import datetime | ||
| from utils import ProjectManager | ||
| from config import PROJECTS_DIR, GENERATION_MODES |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "GENERATION_MODES" acestep/ui/ace_studio_streamlit/components/dashboard.pyRepository: ace-step/ACE-Step-1.5
Length of output: 116
Remove unused import GENERATION_MODES from line 7.
The import is present but never referenced in the file. Change the import to:
from config import PROJECTS_DIR🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/components/dashboard.py` at line 7, Remove
the unused symbol GENERATION_MODES from the import statement: update the import
that currently reads "from config import PROJECTS_DIR, GENERATION_MODES" to only
import PROJECTS_DIR (i.e., "from config import PROJECTS_DIR") in the
ace_studio_streamlit.components.dashboard module and rerun linters/tests to
confirm no other references to GENERATION_MODES exist.
| st.caption(f"⏱️ {project['duration']}s @ {project.get('bpm', '?')} BPM") | ||
|
|
||
| # Modified time | ||
| modified = datetime.fromisoformat(project['modified_at']) |
There was a problem hiding this comment.
datetime.fromisoformat will raise ValueError for any malformed modified_at string.
A single project with corrupt metadata will crash the entire dashboard render. Wrap in a try/except with a fallback.
🔧 Proposed fix
- modified = datetime.fromisoformat(project['modified_at'])
- st.caption(f"📅 {modified.strftime('%b %d, %H:%M')}")
+ try:
+ modified = datetime.fromisoformat(project['modified_at'])
+ st.caption(f"📅 {modified.strftime('%b %d, %H:%M')}")
+ except (ValueError, KeyError):
+ st.caption("📅 Unknown date")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/components/dashboard.py` at line 64, Wrap the
datetime.fromisoformat(project['modified_at']) call in a try/except inside the
dashboard rendering code so a malformed modified_at doesn't crash the whole
dashboard; catch ValueError (and optionally TypeError) around the call that sets
modified, and on exception set a safe fallback (e.g., None or datetime.min) or
skip parsing for that project, and ensure downstream code that uses the modified
variable can handle the fallback. Use the exact symbol modified and the
expression datetime.fromisoformat(project['modified_at']) to locate and change
the code.
| total_duration = sum(p.get('duration', 0) for p in projects) | ||
| st.metric("Total Duration", f"{total_duration // 60}m" if total_duration else "0m") |
There was a problem hiding this comment.
sum() with a potential None duration will raise TypeError.
p.get('duration', 0) returns None when the key is present but its value is None (e.g. a project saved without an explicit duration). Use p.get('duration') or 0 instead.
🔧 Proposed fix
- total_duration = sum(p.get('duration', 0) for p in projects)
+ total_duration = sum(p.get('duration') or 0 for p in projects)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/components/dashboard.py` around lines 97 -
98, The current computation of total_duration uses p.get('duration', 0) which
can return None if the 'duration' key exists but is None; update the generator
in the total_duration calculation (the expression that iterates over projects in
dashboard.py, referencing total_duration and projects) to coerce None to zero
(use p.get('duration') or 0) so sum() only sees numbers, then keep the
st.metric("Total Duration", ...) output unchanged.
| ) | ||
|
|
||
|
|
||
| def cover_ui(audio_path: Path, duration_sec: float) -> None: |
There was a problem hiding this comment.
duration_sec parameter is unused in cover_ui.
cover_ui accepts duration_sec but never references it (unlike repaint_ui and complete_ui which pass it to region_selector). If the parameter is kept for a consistent signature across all three task UIs, document that intent; otherwise remove it to avoid confusion.
Proposed fix (if not needed for signature consistency)
-def cover_ui(audio_path: Path, duration_sec: float) -> None:
+def cover_ui(audio_path: Path) -> None:If keeping for interface parity, add a brief note:
def cover_ui(audio_path: Path, duration_sec: float) -> None:
- """Create a cover or restyle from source audio."""
+ """Create a cover or restyle from source audio.
+
+ Args:
+ audio_path: Source audio file.
+ duration_sec: Unused; kept for interface parity with other task UIs.
+ """📝 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.
| def cover_ui(audio_path: Path, duration_sec: float) -> None: | |
| def cover_ui(audio_path: Path) -> None: |
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 68-68: Unused function argument: duration_sec
(ARG001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/components/editor_tasks.py` at line 68, The
cover_ui function currently declares duration_sec but never uses it; to preserve
signature parity with repaint_ui and complete_ui, rename the parameter to
_duration_sec (or prefix it with an underscore) and add a one-line docstring in
cover_ui stating the parameter is intentionally unused and kept for consistent
task UI signatures; this avoids linter warnings and documents intent without
changing callers that pass duration_sec to region_selector in the other task
UIs.
| if sf is not None: | ||
| try: | ||
| y, _ = sf.read(str(audio_path), always_2d=True) | ||
| return y.mean(axis=1) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
sf.read fallback loads the full audio array at native sample rate — potential OOM for long files.
The librosa path resamples to 8 000 Hz (target_sr=8000), but the soundfile fallback reads all samples at the original rate (44 100 Hz stereo → ~52 M samples for a 10-minute track). This inconsistency can cause memory pressure and slow renders for longer files.
🔧 Proposed fix
if sf is not None:
try:
y, sr = sf.read(str(audio_path), always_2d=True)
- return y.mean(axis=1)
+ mono = y.mean(axis=1)
+ # Downsample to match librosa path
+ step = max(1, len(mono) // (target_sr * int(sf.info(str(audio_path)).duration + 1)))
+ return mono[::step]
except Exception:
passOr, more robustly, resample via scipy.signal.resample to target_sr. Either way, bring the two paths to parity.
🧰 Tools
🪛 Ruff (0.15.2)
[error] 119-120: try-except-pass detected, consider logging the exception
(S110)
[warning] 119-119: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/components/editor_waveform.py` around lines
115 - 120, The soundfile fallback in the function that uses sf.read(audio_path)
reads the full file at native sample rate which can OOM; change the fallback to
load a manageable chunk or resample to the same target_sr used by the librosa
path (target_sr) — after reading y and original sample rate (from sf.info or
sf.read's return), convert stereo with the same y.mean(axis=1) logic and then
resample the 1-D signal to target_sr (e.g., using scipy.signal.resample) before
returning so both code paths produce the same downsampled waveform and avoid
huge memory usage.
| #!/bin/bash | ||
| # Quick start script for ACE Studio Streamlit UI | ||
|
|
||
| set -e | ||
|
|
||
| echo "🎹 ACE Studio - Quick Start" | ||
| echo "==================================" | ||
|
|
||
| # Check Python | ||
| echo "Checking Python..." | ||
| python --version | ||
|
|
||
| # Check if venv exists | ||
| if [ ! -d "../../../.venv" ]; then | ||
| echo "Creating virtual environment..." | ||
| python -m venv ../../../.venv | ||
| fi | ||
|
|
||
| # Activate venv | ||
| echo "Activating virtual environment..." | ||
| source ../../../.venv/bin/activate | ||
|
|
||
| # Install dependencies | ||
| echo "Installing Streamlit dependencies..." | ||
| pip install -q -r requirements.txt | ||
|
|
||
| # Run the app | ||
| echo "" | ||
| echo "==================================" | ||
| echo "✅ Setup complete!" | ||
| echo "🚀 Starting ACE Studio..." | ||
| echo "📱 Open: http://localhost:8501" | ||
| echo "==================================" | ||
| echo "" | ||
|
|
||
| streamlit run main.py |
There was a problem hiding this comment.
Script will silently break when run from any directory other than its own; Windows venv activation unsupported.
Three compounding issues:
- No
cd "$(dirname "$0")"— all relative paths (../../../.venv,requirements.txt,main.py) are resolved against the caller's CWD, not the script's location. source ../../../.venv/bin/activatefails on Windows (path separator, and should beScripts/activate.batorActivate.ps1). The project targets Windows per the repository guidelines.pythonmay not exist on many Linux distros; usepython3or detect explicitly.
🔧 Proposed fix
#!/bin/bash
# Quick start script for ACE Studio Streamlit UI
+# Always run from the directory containing this script
+cd "$(dirname "$0")" || exit 1
+
set -e
echo "🎹 ACE Studio - Quick Start"
echo "=================================="
# Check Python
echo "Checking Python..."
-python --version
+python3 --version
# Check if venv exists
if [ ! -d "../../../.venv" ]; then
echo "Creating virtual environment..."
- python -m venv ../../../.venv
+ python3 -m venv ../../../.venv
fi
# Activate venv
echo "Activating virtual environment..."
source ../../../.venv/bin/activate📝 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.
| #!/bin/bash | |
| # Quick start script for ACE Studio Streamlit UI | |
| set -e | |
| echo "🎹 ACE Studio - Quick Start" | |
| echo "==================================" | |
| # Check Python | |
| echo "Checking Python..." | |
| python --version | |
| # Check if venv exists | |
| if [ ! -d "../../../.venv" ]; then | |
| echo "Creating virtual environment..." | |
| python -m venv ../../../.venv | |
| fi | |
| # Activate venv | |
| echo "Activating virtual environment..." | |
| source ../../../.venv/bin/activate | |
| # Install dependencies | |
| echo "Installing Streamlit dependencies..." | |
| pip install -q -r requirements.txt | |
| # Run the app | |
| echo "" | |
| echo "==================================" | |
| echo "✅ Setup complete!" | |
| echo "🚀 Starting ACE Studio..." | |
| echo "📱 Open: http://localhost:8501" | |
| echo "==================================" | |
| echo "" | |
| streamlit run main.py | |
| #!/bin/bash | |
| # Quick start script for ACE Studio Streamlit UI | |
| # Always run from the directory containing this script | |
| cd "$(dirname "$0")" || exit 1 | |
| set -e | |
| echo "🎹 ACE Studio - Quick Start" | |
| echo "==================================" | |
| # Check Python | |
| echo "Checking Python..." | |
| python3 --version | |
| # Check if venv exists | |
| if [ ! -d "../../../.venv" ]; then | |
| echo "Creating virtual environment..." | |
| python3 -m venv ../../../.venv | |
| fi | |
| # Activate venv | |
| echo "Activating virtual environment..." | |
| source ../../../.venv/bin/activate | |
| # Run the app | |
| echo "" | |
| echo "==================================" | |
| echo "✅ Setup complete!" | |
| echo "🚀 Starting ACE Studio..." | |
| echo "📱 Open: http://localhost:8501" | |
| echo "==================================" | |
| echo "" | |
| streamlit run main.py |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/run.sh` around lines 1 - 36, The script uses
relative paths and a hardcoded `python` and Unix-only venv activation, so update
run.sh to first change to the script's directory with cd "$(dirname "$0")" (or a
portable dirname resolution) so ../../../.venv, requirements.txt and main.py are
resolved correctly; detect which Python to use by checking for `python3` then
`python` (set a python_cmd variable) and use that for venv creation; compute a
venv_dir variable pointing to the resolved .venv path and create it if missing;
and make activation platform-aware in the activation block (for Unix: source
"$venv_dir/bin/activate", for Windows Git Bash/Cygwin: source
"$venv_dir/Scripts/activate", and for native Windows prompt/PowerShell provide
instructions or use conditional checks for WIN32/PowerShell to run
Scripts/Activate.ps1 or Scripts/activate.bat) so venv activation works on
Windows and Unix.
| from .audio_utils import ( | ||
| save_audio_file, | ||
| load_audio_file, | ||
| get_audio_duration, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find acestep/ui/ace_studio_streamlit -type f -name "*.py" | head -20Repository: ace-step/ACE-Step-1.5
Length of output: 1010
🏁 Script executed:
cat acestep/ui/ace_studio_streamlit/utils/__init__.pyRepository: ace-step/ACE-Step-1.5
Length of output: 610
🏁 Script executed:
cat acestep/ui/ace_studio_streamlit/utils/audio_utils.pyRepository: ace-step/ACE-Step-1.5
Length of output: 2703
🏁 Script executed:
find . -maxdepth 2 -type f \( -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" -o -name "setup.cfg" \)Repository: ace-step/ACE-Step-1.5
Length of output: 205
🏁 Script executed:
cat pyproject.toml | head -100Repository: ace-step/ACE-Step-1.5
Length of output: 4061
🏁 Script executed:
# Check if soundfile is mentioned in dependencies
rg "soundfile|librosa" pyproject.tomlRepository: ace-step/ACE-Step-1.5
Length of output: 89
🏁 Script executed:
# Check if librosa is mentioned anywhere in dependencies
rg "librosa" pyproject.toml requirements*.txtRepository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
# Check for all usages of librosa.output.write_wav and librosa.get_duration across the codebase
rg -n "output\.write_wav|get_duration" --type=py acestep/Repository: ace-step/ACE-Step-1.5
Length of output: 5870
🏁 Script executed:
# Check the specific usages in audio_player.py and editor_waveform.py
cat -n acestep/ui/ace_studio_streamlit/components/audio_player.py | sed -n '35,50p'Repository: ace-step/ACE-Step-1.5
Length of output: 672
🏁 Script executed:
cat -n acestep/ui/ace_studio_streamlit/components/editor_waveform.py | sed -n '85,105p'Repository: ace-step/ACE-Step-1.5
Length of output: 810
🏁 Script executed:
# Verify librosa is actually imported in these files
rg -n "import librosa" --type=py acestep/ui/ace_studio_streamlit/Repository: ace-step/ACE-Step-1.5
Length of output: 312
🏁 Script executed:
# Check the full context of audio_player.py to see the import pattern
cat -n acestep/ui/ace_studio_streamlit/components/audio_player.py | head -50Repository: ace-step/ACE-Step-1.5
Length of output: 1783
🏁 Script executed:
# Verify that librosa is indeed a transitive dependency (not explicitly listed)
# by checking if it's imported elsewhere and what pulls it in
rg -l "librosa" --type=py acestep/ | wc -lRepository: ace-step/ACE-Step-1.5
Length of output: 66
🏁 Script executed:
# Check if librosa is a transitive dependency (what depends on it)
rg "librosa" --type=py acestep/ --max-count=1 -l | xargs -I {} bash -c "echo '=== {} ==='; head -20 {}"Repository: ace-step/ACE-Step-1.5
Length of output: 1685
🏁 Script executed:
# Get more context on editor_waveform.py to understand why it uses path= correctly
cat -n acestep/ui/ace_studio_streamlit/components/editor_waveform.py | head -40Repository: ace-step/ACE-Step-1.5
Length of output: 1258
Replace librosa.output.write_wav with soundfile.write in audio_utils.py and update deprecated parameter names in two locations.
audio_utils.py contains two deprecated librosa calls:
- Line 34:
librosa.output.write_wav()was removed in librosa 0.8.0. Replace withsoundfile.write():
Fix for save_audio_file()
+import soundfile as sf
...
def save_audio_file(audio_data: np.ndarray, file_path: str, sr: int = 16000) -> None:
"""Save audio data to file"""
- if not librosa:
- raise ImportError("librosa is required for audio saving")
try:
- librosa.output.write_wav(file_path, audio_data, sr=sr)
+ sf.write(file_path, audio_data, sr)
logger.info(f"Saved audio to {file_path}")
except Exception as e:
logger.error(f"Failed to save audio to {file_path}: {e}")
raise- Line 47 in
get_audio_duration(): Update deprecatedfilename=parameter topath=:
- duration = librosa.get_duration(filename=file_path)
+ duration = librosa.get_duration(path=file_path)Also fix audio_player.py line 41, which has the same deprecated parameter:
- duration = librosa.get_duration(filename=audio_path)
+ duration = librosa.get_duration(path=audio_path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/utils/__init__.py` around lines 11 - 15, The
code uses deprecated librosa APIs; in audio_utils.py replace
librosa.output.write_wav(...) inside save_audio_file with soundfile.write(...)
(import soundfile as sf) ensuring you pass the audio array, path, and samplerate
in the correct order and dtype if needed; in get_audio_duration() change the
deprecated filename= keyword to path= when calling librosa.get_duration (or
related call); and in audio_player.py update the similar call on line 41 to use
path= instead of filename=. Refer to the functions save_audio_file and
get_audio_duration and the audio_player call to locate and update these spots.
| def get_audio_duration(file_path: str) -> float: | ||
| """Get audio duration in seconds""" | ||
| if not librosa: | ||
| raise ImportError("librosa is required for audio analysis") | ||
|
|
||
| try: | ||
| duration = librosa.get_duration(filename=file_path) | ||
| return duration | ||
| except Exception as e: | ||
| logger.error(f"Failed to get duration of {file_path}: {e}") | ||
| return 0.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the target file
fd -t f audio_utils.py | head -5Repository: ace-step/ACE-Step-1.5
Length of output: 140
🏁 Script executed:
# Check for requirements files
fd -t f -e txt -e txt -e in requirements | head -10Repository: ace-step/ACE-Step-1.5
Length of output: 227
🏁 Script executed:
# Get the actual content of the audio_utils.py file
find . -name audio_utils.py -path "*/ace_studio_streamlit/utils/*" -exec cat -n {} \;Repository: ace-step/ACE-Step-1.5
Length of output: 3312
🏁 Script executed:
# Check main requirements.txt for librosa version
cat requirements.txt | grep -i librosaRepository: ace-step/ACE-Step-1.5
Length of output: 47
🏁 Script executed:
# Check ui-specific requirements
cat acestep/ui/ace_studio_streamlit/requirements.txt | grep -i librosaRepository: ace-step/ACE-Step-1.5
Length of output: 80
🏁 Script executed:
# Check all requirements for librosa references
grep -r "librosa" requirements*.txt 2>/dev/nullRepository: ace-step/ACE-Step-1.5
Length of output: 47
🌐 Web query:
librosa get_duration path parameter filename deprecated 0.10 changelog
💡 Result:
In librosa 0.10.0, librosa.get_duration renamed its file argument keyword from filename to path. The old filename= keyword remains as a deprecated alias (it’s internally remapped to path) and is planned to be removed in librosa 1.0. Use librosa.get_duration(path="file.wav") going forward. [1], [2]
Where this is documented
- Changelog entry (PR #1625): “
librosa.get_durationparameterfilenamehas been renamed topath.” [1] - API/source docs:
filename : Deprecated … renamed to path in 0.10 … will be removed in 1.0.[2]
Sources
- librosa changelog (v0.10 / PR
#1625) — https://librosa.org/doc/latest/changelog.html [1] - librosa
get_durationdocs/source showingfilenamedeprecated →path(0.10.0 deprecation, removed in 1.0) — https://librosa.org/doc/0.11.0/_modules/librosa/core/audio.html [2]
Update librosa.get_duration() parameter from deprecated filename to path.
The filename parameter was renamed to path in librosa 0.10.0. While it still works as a deprecated alias in the current pinned version (0.10.2), it will be removed in librosa 1.0. Update to path now to avoid breakage.
Proposed fix
try:
- duration = librosa.get_duration(filename=file_path)
+ duration = librosa.get_duration(path=file_path)
return duration🧰 Tools
🪛 Ruff (0.15.2)
[warning] 44-44: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 48-48: Consider moving this statement to an else block
(TRY300)
[warning] 49-49: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/utils/audio_utils.py` around lines 41 - 51,
The get_audio_duration function uses the deprecated
librosa.get_duration(filename=...), so update the call in get_audio_duration to
use the new parameter name path (i.e., librosa.get_duration(path=file_path)) to
avoid future breakage; leave the ImportError check and exception logging
(logger.error in the except block) intact so behavior and error reporting remain
the same.
| def normalize_audio(audio_data: np.ndarray, target_db: float = -1.0) -> np.ndarray: | ||
| """Normalize audio to target loudness (dB)""" | ||
| try: | ||
| # Calculate current RMS level | ||
| rms = np.sqrt(np.mean(audio_data ** 2)) | ||
| if rms == 0: | ||
| return audio_data | ||
|
|
||
| # Convert target dB to linear scale | ||
| target_linear = 10 ** (target_db / 20.0) | ||
|
|
||
| # Scale audio | ||
| normalized = audio_data * (target_linear / rms) | ||
|
|
||
| # Prevent clipping | ||
| max_val = np.max(np.abs(normalized)) | ||
| if max_val > 1.0: | ||
| normalized = normalized / max_val | ||
|
|
||
| return normalized | ||
| except Exception as e: | ||
| logger.error(f"Failed to normalize audio: {e}") | ||
| return audio_data |
There was a problem hiding this comment.
Exact float comparison rms == 0 may miss near-silent audio.
np.mean(audio_data ** 2) can produce a tiny non-zero value for near-silent audio, causing an enormous scale factor in line 66. Consider using a small threshold instead.
Proposed fix
- if rms == 0:
+ if rms < 1e-10:
return audio_data🧰 Tools
🪛 Ruff (0.15.2)
[warning] 73-73: Consider moving this statement to an else block
(TRY300)
[warning] 74-74: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/utils/audio_utils.py` around lines 54 - 76,
The normalize_audio function uses exact float comparison (rms == 0) which fails
for near-silent inputs; change this to compare against a small epsilon (e.g.,
rms < 1e-8) to detect silence and avoid producing a huge scale factor. In
normalize_audio, compute rms as before but if rms < epsilon either return the
original audio_data (or a safe zeroed array) instead of scaling, and use the
epsilon check wherever rms is used for division/scaling to prevent extreme
amplification or division-by-nearly-zero.
|
|
||
| checkpoint_dir = str(_project_root / "checkpoints") | ||
|
|
There was a problem hiding this comment.
Hardcoded "checkpoints" path — use CHECKPOINTS_DIR from config.
Line 121 builds the checkpoint directory by joining _project_root / "checkpoints", but the config module already defines CHECKPOINTS_DIR for this purpose. Using the constant ensures consistency if the path is ever changed.
Proposed fix
+from config import CHECKPOINTS_DIR
...
- checkpoint_dir = str(_project_root / "checkpoints")
+ checkpoint_dir = str(CHECKPOINTS_DIR)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/ace_studio_streamlit/utils/cache.py` around lines 120 - 122, The
code currently builds checkpoint_dir as str(_project_root / "checkpoints") which
hardcodes the folder name; change it to use the CHECKPOINTS_DIR constant from
the config module (e.g., str(_project_root / CHECKPOINTS_DIR) or
config.CHECKPOINTS_DIR) and ensure CHECKPOINTS_DIR is imported or referenced
where checkpoint_dir is defined so the path stays consistent with the app
config; keep the variable name checkpoint_dir and _project_root usage unchanged.
|
only use streamlit as your folder name please donot use ace_studio as prefix |
- acestep/ui/ace_studio_streamlit/ -> acestep/ui/streamlit/ - Updated all doc references to use new path
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (30)
acestep/ui/streamlit/run.bat (1)
1-1:⚠️ Potential issue | 🟠 MajorUse CRLF line endings for this
.batfile before merge.Line 1 is currently in an LF-only file per static analysis; batch parsing can be unreliable with Unix endings in some Windows environments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/run.bat` at line 1, The run.bat file uses LF-only line endings which can break batch parsing on Windows; convert run.bat to use CRLF line endings (Windows-style) before merging. Open the file named run.bat (the batch entrypoint), change its line ending setting to CRLF (e.g., via your editor's EOL menu or git config/core.eol and .gitattributes), save the file, and verify the first line now ends with CRLF so the batch script runs reliably on Windows.acestep/ui/streamlit/INSTALL.md (3)
99-114:⚠️ Potential issue | 🟠 MajorDocker instructions likely miss the parent
acesteppackage.The snippet builds from this subdirectory and copies only local files; if the app imports from
acestep.*, the container can fail withModuleNotFoundError.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/INSTALL.md` around lines 99 - 114, The Dockerfile currently copies only the subdirectory (COPY . .) and may omit the top-level aceStep package, causing ModuleNotFoundError when importing aceStep.*; fix by ensuring the container includes the repository root or installs the package: either build the image from the project root so the aceStep package is copied into /app, or modify the Dockerfile to copy the repo root (or add the parent package files) into the image and run a package install (e.g., pip install . or pip install -e .) after copying requirements.txt so imports like aceStep.* resolve at runtime; update the CMD only if you change workdir.
65-69:⚠️ Potential issue | 🟠 MajorCorrect the ROCm environment variable example.
The example uses a colon-formatted value with
PYTORCH_HIP_ALLOC_CONF; that format is typically used for hipBLAS workspace config, not allocator key/value options.What is the official PyTorch ROCm documentation for the expected value format of PYTORCH_HIP_ALLOC_CONF (or PYTORCH_ALLOC_CONF) vs HIPBLAS_WORKSPACE_CONFIG?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/INSTALL.md` around lines 65 - 69, Update the ROCm instructions to use the correct allocator config variable and format: replace the colon-formatted PYTORCH_HIP_ALLOC_CONF example with the proper key=value JSON-like or comma-separated allocator options using PYTORCH_ALLOC_CONF (or show both variables and their intended purposes), and clarify that HIPBLAS_WORKSPACE_CONFIG (or HIPBLAS_WORKSPACE) uses the colon-style format for rocBLAS/hipBLAS workspace only; specifically mention the correct variable names PYTORCH_ALLOC_CONF, PYTORCH_HIP_ALLOC_CONF, and HIPBLAS_WORKSPACE_CONFIG so readers know which format applies to each and adjust the example value accordingly.
46-46:⚠️ Potential issue | 🟡 MinorFix contradictory minimum requirement wording.
Line 46 says
4GB VRAM (CPU only), which mixes GPU memory terminology with CPU-only mode.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/INSTALL.md` at line 46, The line "4GB VRAM (CPU only)" mixes GPU and CPU terminology; update the INSTALL.md entry to be unambiguous by either (A) changing it to "4GB RAM (CPU-only)" if you mean system memory for CPU mode, or (B) if you intend a GPU minimum, change it to "4GB VRAM (with GPU)"; locate the exact string "4GB VRAM (CPU only)" and replace it accordingly so the requirements clearly distinguish CPU RAM vs GPU VRAM.acestep/ui/streamlit/README.md (2)
181-185:⚠️ Potential issue | 🟡 MinorFix broken relative links to repository-level docs.
../CONTRIBUTING.mdand../LICENSEresolve toacestep/ui/, not repo root, from this README location.Suggested patch
-Contributions welcome! See [ACE-Step CONTRIBUTING.md](../CONTRIBUTING.md) +Contributions welcome! See [ACE-Step CONTRIBUTING.md](../../../CONTRIBUTING.md) @@ -Same as ACE-Step - see [LICENSE](../LICENSE) +Same as ACE-Step - see [LICENSE](../../../LICENSE)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/README.md` around lines 181 - 185, The links in acestep/ui/streamlit/README.md currently point to ../CONTRIBUTING.md and ../LICENSE which resolve to acestep/ui/; update those references to the correct repo-root paths (e.g., ../../CONTRIBUTING.md and ../../LICENSE should both be prefixed with ../.. so use ../../CONTRIBUTING.md and ../../LICENSE) so the CONTRIBUTING and LICENSE links resolve to the repository root.
109-113:⚠️ Potential issue | 🟠 MajorRemove or implement the documented keyboard shortcuts.
Lines 109–113 document
R/S/Dshortcuts; please align docs with actual implemented behavior.#!/bin/bash # Check whether keyboard shortcuts are implemented in streamlit UI code fd -t f --extension py | xargs rg -n -C2 "shortcut|hotkey|keyboard|keydown|keypress|st\\.session_state\\[\"tab\"\\]|sidebar\\.radio"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/README.md` around lines 109 - 113, The README lists keyboard shortcuts R/S/D that are not present in the UI; either remove those entries or implement them: if removing, update the README to delete the three bullets; if implementing, add a keyboard handler (via a small Streamlit component or JS injection) that listens for keydown of 'R', 'S', 'D' and maps them to the existing UI actions (call refresh_current_tab(), open_settings(), go_to_dashboard() or set st.session_state["tab"] consistent with the sidebar.radio control), and then update the README to note the implemented behavior; ensure the implementation references the same state keys (st.session_state["tab"]) and UI control names so documentation and code stay aligned.acestep/ui/streamlit/components/editor_audio_picker.py (2)
100-101:⚠️ Potential issue | 🔴 CriticalSanitize upload persistence to avoid overwrite/path issues.
Using
Path(tempfile.gettempdir()) / uploaded.nameat Lines 100–101 trusts user-supplied names and creates deterministic temp paths.Suggested patch
- tmp = Path(tempfile.gettempdir()) / uploaded.name - tmp.write_bytes(uploaded.getvalue()) - return tmp + safe_name = Path(uploaded.name).name + suffix = Path(safe_name).suffix + with tempfile.NamedTemporaryFile(delete=False, suffix=suffix) as tmp: + tmp.write(uploaded.getvalue()) + return Path(tmp.name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/editor_audio_picker.py` around lines 100 - 101, The code writes uploaded bytes to a predictable path using Path(tempfile.gettempdir()) / uploaded.name which trusts uploaded.name and can cause overwrites or path traversal; fix this in editor_audio_picker.py by deriving a safe filename (e.g., strip directories via os.path.basename or a secure_filename equivalent) and create a unique temp file using tempfile.NamedTemporaryFile or tempfile.mkstemp instead of concatenating uploaded.name; replace the tmp assignment and subsequent tmp.write_bytes(uploaded.getvalue()) with logic that safely generates a unique temp path and writes the bytes to it, referencing the uploaded.name and tmp.write_bytes usages to locate the change.
22-37:⚠️ Potential issue | 🟠 MajorTab execution order can overwrite the user’s chosen audio source.
Because all tab bodies run on rerun, Lines 28–35 can reassign
audio_pathmultiple times; final value depends on execution order, not explicit source choice.Suggested patch
- tab_proj, tab_out, tab_upload = st.tabs( - ["📁 Projects", "📂 All outputs", "⬆️ Upload file"] - ) - - audio_path: Optional[Path] = None - - with tab_proj: - audio_path = _pick_from_projects(audio_path) - - with tab_out: - audio_path = _pick_from_outputs(audio_path) - - with tab_upload: - audio_path = _pick_from_upload(audio_path) - - return audio_path + source = st.radio( + "Audio source", + ["📁 Projects", "📂 All outputs", "⬆️ Upload file"], + horizontal=True, + key="ed_source", + ) + if source == "📁 Projects": + return _pick_from_projects(None) + if source == "📂 All outputs": + return _pick_from_outputs(None) + return _pick_from_upload(None)In Streamlit, does code inside non-selected st.tabs execute on each rerun, and is all tab content computed regardless of active tab?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/editor_audio_picker.py` around lines 22 - 37, The current implementation calls _pick_from_projects, _pick_from_outputs, and _pick_from_upload inside each st.tabs context so all three run on every rerun and later ones overwrite audio_path; replace the tabs-driven execution with a single explicit selector (e.g., st.radio or st.selectbox "audio_source") and then call only the corresponding picker based on that selection (call _pick_from_projects, _pick_from_outputs, or _pick_from_upload) to set audio_path so only the chosen source executes and cannot be overwritten by other tab bodies.acestep/ui/streamlit/run.sh (1)
4-25:⚠️ Potential issue | 🟠 MajorHarden startup: normalize CWD and resolve Python executable explicitly.
Lines 4–25 can fail when called outside this folder and on systems where only
python3is installed.Suggested patch
set -e +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "$SCRIPT_DIR" + +if command -v python3 >/dev/null 2>&1; then + PYTHON_CMD="python3" +elif command -v python >/dev/null 2>&1; then + PYTHON_CMD="python" +else + echo "Python not found (tried python3 and python)." + exit 1 +fi echo "🎹 ACE Studio - Quick Start" @@ echo "Checking Python..." -python --version +"$PYTHON_CMD" --version @@ if [ ! -d "../../../.venv" ]; then echo "Creating virtual environment..." - python -m venv ../../../.venv + "$PYTHON_CMD" -m venv ../../../.venv fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/run.sh` around lines 4 - 25, Normalize the working directory at the top of run.sh (use dirname/ BASH_SOURCE to cd to the script's directory) and resolve the Python executable before use (prefer command -v python || command -v python3), then replace direct calls to python --version and python -m venv with the resolved PYTHON variable; also use that same PYTHON to create the venv (instead of python -m venv) and keep sourcing ../../../.venv/bin/activate and pip install -q -r requirements.txt as before so the script works when invoked from other CWDs and on systems where only python3 exists.acestep/ui/streamlit/QUICKSTART.md (1)
73-75:⚠️ Potential issue | 🟠 MajorAvoid claiming parallel batch execution unless it is actually wired.
Lines 73–75 state parallel processing; please align this with the real behavior (or implement actual parallel execution).
#!/bin/bash # Verify whether batch generation uses parallel execution primitives fd -t f "batch_generator.py" | xargs -I{} sh -c ' echo "== {} =="; rg -n -C2 "parallel_count|Process in Parallel|generate_batch|Thread|asyncio|multiprocess|concurrent|executor" "{}" '🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/QUICKSTART.md` around lines 73 - 75, The doc currently claims "Batch generation with parallel processing" (alongside bullets "Queue up to 8 songs" and "See progress for each song") but the codebase doesn't show parallel primitives; either remove or correct that bullet or implement real parallelism. Fix options: (A) Edit QUICKSTART.md to remove/soften "parallel processing" and instead state the actual behavior (e.g., sequential batches) so the bullets "Queue up to 8 songs" and "See progress for each song" remain accurate, or (B) implement parallel batch execution in the batch generator by adding concurrency to the function(s) referenced by generate_batch / batch_generator.py (use ThreadPoolExecutor or asyncio with gather, or multiprocessing.Pool depending on CPU/IO workload), ensure progress reporting per-song is thread-safe, and update README to reflect the new parallel implementation. Verify by searching for generate_batch, batch_generator.py, and any Thread/asyncio/Process/executor usage and update QUICKSTART.md accordingly.acestep/ui/streamlit/components/editor.py (1)
28-37:⚠️ Potential issue | 🟠 MajorDiT-not-ready warning should return early to prevent rendering controls that depend on a loaded model.
The warning is displayed but execution continues, allowing the user to pick audio and trigger a task that will fail downstream in
run_edit_task. Addreturnafter the warning.🔧 Proposed fix
if not is_dit_ready(): st.warning( "DiT model is **not loaded**. " "Load it in **⚙️ Settings → Models** first." ) + return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/editor.py` around lines 28 - 37, The warning shown when DiT isn't ready must short-circuit UI rendering to avoid downstream failures: in the block that checks is_dit_ready(), after calling st.warning(...) immediately return to stop execution so controls like pick_audio_source() and subsequent calls (e.g., run_edit_task) are not rendered or invoked; update the is_dit_ready() check in editor.py to return early when the model is not loaded.acestep/ui/streamlit/components/dashboard.py (3)
96-98:⚠️ Potential issue | 🟡 Minor
p.get('duration', 0)returnsNonewhen the key exists with aNonevalue, breakingsum().Use
p.get('duration') or 0to coerceNoneto zero.- total_duration = sum(p.get('duration', 0) for p in projects) + total_duration = sum(p.get('duration') or 0 for p in projects)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/dashboard.py` around lines 96 - 98, The sum calculation for total_duration uses p.get('duration', 0) which returns None when 'duration' exists but is None, causing sum() to fail; update the generator to coerce None to zero by using p.get('duration') or 0 (affecting the expression that assigns total_duration from projects), then keep the st.metric("Total Duration", ...) usage unchanged so the displayed value remains correct.
7-7:⚠️ Potential issue | 🟡 MinorRemove unused import
GENERATION_MODES.
GENERATION_MODESis imported but never referenced in this file.-from config import PROJECTS_DIR, GENERATION_MODES +from config import PROJECTS_DIR🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/dashboard.py` at line 7, Remove the unused import by editing the import statement that currently reads "from config import PROJECTS_DIR, GENERATION_MODES": drop "GENERATION_MODES" so only the referenced symbol PROJECTS_DIR is imported; this removes the unused symbol warning and keeps the rest of dashboard.py unchanged.
63-65:⚠️ Potential issue | 🟡 Minor
datetime.fromisoformatwill crash the dashboard if any project has a malformedmodified_at.A single corrupt metadata file takes down the entire dashboard render. Wrap in try/except with a fallback.
🔧 Proposed fix
- modified = datetime.fromisoformat(project['modified_at']) - st.caption(f"📅 {modified.strftime('%b %d, %H:%M')}") + try: + modified = datetime.fromisoformat(project['modified_at']) + st.caption(f"📅 {modified.strftime('%b %d, %H:%M')}") + except (ValueError, KeyError): + st.caption("📅 Unknown date")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/dashboard.py` around lines 63 - 65, Wrap the datetime.fromisoformat call that parses project['modified_at'] in a try/except inside the render flow (where modified is assigned and st.caption is called) to prevent a malformed timestamp from crashing the dashboard; on exception, set a safe fallback (e.g., None or a placeholder string like "Unknown date") and render st.caption with that fallback instead of using modified.strftime — update the block that defines modified and calls st.caption to handle parsing errors gracefully.acestep/ui/streamlit/main.py (1)
130-138:⚠️ Potential issue | 🟠 MajorBare
except Exception: passsilently swallows errors — useloguruto log the failure.This was flagged by Ruff (S110, BLE001) and in a previous review. A failing
list_projects()will be invisible to developers debugging issues.🔧 Proposed fix
+from loguru import logger ... try: from utils import ProjectManager from config import PROJECTS_DIR pm = ProjectManager(PROJECTS_DIR) projects = pm.list_projects() st.metric("💾 Projects", len(projects)) - except Exception: - pass + except Exception as exc: + logger.warning(f"Failed to load project count: {exc}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/main.py` around lines 130 - 138, Replace the bare "except Exception: pass" with explicit error handling that logs the failure using loguru: catch Exception as e around the block that imports ProjectManager/PROJECTS_DIR and calls pm = ProjectManager(PROJECTS_DIR); projects = pm.list_projects(); then call logger.exception or logger.error with context (e.g., "Failed to list projects in ProjectManager.list_projects") and still set a fallback for st.metric (e.g., show 0 or "error"); reference ProjectManager, PROJECTS_DIR, list_projects and st.metric so the log clearly ties to this operation.acestep/ui/streamlit/utils/__init__.py (1)
11-15:⚠️ Potential issue | 🔴 Critical
audio_utils.pystill uses deprecatedlibrosa.output.write_wavandlibrosa.get_duration(filename=...).The re-exports here are fine, but the underlying
audio_utils.pycontains two deprecated librosa calls that will break with librosa 1.0:
librosa.output.write_wav()(removed in librosa 0.8.0) → usesoundfile.write()librosa.get_duration(filename=...)(deprecated in 0.10.0) → usepath=These were flagged in a previous review and remain unfixed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/utils/__init__.py` around lines 11 - 15, audio_utils.py still calls deprecated librosa APIs used by save_audio_file and get_audio_duration; replace any use of librosa.output.write_wav(...) with soundfile.write(...) (import soundfile as sf) and pass the audio array and sample rate appropriately, and change librosa.get_duration(filename=...) to librosa.get_duration(path=...) (or call sf.info for duration) so the functions save_audio_file, load_audio_file, and get_audio_duration no longer rely on removed/deprecated signatures; update imports and adjust argument names to match soundfile.write and librosa.get_duration(path=...) accordingly.acestep/ui/streamlit/components/generation_wizard.py (2)
348-354:⚠️ Potential issue | 🟠 Major
genre=captionincorrectly persists the full song description as the genre field.
captionis the freeform text prompt, not the genre. Thequick_genreandquick_moodvalues selected in the UI (Lines 74, 80) are never passed intogenerate_song. Pass them explicitly and use them insave_metadata.🔧 Proposed fix outline
Add
genreandmoodparameters togenerate_song, forwardquick_genre/quick_moodfrom the call site (Line 229), and use them:pm.save_metadata( project_path, - genre=caption, - mood="Generated", + genre=genre, + mood=mood, duration=duration, bpm=bpm, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/generation_wizard.py` around lines 348 - 354, The code mistakenly persists the full freeform prompt (caption) as the genre; update the generate_song function signature to accept genre and mood parameters (e.g., genre, mood), forward the UI-selected quick_genre and quick_mood from the place where generate_song is called, and inside generate_song call pm.save_metadata(project_path, genre=genre, mood=mood, duration=duration, bpm=bpm) (instead of genre=caption and hardcoded mood), ensuring all call sites are updated to pass the new arguments.
1-370: 🛠️ Refactor suggestion | 🟠 MajorModule exceeds the 200 LOC hard cap (~370 lines).
Extract
generate_song(and its helpers) into a dedicated module (e.g.,generation_runner.py), keeping only the UI form ingeneration_wizard.py. As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/generation_wizard.py` around lines 1 - 370, The file is over the 200-LOC cap; move the heavy generation logic out of the UI by extracting the generate_song function (and its helpers/related imports) into a new module (e.g., generation_runner.py). In generation_runner.py implement generate_song with its dependencies: get_dit_handler, get_llm_handler, GenerationParams, GenerationConfig, generate_music, ProjectManager, and OUTPUT_DIR; preserve exception logging via logger and return/use the same behaviors. In generation_wizard.py keep only the UI/form code and replace the local generate_song definition with a single import from generation_runner (from .generation_runner import generate_song) and update any imports (Path/ProjectManager/OUTPUT_DIR references) so the UI remains <=200 LOC. Ensure relative imports and keys (generate_song, ProjectManager, get_dit_handler, get_llm_handler, GenerationParams, GenerationConfig, generate_music) are adjusted so tests and runtime continue to work.acestep/ui/streamlit/components/batch_generator.py (1)
130-170:⚠️ Potential issue | 🟠 Major
generate_batchis a stub that unconditionally reports "✅ Success" without performing generation, and usesget_dit_handler()instead ofis_dit_ready().The TODO on Line 162 confirms generation is not wired. Every queued song shows success without any audio being produced, misleading users. Either implement the actual generation call (via
generate_musicas done ingeneration_wizard.py/editor_runner.py), or mark items as "⚠️ Not implemented" and gate the Generate All button behind a readiness flag.Also, Line 133 checks
get_dit_handler()(handler creation) instead ofis_dit_ready()(model loading), inconsistent with every other component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/batch_generator.py` around lines 130 - 170, generate_batch currently fakes success and checks get_dit_handler() instead of model readiness; replace the stub with the real generation flow: verify readiness with is_dit_ready() and fail early if not ready, obtain the handler (get_dit_handler()) and call the actual generation routine (use generate_music or the same dit_handler.generate(...) pattern used in generation_wizard.py/editor_runner.py), save the produced audio via ProjectManager.create_project and pm.save_audio(project_path, audio, "output.wav"), and update results entries to "✅ Success" only on successful save or to an error state (e.g., "❌ Failed" or "⚠️ Not implemented") on exceptions; alternatively, if you don't implement generation now, explicitly mark entries as "⚠️ Not implemented" and disable the Generate All button when is_dit_ready() is false.acestep/ui/streamlit/components/audio_player.py (1)
39-46:⚠️ Potential issue | 🟠 MajorDeprecated
librosa.get_duration(filename=...)and bareexcept:— usepath=and catch explicitly.
filenamewas deprecated in librosa 0.10.0 (removed in 1.0). Also, bareexcept:violates the coding guideline "Handle errors explicitly; avoid bareexcept."🔧 Proposed fix
try: import librosa - duration = librosa.get_duration(filename=audio_path) + duration = librosa.get_duration(path=audio_path) minutes = int(duration // 60) seconds = int(duration % 60) st.metric("Duration", f"{minutes}m {seconds}s") - except: + except Exception: st.metric("Duration", "Unknown")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/audio_player.py` around lines 39 - 46, The duration lookup uses the removed librosa `filename=` kwarg and a bare except; change the call in this component to use librosa.get_duration(path=audio_path) and replace the bare except with explicit exception handling: catch ImportError separately (when librosa isn't installed) and catch runtime errors from get_duration as Exception (e.g., as e) so you can set the metric to "Unknown" and surface or log the actual error (via st.error or your logger) instead of silently swallowing it; target the code around the librosa import/get_duration block in audio_player.py and leave the st.metric("Duration", ...) behavior intact.acestep/ui/streamlit/utils/audio_utils.py (3)
54-76: Exact float comparisonrms == 0may miss near-silent audio.Already flagged in a prior review. Use a small threshold (e.g.,
rms < 1e-10).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/utils/audio_utils.py` around lines 54 - 76, In normalize_audio, avoid exact float comparison rms == 0 which misses near-silent signals; change the check to use a small threshold (e.g., rms < 1e-10 or rms < np.finfo(float).eps) when testing the rms variable so near-zero energy audio is treated as silent and returned unchanged; update the rms check inside the normalize_audio function and keep the rest of the scaling/clip prevention logic unchanged.
28-38:librosa.output.write_wavwas removed in librosa 0.8.0 — runtime crash.Already flagged in a prior review. Replace with
soundfile.write.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/utils/audio_utils.py` around lines 28 - 38, The save_audio_file function currently calls the removed librosa.output.write_wav and will crash; replace that call with soundfile.write (e.g., import soundfile as sf) inside save_audio_file (and keep existing error handling/logging), passing file_path, audio_data, and samplerate=sr; ensure soundfile is imported/installed or raise a clear ImportError if missing and remove the librosa.output.write_wav reference.
41-51: Deprecatedfilenameparameter inlibrosa.get_duration()— usepath.Already flagged in a prior review.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/utils/audio_utils.py` around lines 41 - 51, The get_audio_duration function uses the deprecated librosa.get_duration(filename=...) parameter; update the call in get_audio_duration to use the path parameter (e.g., librosa.get_duration(path=file_path)) so it uses the supported API, keep the existing error handling and ImportError check for librosa, and ensure the variable file_path is passed to the path argument.acestep/ui/streamlit/utils/cache.py (2)
121-121: Hardcoded"checkpoints"path — useCHECKPOINTS_DIRfrom config.Already flagged in a prior review. Use the centralized constant for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/utils/cache.py` at line 121, Replace the hardcoded "checkpoints" string when building checkpoint_dir with the centralized constant CHECKPOINTS_DIR from the config: use _project_root / CHECKPOINTS_DIR instead of _project_root / "checkpoints", ensure CHECKPOINTS_DIR is imported into the module, and update the assignment to checkpoint_dir = str(_project_root / CHECKPOINTS_DIR) so other code uses the shared constant.
106-111: Defaultbackend="mlx"is Apple Silicon–only — will fail on Linux/Windows.Already flagged in a prior review. The default should be platform-aware.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/utils/cache.py` around lines 106 - 111, The default backend for initialize_llm is currently hardcoded to "mlx" which only works on Apple Silicon; change it to select a platform-appropriate default at runtime instead of a fixed parameter value: update initialize_llm to accept backend: str = None (or keep parameter but if backend is "auto" or None) and detect the platform (e.g., using sys.platform / platform.machine or torch.backends) inside initialize_llm to set backend to "mlx" for macOS ARM and a compatible backend (e.g., "gptq"/"vllm"/"cuda" or "cpu") for Linux/Windows, and ensure device defaults are chosen consistently (device "auto" should map to "cpu" vs "cuda" based on availability); adjust any callers or tests that assume the old default.acestep/ui/streamlit/components/editor_tasks.py (1)
68-68:duration_secparameter is unused incover_ui.Already flagged in a prior review. Either remove it or prefix with underscore to signal intentional non-use for interface parity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/editor_tasks.py` at line 68, The cover_ui function declares an unused parameter duration_sec; update the function signature in cover_ui(audio_path: Path, duration_sec: float) to either remove duration_sec or rename it to _duration_sec to indicate intentional non-use (whichever maintains API parity), and adjust any callers or tests to match the new signature to avoid breakage.acestep/ui/streamlit/components/settings_panel.py (2)
133-137: LLM device silently inherits the DiT device selection.
dit_deviceis passed as the LLM device on Line 136, coupling two independent model configurations. Theif "dit_device_select" in st.session_stateguard is also dead code since the DiT selectbox always creates that key earlier in the same function. Add a separate LLM device selector or make the coupling explicit in the label.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/settings_panel.py` around lines 133 - 137, The LLM device is incorrectly tied to the DiT device selection; update the UI and call to initialize_llm to use an independent LLM device control instead of reusing dit_device and the dead guard. Add/create a separate selectbox (e.g., st.session_state["llm_device_select"]) for the LLM device and pass that value to initialize_llm(lm_model_path=lm_model, backend=backend, device=st.session_state["llm_device_select"] or "auto"); remove the unnecessary check for "dit_device_select" and any code that assumes the DiT selector should control the LLM device, or alternatively make the coupling explicit by changing labels to indicate the LLM will mirror the DiT device.
1-297: Module exceeds 200 LOC hard cap (~297 lines).Extract tab body functions (
_show_model_settings,_show_hardware_settings,_show_storage_settings,_show_about_section, helpers) into a sibling module to bring this file under the limit.acestep/ui/streamlit/utils/project_manager.py (1)
40-43: Path traversal — unsanitizednamecan escapeprojects_dir.Already flagged in a prior review.
create_project,get_project, anddelete_projectall join user-controllednamedirectly toprojects_dir. Validate with.resolve().is_relative_to(...)or sanitize the name.Also applies to: 57-62, 114-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/utils/project_manager.py` around lines 40 - 43, The project name is used directly to build paths in create_project, get_project, and delete_project, allowing path traversal; fix by validating/sanitizing the user-supplied name before using it: reject names containing path separators or parent refs (e.g. "/" or "..") and/or normalize the joined path and assert (projects_dir / name).resolve().is_relative_to(projects_dir.resolve()), raising an error if the check fails; apply this validation in create_project, get_project, and delete_project (and any other place that joins user-controlled name) so no operation can escape projects_dir.acestep/ui/streamlit/components/editor_waveform.py (1)
115-120:sf.readfallback loads full audio at native sample rate — potential OOM for long files.Already flagged in a prior review. The librosa path downsamples to 8 kHz, but the soundfile fallback returns the full array at native rate. Downsample or decimate to match the librosa path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/editor_waveform.py` around lines 115 - 120, The soundfile fallback currently reads the full file with sf.read(str(audio_path)) which can OOM for long files; change the fallback in the same function that uses sf.read to downsample/decimate the loaded audio to the same target sample rate used by the librosa path (8 kHz) before returning y.mean(axis=1). After calling sf.read(str(audio_path), always_2d=True), compute the mono signal (y.mean(axis=1)) and then resample it to 8000 Hz (e.g., via librosa.resample or scipy.signal.resample/decimate) so the output sample rate and length match the librosa branch.
🧹 Nitpick comments (10)
acestep/ui/streamlit/components/generation_wizard.py (1)
221-221: Unused unpacked variablescol_landcol_r.Prefix with underscores to signal intentional discard:
- col_l, col_c, col_r = st.columns([1, 2, 1]) + _col_l, col_c, _col_r = st.columns([1, 2, 1])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/generation_wizard.py` at line 221, The unpacked Streamlit columns col_l, col_c, col_r include two unused variables; update the unpacking in generation_wizard.py to prefix unused locals with underscores (e.g., _col_l and _col_r) while keeping the used variable (col_c) unchanged so linters recognize the intentional discard and avoid unused-variable warnings.acestep/ui/streamlit/components/batch_generator.py (1)
55-68:status_emojidict is rebuilt on every loop iteration — hoist it outside the loop.🔧 Proposed fix
+ status_emoji = { + "queued": "⏳", + "generating": "⚙️", + "completed": "✅", + "failed": "❌", + } + if st.session_state.batch_queue: # Show as grid cols = st.columns(4) for idx, song in enumerate(st.session_state.batch_queue): with cols[idx % 4]: with st.container(border=True): st.markdown(f"**#{idx + 1}**") st.caption(song["caption"][:50] + "..." if len(song["caption"]) > 50 else song["caption"]) - # Status indicator - status_emoji = { - "queued": "⏳", - "generating": "⚙️", - "completed": "✅", - "failed": "❌" - } st.caption(f"{status_emoji.get(song['status'], '?')} {song['status'].title()}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/batch_generator.py` around lines 55 - 68, The status_emoji mapping is recreated on every iteration of the loop in the for idx, song in enumerate(st.session_state.batch_queue) block; hoist that dict (status_emoji) out of the loop (e.g., define it once above the for loop or as a module-level constant) and then use status_emoji.get(song['status'], '?') inside the loop so the mapping is not rebuilt each iteration.acestep/ui/streamlit/components/audio_player.py (1)
9-9:labelparameter is unused in bothaudio_player_widgetandsimple_audio_player.If these are intended for future use, prefix with underscore. Otherwise, remove them to avoid confusion.
Also applies to: 62-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/audio_player.py` at line 9, The `label` parameter is declared but unused in both functions; update the signatures for audio_player_widget and simple_audio_player to either remove the unused `label` parameter entirely or, if you intend to reserve it for future use, rename it to `_label` to indicate it's intentionally unused; ensure any callers are updated or handle the changed signature and remove any unused local references to avoid linter warnings.acestep/ui/streamlit/components/editor.py (1)
1-9: Replace EN DASH (–) with HYPHEN-MINUS (-) in docstrings.Ruff RUF002 flags ambiguous dashes on lines 2, 5–8. Use standard ASCII hyphens for consistency.
🔧 Proposed fix
-Editor component – thin orchestrator for interactive audio editing. +Editor component - thin orchestrator for interactive audio editing. Delegates to: -- ``editor_audio_picker`` – file selection from projects / outputs / upload -- ``editor_waveform`` – waveform display and region selector -- ``editor_tasks`` – repaint, cover, and complete UIs -- ``editor_runner`` – shared generation call +- ``editor_audio_picker`` - file selection from projects / outputs / upload +- ``editor_waveform`` - waveform display and region selector +- ``editor_tasks`` - repaint, cover, and complete UIs +- ``editor_runner`` - shared generation call🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/editor.py` around lines 1 - 9, The module docstring uses EN DASH characters (Unicode U+2013) in the list of delegated components; update those to standard ASCII hyphen-minus characters (U+002D) so lines like the descriptions for editor_audio_picker, editor_waveform, editor_tasks, and editor_runner use "-" instead of "–". Locate the top-of-file docstring in the editor module and replace each en dash with a hyphen-minus to satisfy RUF002.acestep/ui/streamlit/components/editor_runner.py (2)
82-126: Add return type annotation to_generate.The other two functions in this module have return type annotations.
_generateshould also declare its return type for consistency and tooling support.def _generate( ... seed: int, -): +) -> "GenerationResult":(Use a string annotation or import the type to avoid a circular/eager import.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/editor_runner.py` around lines 82 - 126, Add a return type annotation to the _generate function (which builds GenerationParams/GenerationConfig and calls generate_music) to match the other functions; annotate it as "GenerationResult" (string literal) or import GenerationResult under TYPE_CHECKING to avoid eager/circular imports so the signature becomes: def _generate(... ) -> "GenerationResult": ensuring the declared type matches the actual return from generate_music.
141-149: Project name sanitization is minimal — consider stripping more problematic characters.
caption[:25].replace(" ", "_").replace("/", "_")still allows characters like\,..,:,<,>,|,?,*, and control chars, which may cause issues on Windows or unexpected directory structures. A simple regex whitelist would be safer.🔧 Suggested improvement
+import re ... - safe = caption[:25].replace(" ", "_").replace("/", "_") + safe = re.sub(r"[^\w\-]", "_", caption[:25])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/editor_runner.py` around lines 141 - 149, The current sanitization of caption into safe (caption[:25].replace(" ", "_").replace("/", "_")) is too permissive; update the logic before calling ProjectManager.create_project to replace any character not in a safe whitelist (letters, digits, underscore, hyphen) with underscores using a regex, collapse repeated underscores, trim leading/trailing dots and spaces, enforce the max length (25) after sanitization, and fall back to a deterministic default (e.g., timestamp or "project") if the result is empty; apply this change where safe is computed prior to calling create_project and ensure dst uses the same sanitized name.acestep/ui/streamlit/config.py (1)
57-58:AUDIO_FORMATSis duplicated inproject_manager.py(Line 108).
ProjectManager.get_audio_fileshardcodes its ownaudio_extensionslist. ImportAUDIO_FORMATSfrom this config instead to keep the single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/config.py` around lines 57 - 58, Replace the duplicate hardcoded audio extensions in ProjectManager.get_audio_files with the single source of truth by importing AUDIO_FORMATS from the config module (acestep.ui.streamlit.config) and using that constant instead of the local audio_extensions list; update any variable names if needed (e.g., remove or replace audio_extensions) so get_audio_files references AUDIO_FORMATS directly.acestep/ui/streamlit/utils/project_manager.py (1)
81-95:save_metadataapplies arbitrary kwargs viasetattrwithout validation.Any misspelled key (e.g.,
gnereinstead ofgenre) is silently dropped by thehasattrcheck. Consider logging a warning for unrecognized keys to catch caller bugs early.Proposed fix
for key, value in kwargs.items(): if hasattr(metadata, key): setattr(metadata, key, value) + else: + logger.warning(f"Unknown metadata key ignored: {key}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/utils/project_manager.py` around lines 81 - 95, In save_metadata, validate incoming kwargs against the ProjectMetadata fields instead of silently dropping misspelled keys: after loading metadata via _load_metadata (or creating ProjectMetadata), iterate kwargs and if hasattr(metadata, key) keep the current setattr behavior, but collect any keys that are not attributes and emit a warning (use the module/class logger or Python logging) listing the unrecognized keys and their values to help callers detect typos; finally update metadata.modified_at and call _save_metadata as before.acestep/ui/streamlit/utils/cache.py (1)
15-18: Duplicatedsys.pathmanipulation — already done inconfig.py.Both
config.py(Lines 17-18) and this file (Lines 17-18) insert the repo root intosys.path. Since any component importing fromconfigwill already have the path set, this is redundant. Consider consolidating the path setup inconfig.pyalone and importing the root path from there.Proposed fix
-# Ensure ACE-Step repo is on Python path -_project_root = Path(__file__).parent.parent.parent.parent.parent -if str(_project_root) not in sys.path: - sys.path.insert(0, str(_project_root)) +from config import ACESTEP_ROOT as _project_root🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/utils/cache.py` around lines 15 - 18, Remove the duplicated sys.path manipulation in this module: delete the top-level block that computes _project_root and inserts it into sys.path, and instead import the canonical project root or ensure sys.path setup from config.py is used (e.g., import the root variable or a function from config.py). Keep the local module free of direct sys.path mutations; reference the existing symbol in config.py (the project root variable or initialization function) so other imports rely on the single canonical setup.acestep/ui/streamlit/components/editor_tasks.py (1)
38-47: Consider extracting the duplicated "Advanced" expander into a shared helper.All three task UIs repeat the same two-column expander with a diffusion-steps slider and a seed input. A small helper (e.g.,
_advanced_options(prefix) -> Tuple[int, int]) would eliminate the triple copy-paste.Example helper
def _advanced_options(prefix: str) -> Tuple[int, int]: """Render shared Advanced expander, return (steps, seed).""" with st.expander("Advanced", expanded=False): col1, col2 = st.columns(2) with col1: steps = st.slider( "Diffusion steps", 4, 100, 8, 4, key=f"{prefix}_steps" ) with col2: seed = st.number_input( "Seed (-1 random)", value=-1, key=f"{prefix}_seed" ) return steps, int(seed)Also applies to: 102-111, 148-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/ui/streamlit/components/editor_tasks.py` around lines 38 - 47, Multiple task UIs duplicate the same "Advanced" expander (creating local variables steps and seed with keys like "rp_steps"/"rp_seed"); extract a helper function named _advanced_options(prefix) that renders the st.expander with two columns, creates the slider and number_input using keys f"{prefix}_steps" and f"{prefix}_seed", returns (steps, int(seed)); then replace each duplicated block with a call to _advanced_options using an appropriate prefix for that task (e.g., "rp"), assigning the returned steps and seed variables where the originals were used.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
acestep/ui/streamlit/.gitignoreacestep/ui/streamlit/.streamlit/config.tomlacestep/ui/streamlit/.streamlit/secrets.tomlacestep/ui/streamlit/INSTALL.mdacestep/ui/streamlit/PROJECT_SUMMARY.mdacestep/ui/streamlit/QUICKSTART.mdacestep/ui/streamlit/README.mdacestep/ui/streamlit/components/__init__.pyacestep/ui/streamlit/components/audio_player.pyacestep/ui/streamlit/components/batch_generator.pyacestep/ui/streamlit/components/dashboard.pyacestep/ui/streamlit/components/editor.pyacestep/ui/streamlit/components/editor_audio_picker.pyacestep/ui/streamlit/components/editor_runner.pyacestep/ui/streamlit/components/editor_tasks.pyacestep/ui/streamlit/components/editor_waveform.pyacestep/ui/streamlit/components/generation_wizard.pyacestep/ui/streamlit/components/settings_panel.pyacestep/ui/streamlit/config.pyacestep/ui/streamlit/main.pyacestep/ui/streamlit/requirements.txtacestep/ui/streamlit/run.batacestep/ui/streamlit/run.shacestep/ui/streamlit/utils/__init__.pyacestep/ui/streamlit/utils/audio_utils.pyacestep/ui/streamlit/utils/cache.pyacestep/ui/streamlit/utils/project_manager.py
✅ Files skipped from review due to trivial changes (3)
- acestep/ui/streamlit/requirements.txt
- acestep/ui/streamlit/.gitignore
- acestep/ui/streamlit/.streamlit/secrets.toml
| audio_bytes = f.read() | ||
|
|
||
| # Display audio player | ||
| st.audio(audio_bytes, format="audio/wav") |
There was a problem hiding this comment.
Hardcoded format="audio/wav" will misidentify non-WAV files (FLAC, MP3, etc.).
The generation pipeline defaults to FLAC output (GenerationConfig.audio_format = "flac"), so this player will send the wrong MIME type for most generated audio. Streamlit's st.audio can auto-detect format if you pass the file path directly instead of bytes, or you can infer from the suffix.
🔧 Proposed fix
+_MIME_MAP = {
+ ".wav": "audio/wav",
+ ".mp3": "audio/mpeg",
+ ".flac": "audio/flac",
+ ".ogg": "audio/ogg",
+ ".m4a": "audio/mp4",
+}
+
def audio_player_widget(audio_path: str, label: str = "Audio", show_download: bool = True):
...
+ mime = _MIME_MAP.get(audio_file.suffix.lower(), "audio/wav")
...
- st.audio(audio_bytes, format="audio/wav")
+ st.audio(audio_bytes, format=mime)
...
- mime="audio/wav",
+ mime=mime,Also applies to: 57-57, 72-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/components/audio_player.py` at line 28, The st.audio
call hardcodes format="audio/wav", causing incorrect MIME for non-WAV outputs;
update the audio player to either pass the audio file path to st.audio so
Streamlit can auto-detect the format or infer the MIME from the file suffix
(e.g., ".flac", ".mp3", ".wav") and pass that as the format argument; change the
st.audio usages in audio_player.py (the call at the shown diff and the other
occurrences referenced around lines 57 and 72) to use the inferred MIME or the
file path instead of always "audio/wav".
| if st.button("🗑️", key=f"del_{project['name']}", help="Delete"): | ||
| if pm.delete_project(project['name']): | ||
| st.success(f"Deleted: {project['name']}") | ||
| st.rerun() |
There was a problem hiding this comment.
st.success() message is immediately lost when st.rerun() is called on the next line.
The user will never see the "Deleted: ..." confirmation. Use st.toast() instead, which persists across reruns.
🔧 Proposed fix
if st.button("🗑️", key=f"del_{project['name']}", help="Delete"):
if pm.delete_project(project['name']):
- st.success(f"Deleted: {project['name']}")
+ st.toast(f"Deleted: {project['name']}", icon="🗑️")
st.rerun()📝 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.
| if st.button("🗑️", key=f"del_{project['name']}", help="Delete"): | |
| if pm.delete_project(project['name']): | |
| st.success(f"Deleted: {project['name']}") | |
| st.rerun() | |
| if st.button("🗑️", key=f"del_{project['name']}", help="Delete"): | |
| if pm.delete_project(project['name']): | |
| st.toast(f"Deleted: {project['name']}", icon="🗑️") | |
| st.rerun() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/components/dashboard.py` around lines 82 - 85, Replace
the ephemeral success call so the confirmation survives the immediate rerun:
when handling the delete button (the block that calls
pm.delete_project(project['name']) and then st.rerun()), swap the
st.success(...) call to st.toast(...) and pass the same message (e.g.,
f"Deleted: {project['name']}") before calling st.rerun(); keep the
pm.delete_project(...) and st.rerun() logic intact but use st.toast instead of
st.success so the message persists across reruns.
| def _get_duration(audio_path: Path) -> float: | ||
| """Return audio duration in seconds.""" | ||
| if sf is not None: | ||
| try: | ||
| return sf.info(str(audio_path)).duration | ||
| except Exception: | ||
| pass | ||
| if librosa is not None: | ||
| try: | ||
| return librosa.get_duration(path=str(audio_path)) | ||
| except Exception: | ||
| pass | ||
| return 120.0 # fallback | ||
|
|
||
|
|
||
| def _load_waveform( | ||
| audio_path: Path, | ||
| target_sr: int = 8000, | ||
| ) -> Optional[np.ndarray]: | ||
| """Load a mono, downsampled waveform for visualisation.""" | ||
| if librosa is not None: | ||
| try: | ||
| y, _ = librosa.load(str(audio_path), sr=target_sr, mono=True) | ||
| return y | ||
| except Exception: | ||
| pass | ||
| if sf is not None: | ||
| try: | ||
| y, _ = sf.read(str(audio_path), always_2d=True) | ||
| return y.mean(axis=1) | ||
| except Exception: | ||
| pass | ||
| return None |
There was a problem hiding this comment.
Silent except: pass blocks in _get_duration and _load_waveform hide failures.
Four try/except Exception: pass blocks (Lines 94, 99, 113, 119) silently swallow errors. When both librosa and soundfile fail, the user gets a 120s fallback duration or no waveform with zero diagnostics. Import logger from loguru and log at warning level so issues are traceable.
Proposed fix (example for _get_duration)
+from loguru import logger
...
def _get_duration(audio_path: Path) -> float:
"""Return audio duration in seconds."""
if sf is not None:
try:
return sf.info(str(audio_path)).duration
- except Exception:
- pass
+ except Exception as exc:
+ logger.warning(f"soundfile duration failed for {audio_path.name}: {exc}")
if librosa is not None:
try:
return librosa.get_duration(path=str(audio_path))
- except Exception:
- pass
+ except Exception as exc:
+ logger.warning(f"librosa duration failed for {audio_path.name}: {exc}")
return 120.0 # fallbackApply the same pattern to _load_waveform.
As per coding guidelines: "Handle errors explicitly in Python; avoid bare except" and "Keep logging actionable."
🧰 Tools
🪛 Ruff (0.15.2)
[error] 94-95: try-except-pass detected, consider logging the exception
(S110)
[warning] 94-94: Do not catch blind exception: Exception
(BLE001)
[error] 99-100: try-except-pass detected, consider logging the exception
(S110)
[warning] 99-99: Do not catch blind exception: Exception
(BLE001)
[warning] 112-112: Consider moving this statement to an else block
(TRY300)
[error] 113-114: try-except-pass detected, consider logging the exception
(S110)
[warning] 113-113: Do not catch blind exception: Exception
(BLE001)
[error] 119-120: try-except-pass detected, consider logging the exception
(S110)
[warning] 119-119: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/components/editor_waveform.py` around lines 89 - 121,
Replace the silent "except: pass" blocks in _get_duration and _load_waveform
with explicit exception handling that logs failures using a logger (import
logger from loguru); for each except Exception block in functions _get_duration
and _load_waveform, call logger.warning with a clear message including the
audio_path, which backend failed (librosa or soundfile), and include the
exception details (e.g., exception object) so diagnostics are available; keep
the existing fallbacks (120.0 duration and returning None) but ensure every
failure path is logged before falling back.
| def _quick_init_dit() -> None: | ||
| """One-click DiT init from the Generate page.""" | ||
| import sys as _sys |
There was a problem hiding this comment.
Redundant import sys as _sys — sys is already imported at module level (Line 7).
def _quick_init_dit() -> None:
"""One-click DiT init from the Generate page."""
- import sys as _sys
-
with st.spinner("Loading DiT model..."):
_status, _ok = initialize_dit(
config_path="acestep-v15-turbo",
device="auto",
- offload_to_cpu=(_sys.platform != "darwin"),
+ offload_to_cpu=(sys.platform != "darwin"),
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/components/generation_wizard.py` around lines 32 - 34,
The function _quick_init_dit contains a redundant local import "import sys as
_sys" because sys is already imported at module scope; remove the local import
from _quick_init_dit and use the module-level sys symbol directly (update any
references inside _quick_init_dit to sys if they currently use _sys).
| from config import PROJECTS_DIR, CACHE_DIR, CHECKPOINTS_DIR | ||
| from utils import ( | ||
| get_dit_handler, | ||
| get_llm_handler, | ||
| is_dit_ready, | ||
| is_llm_ready, | ||
| initialize_dit, | ||
| initialize_llm, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Imports rely on implicit sys.path manipulation — use explicit relative imports.
from config import ... and from utils import ... are bare (non-relative) imports that only resolve because config.py manipulates sys.path at import time. This is fragile and breaks if the module is ever imported from a different working directory or by a test runner. Prefer explicit relative imports (from ..config import ..., from ..utils import ...) throughout the package.
Proposed fix
-from config import PROJECTS_DIR, CACHE_DIR, CHECKPOINTS_DIR
-from utils import (
+from ..config import PROJECTS_DIR, CACHE_DIR, CHECKPOINTS_DIR
+from ..utils import (
get_dit_handler,
get_llm_handler,
is_dit_ready,As per coding guidelines: "Keep data flow explicit (data in, data out); side effects must be obvious and deliberate."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/components/settings_panel.py` around lines 10 - 18,
Change the bare imports to explicit relative package imports: replace "from
config import PROJECTS_DIR, CACHE_DIR, CHECKPOINTS_DIR" with an appropriate
relative import (e.g., "from ..config import PROJECTS_DIR, CACHE_DIR,
CHECKPOINTS_DIR") and replace "from utils import (...get_dit_handler,
get_llm_handler, is_dit_ready, is_llm_ready, initialize_dit, initialize_llm...)"
with the corresponding relative form (e.g., "from ..utils import ..."). Update
only the import lines in settings_panel.py (targeting the symbols
get_dit_handler, get_llm_handler, is_dit_ready, is_llm_ready, initialize_dit,
initialize_llm, PROJECTS_DIR, CACHE_DIR, CHECKPOINTS_DIR) so the module no
longer depends on implicit sys.path manipulation; run tests to confirm imports
resolve.
| cd .. && python -m acestep.model_downloader | ||
| ``` |
There was a problem hiding this comment.
Fix the model-downloader troubleshooting command path.
At Line 204, cd .. from acestep/ui/streamlit does not reach the repo root where python -m acestep.model_downloader is expected to run.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/QUICKSTART.md` around lines 204 - 205, The
troubleshooting command shown from within acestep/ui/streamlit uses `python -m
acestep.model_downloader` but the preceding `cd ..` does not reach the
repository root, so update the instruction in QUICKSTART.md to change the
working-directory step so the module can be executed (e.g., replace the `cd ..`
step with `cd ../..` or otherwise instruct the user to run `python -m
acestep.model_downloader` from the repo root); locate the line referencing
`acestep.model_downloader` and adjust the path instruction accordingly.
| ``` | ||
| acestep/ui/streamlit/ | ||
| ├── main.py # Main Streamlit app | ||
| ├── config.py # Configuration constants | ||
| ├── requirements.txt # Python dependencies | ||
| ├── components/ # UI components | ||
| │ ├── dashboard.py # Home page with recent projects | ||
| │ ├── generation_wizard.py # Song creation wizard | ||
| │ ├── editor.py # Audio editing tools | ||
| │ ├── batch_generator.py # Batch generation queue | ||
| │ ├── settings_panel.py # Configuration panel | ||
| │ └── audio_player.py # Audio playback widget | ||
| ├── utils/ # Utility modules | ||
| │ ├── cache.py # Handler caching | ||
| │ ├── project_manager.py # Project management | ||
| │ └── audio_utils.py # Audio file handling | ||
| └── projects/ # Saved projects directory | ||
| ``` |
There was a problem hiding this comment.
Specify a language for this fenced code block to satisfy markdown linting.
The project’s static analysis flagged this fence as missing a language identifier.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/README.md` around lines 40 - 57, The fenced code block
in the README (acestep/ui/streamlit/README.md) is missing a language identifier
which breaks markdown linting; open the README and change the opening fence from
``` to a labeled fence such as ```text or ```bash (or ```tree) so the block is
detected as code by linters and renderers—update the block that lists files (the
tree-like block under the project path) to use the chosen language identifier.
| if not exist "..\..\..\.venv" ( | ||
| echo Creating virtual environment... | ||
| python -m venv ..\..\..\.venv | ||
| ) | ||
|
|
||
| REM Activate venv | ||
| echo Activating virtual environment... | ||
| call ..\..\..\.venv\Scripts\activate.bat | ||
|
|
||
| REM Install dependencies | ||
| echo Installing Streamlit dependencies... | ||
| pip install -q -r requirements.txt | ||
|
|
||
| REM Run the app | ||
| echo. | ||
| echo ================================== | ||
| echo ✅ Setup complete! | ||
| echo 🚀 Starting ACE Studio... | ||
| echo 📱 Open: http://localhost:8501 | ||
| echo ================================== | ||
| echo. | ||
|
|
||
| streamlit run main.py |
There was a problem hiding this comment.
Make path resolution independent of the caller’s working directory.
Lines 14–36 assume the script is launched from acestep/ui/streamlit; invoking it from elsewhere will break .venv, requirements.txt, and main.py resolution.
Suggested patch
`@echo` off
REM Quick start script for ACE Studio Streamlit UI (Windows)
setlocal enabledelayedexpansion
+pushd "%~dp0" || exit /b 1
@@
streamlit run main.py
-endlocal
+set "EXIT_CODE=%ERRORLEVEL%"
+popd
+endlocal & exit /b %EXIT_CODE%🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/run.bat` around lines 14 - 36, The batch script assumes
the current working directory; make it robust by resolving the script directory
with %~dp0 (or pushd "%~dp0" / popd) and then use that base to construct
absolute paths to the virtualenv, requirements.txt and main.py (replace
occurrences of ..\..\..\.venv, requirements.txt and main.py with paths built
from the script directory), ensure paths are quoted, and call the venv
activation via the resolved path (e.g., call "%VENV%\Scripts\activate.bat") so
the script works regardless of the caller’s working directory.
| def clear_handlers() -> None: | ||
| """Clear all cached handlers (forces re-creation).""" | ||
| st.cache_resource.clear() |
There was a problem hiding this comment.
clear_handlers() clears all @st.cache_resource singletons, not just the handlers.
st.cache_resource.clear() is a global nuke — it will also evict any other cached resources in the app (e.g., dataset handler, or resources cached by third-party components). If the intent is to only reset model handlers, use the per-function .clear() method instead.
Proposed fix
def clear_handlers() -> None:
"""Clear all cached handlers (forces re-creation)."""
- st.cache_resource.clear()
+ get_dit_handler.clear()
+ get_llm_handler.clear()
+ get_dataset_handler.clear()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/utils/cache.py` around lines 145 - 147, The
clear_handlers function currently calls st.cache_resource.clear(), which evicts
every cached resource globally; instead, call the per-function .clear() on only
the cached handler factories (replace st.cache_resource.clear() in
clear_handlers with explicit clears like my_handler_factory.clear(),
model_handler_factory.clear(), etc. where
my_handler_factory/model_handler_factory are the `@st.cache_resource-decorated`
functions that create your handlers) so only those handler singletons are reset
and other app resources remain untouched.
| def _load_metadata(self, project_path: Path) -> Optional[ProjectMetadata]: | ||
| """Load metadata JSON file""" | ||
| metadata_path = project_path / "metadata.json" | ||
| if metadata_path.exists(): | ||
| try: | ||
| with open(metadata_path, "r") as f: | ||
| data = json.load(f) | ||
| return ProjectMetadata(**data) | ||
| except Exception as e: | ||
| logger.error(f"Failed to load metadata from {metadata_path}: {e}") | ||
| return None |
There was a problem hiding this comment.
_load_metadata will crash on schema-mismatched JSON.
If metadata.json contains extra keys (e.g., from a future version) or is missing required fields, ProjectMetadata(**data) raises TypeError. The surrounding except Exception catches it, but the project silently disappears from listings. Consider filtering data to known fields before construction.
Proposed fix
try:
with open(metadata_path, "r") as f:
data = json.load(f)
- return ProjectMetadata(**data)
+ # Filter to known fields to tolerate schema drift
+ import dataclasses
+ known = {f.name for f in dataclasses.fields(ProjectMetadata)}
+ return ProjectMetadata(**{k: v for k, v in data.items() if k in known})
except Exception as e:
logger.error(f"Failed to load metadata from {metadata_path}: {e}")🧰 Tools
🪛 Ruff (0.15.2)
[warning] 137-137: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/ui/streamlit/utils/project_manager.py` around lines 129 - 139, The
_load_metadata function currently passes raw JSON to ProjectMetadata(**data)
which raises TypeError on extra/missing keys and causes projects to disappear;
change it to validate and sanitize the dict before constructing ProjectMetadata
by: load the JSON into data, build a filtered dict containing only the fields
defined on ProjectMetadata (e.g., inspect ProjectMetadata.__fields__ or its
dataclass attributes), then call ProjectMetadata(**filtered_data) inside the
existing try/except; keep the error log but include the original data and
exception, and return None on failure so listings remain stable.
3 participants
Alternative UI to the existing Gradio interface, built with Streamlit.
Key features:
Architecture:
All generation calls use the existing acestep.inference.generate_music() API - no changes to the core ACE-Step codebase.
Summary by CodeRabbit
New Features
Documentation
Chores