refactor: relocate gradio_ui → ui/gradio and remove old shim#599
refactor: relocate gradio_ui → ui/gradio and remove old shim#599
Conversation
- Move all UI code from acestep/gradio_ui/ to acestep/ui/gradio/ - Split results_handlers.py (2480 LOC) into 8 focused modules under events/results/ - Split generation_handlers.py (2059 LOC) into 7 focused modules under events/generation/ - Split training_handlers.py (1207 LOC) into 5 focused modules under events/training/ - Update acestep_v15_pipeline.py import to use new path - Delete the entire acestep/gradio_ui/ directory (no backward-compat shims) - Add unit tests for all new sub-modules and facades - All 254 tests pass (2 pre-existing help_content_test failures unchanged)
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughReorganizes the Gradio UI from Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Gradio UI
participant GenFacade as generation_handlers
participant DiT as DiT Handler
participant LLM as LLM Handler
participant Audio as Audio Processing
UI->>GenFacade: analyze_src_audio(src_audio)
GenFacade->>Audio: convert_src_audio_to_codes(src_audio)
GenFacade->>LLM: understand_music(codes)
LLM-->>GenFacade: caption, lyrics, metadata
GenFacade->>DiT: validate/convert tensors (optional)
GenFacade-->>UI: return codes + metadata + UI updates
sequenceDiagram
participant UI as Gradio UI
participant BatchMgr as batch_management
participant GenProg as generation_progress
participant DiT as DiT Handler
participant LLM as LLM Handler
participant Scorer as scoring
participant LRC as lrc_utils
UI->>BatchMgr: generate_with_batch_management(params)
BatchMgr->>GenProg: delegate batch generation
GenProg->>DiT: generate_music(config)
DiT-->>GenProg: audio samples + metadata
GenProg->>Scorer: calculate_score_handler(audio_codes)
Scorer->>LLM: compute PMI scores
Scorer-->>GenProg: scores
GenProg->>LRC: generate_lrc_handler(sample)
LRC-->>GenProg: lrc/vtt
GenProg-->>BatchMgr: progressive UI updates
BatchMgr-->>UI: final batch state + stored results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
CodeQL flagged line 57 (and related lines) in data_module.py as 'Uncontrolled data used in path expression' (High severity). Changes: - Add _resolve_and_validate_dir(): resolves symlinks, verifies the path is an existing directory before any filesystem operations. - Add _safe_child_path(): ensures paths from manifest.json cannot escape tensor_dir via ../ traversal or symlinks. - PreprocessedTensorDataset.__init__: use resolved tensor_dir; validate every manifest path against the sandbox before use. - AceStepTrainingDataset._validate_samples: resolve audio_path entries via realpath before storing, so downstream code never operates on raw user input. - load_dataset_from_json: resolve and validate before opening. - Add 12 unit tests covering traversal rejection, normal paths, nonexistent paths, and manifest sanitisation.
Add acestep/training/path_safety.py with a centralised safe_path() function that uses os.path.normpath + startswith — the exact pattern CodeQL recognises as a py/path-injection sanitiser. Apply safe_path() at every user-controlled filesystem entry point: - data_module.py (PreprocessedTensorDataset, load_dataset_from_json) - audio_io.py (_read_text_file, load_json_metadata, get_audio_duration) - csv_metadata.py (load_csv_metadata) - scan.py (scan_directory) - serialization.py (save_dataset, load_dataset) - trainer.py (LoRATrainer, LoKRTrainer) - lora_utils.py (save/load weights and checkpoints) - lokr_utils.py (save weights and checkpoints) - lokr_training.py (start_lokr_training, list/export_lokr) - lora_training.py (start_training, export_lora) - preprocess.py (load/preprocess/load_training_dataset) - training_utils.py (_safe_join now uses normpath+startswith) Update data_module_test.py to test safe_path integration instead of removed _resolve_and_validate_dir / _safe_child_path helpers. All 264 existing tests pass (2 pre-existing failures in help_content_test.py are unrelated to this change).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
acestep/ui/gradio/help_content_test.py (1)
1-254: 🛠️ Refactor suggestion | 🟠 MajorSplit this test module to stay under 200 LOC.
The file is ~254 LOC, exceeding the hard cap. Consider splitting into focused test modules (e.g.,
help_content_md_test.py,help_content_i18n_test.py,help_content_button_test.py).As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
🟠 Major comments (28)
acestep/ui/gradio/events/training/lora_training.py-47-56 (1)
47-56:⚠️ Potential issue | 🟠 MajorValidate
tensor_dirandlora_output_diragainstSAFE_TRAINING_ROOT.Both paths come from UI inputs and are used to read/write on disk. They should be normalized with
_safe_jointo prevent traversal/out-of-sandbox access and to align with the new path-safety work.Safer path handling
tensor_dir = tensor_dir.strip() - - if not os.path.exists(tensor_dir): - yield f"❌ Tensor directory not found: {tensor_dir}", "", None, training_state + safe_tensor_dir = _safe_join(SAFE_TRAINING_ROOT, tensor_dir) + if safe_tensor_dir is None or not os.path.exists(safe_tensor_dir): + yield f"❌ Tensor directory not found or invalid: {tensor_dir}", "", None, training_state return + tensor_dir = safe_tensor_dir @@ - training_config = TrainingConfig( + safe_lora_output_dir = _safe_join(SAFE_TRAINING_ROOT, lora_output_dir) + if safe_lora_output_dir is None: + training_state["is_training"] = False + yield "❌ Invalid LoRA output directory.", "", None, training_state + return + training_config = TrainingConfig( @@ - seed=training_seed, output_dir=lora_output_dir, + seed=training_seed, output_dir=safe_lora_output_dir,Also applies to: 140-148
acestep/ui/gradio/events/training/lora_training.py-1-235 (1)
1-235: 🛠️ Refactor suggestion | 🟠 MajorSplit
start_training(and this module) to meet size/one-responsibility constraints.This file is ~295 LOC and
start_trainingbundles validation, device config, training execution, logging, and plotting. Please extract helpers (e.g.,_validate_inputs,_build_training_config,_stream_training_updates) and/or move parts to a separate module so the file stays under the 200 LOC hard cap.
As per coding guidelines: “Target module size: optimal <= 150 LOC, hard cap 200 LOC”; “Function decomposition: do one thing at a time; if a function description naturally contains 'and', split it”; “Keep functions focused and short in Python”.acestep/ui/gradio/events/training/preprocess_test.py-15-89 (1)
15-89: 🛠️ Refactor suggestion | 🟠 MajorAdd docstrings to the new test methods.
The new test functions are missing docstrings. Please add concise purpose statements to each test.
As per coding guidelines, "Docstrings are mandatory for all new or modified Python modules, classes, and functions" and "Docstrings must be concise and include purpose plus key inputs/outputs (and raised exceptions when relevant)".
acestep/ui/gradio/events/training/training_utils.py-22-110 (1)
22-110: 🛠️ Refactor suggestion | 🟠 MajorDocstrings should include key inputs/outputs.
Several new helpers have purpose-only docstrings. Please add brief inputs/outputs details (and exceptions where relevant).
As per coding guidelines, "Docstrings are mandatory for all new or modified Python modules, classes, and functions" and "Docstrings must be concise and include purpose plus key inputs/outputs (and raised exceptions when relevant)".
acestep/ui/gradio/events/training/training_utils_test.py-15-105 (1)
15-105: 🛠️ Refactor suggestion | 🟠 MajorAdd docstrings to the new test methods.
Test functions need concise docstrings to comply with the documentation rule.
As per coding guidelines, "Docstrings are mandatory for all new or modified Python modules, classes, and functions" and "Docstrings must be concise and include purpose plus key inputs/outputs (and raised exceptions when relevant)".
acestep/ui/gradio/events/training/training_utils.py-47-62 (1)
47-62:⚠️ Potential issue | 🟠 MajorHarden
_safe_joinagainst symlink escape.
abspathdoesn’t resolve symlinks, so a symlink insidebase_rootcan still escape. Userealpathfor both root and joined paths before the commonpath check.🔒 Proposed fix
- abs_root = os.path.abspath(base_root) - joined = os.path.abspath(os.path.join(abs_root, candidate)) + abs_root = os.path.realpath(base_root) + joined = os.path.realpath(os.path.join(abs_root, candidate))acestep/ui/gradio/events/training/preprocess_test.py-57-89 (1)
57-89:⚠️ Potential issue | 🟠 MajorAdd a success-path test for
preprocess_dataset.Current tests cover only error outcomes. Please add a passing-case assertion to satisfy the required success-path coverage.
As per coding guidelines, "Include at least: one success-path test, one regression/edge-case test for the bug being fixed, and one non-target behavior check when relevant".✅ Example success-path test
class TestPreprocessDataset(unittest.TestCase): """Tests for preprocess_dataset.""" @@ def test_no_model(self): builder = MagicMock() builder.samples = [MagicMock()] builder.get_labeled_count.return_value = 5 result = preprocess_dataset("/out", "lora", None, builder) self.assertIn("❌", result) + + def test_success(self): + builder = MagicMock() + builder.samples = [MagicMock()] + builder.get_labeled_count.return_value = 5 + model = MagicMock() + result = preprocess_dataset("/out", "lora", model, builder) + self.assertNotIn("❌", result)acestep/ui/gradio/events/training/training_utils_test.py-31-46 (1)
31-46:⚠️ Potential issue | 🟠 MajorMake path tests cross-platform (Windows-safe).
Hard-coded POSIX paths can fail on Windows. Build paths using
os.pathand validate viacommonpathinstead of string prefix checks.🧪 Cross-platform test adjustment
-import unittest +import os +import unittest @@ def test_valid_relative_path(self): - result = _safe_join("/base", "subdir/file.txt") - # Should be under /base - self.assertIsNotNone(result) - self.assertTrue(result.startswith("/base")) + base = os.path.abspath(os.path.join(os.sep, "base")) + result = _safe_join(base, os.path.join("subdir", "file.txt")) + self.assertIsNotNone(result) + self.assertEqual(os.path.commonpath([base, result]), base) @@ def test_absolute_path_rejected(self): - result = _safe_join("/base", "/etc/passwd") + base = os.path.abspath(os.path.join(os.sep, "base")) + abs_path = os.path.abspath(os.path.join(os.sep, "etc", "passwd")) + result = _safe_join(base, abs_path) self.assertIsNone(result) @@ def test_traversal_rejected(self): - result = _safe_join("/base/root", "../../etc/passwd") + base = os.path.abspath(os.path.join(os.sep, "base", "root")) + result = _safe_join(base, os.path.join("..", "..", "etc", "passwd")) self.assertIsNone(result)acestep/ui/gradio/events/results/batch_queue.py-1-209 (1)
1-209: 🛠️ Refactor suggestion | 🟠 MajorModule exceeds the 200 LOC hard cap.
Please split into smaller modules (e.g., batch storage vs param capture vs restore).
As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
acestep/ui/gradio/events/training/preprocess.py-1-230 (1)
1-230: 🛠️ Refactor suggestion | 🟠 MajorModule exceeds the 200 LOC hard cap.
Please split this file into smaller modules (e.g., dataset loading vs preprocessing vs tensor loading).
As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
acestep/ui/gradio/events/results/batch_queue.py-162-165 (1)
162-165:⚠️ Potential issue | 🟠 MajorReturn arity mismatch on the error path.
restore_batch_parametersreturns 24 values on success but only 20 on the error path, which will break Gradio output wiring.✅ Fix arity to 24
- return [gr.update()] * 20 + return [gr.update()] * 24acestep/ui/gradio/events/results/scoring.py-70-74 (1)
70-74:⚠️ Potential issue | 🟠 MajorGuard
llm_handlerbefore accessing.llm_initialized.If
llm_handlerisNoneand audio codes exist, this path raisesAttributeErrorand bypasses the fallback.🐛 Safe guard
- if not llm_handler.llm_initialized: + if not llm_handler or not llm_handler.llm_initialized:acestep/ui/gradio/events/generation/mode_ui.py-15-172 (1)
15-172: 🛠️ Refactor suggestion | 🟠 MajorSplit
mode_ui.pyto respect the 200 LOC cap.
This module exceeds the hard cap; consider moving helper functions into separate files (e.g., automation updates, repainting labels, extract helpers). As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC."acestep/ui/gradio/events/training/lokr_training.py-254-320 (1)
254-320:⚠️ Potential issue | 🟠 MajorHarden export paths against traversal/symlink escape.
export_pathandlokr_output_dirare user-controlled and used for filesystem writes. Resolve and validate them (e.g., restrict exports to a configured base directory) before creating directories/copying files.acestep/ui/gradio/events/training/lokr_training.py-45-52 (1)
45-52:⚠️ Potential issue | 🟠 MajorValidate and sandbox
tensor_dirbefore filesystem access.
This is user-controlled input used for filesystem traversal; resolve and validate it against an allowed base (reusing the new path-sanitization helpers added indata_module.py, or a local equivalent) to prevent traversal/symlink escape.acestep/ui/gradio/events/generation/service_init.py-26-169 (1)
26-169: 🛠️ Refactor suggestion | 🟠 MajorSplit this module to respect the 200 LOC cap.
service_init.pyis beyond the hard cap; please split LM init, batch-size clamping, and UI update assembly into smaller helpers/modules. As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC."acestep/ui/gradio/events/generation/ui_helpers.py-1-228 (1)
1-228: 🛠️ Refactor suggestion | 🟠 MajorSplit
ui_helpers.pyto satisfy the 200 LOC cap.
The module exceeds the hard cap; consider moving auto-checkbox helpers, audio visibility helpers, and instruction helpers into separate modules. As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC."acestep/ui/gradio/events/training/dataset_ops.py-16-221 (1)
16-221: 🛠️ Refactor suggestion | 🟠 MajorSplit
dataset_ops.pyto meet the 200 LOC cap.
The module exceeds the hard cap; consider moving scan/label/preview/save into separate focused modules. As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC."acestep/ui/gradio/events/results/batch_management.py-22-167 (1)
22-167: 🛠️ Refactor suggestion | 🟠 MajorSplit
batch_management.pyto respect the 200 LOC cap.
This file is well beyond the hard cap; consider splitting orchestration, queue ops, and helper utilities into separate modules. As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC."acestep/ui/gradio/events/training/lokr_training.py-19-195 (1)
19-195: 🛠️ Refactor suggestion | 🟠 MajorBreak up the LoKr training module/function.
start_lokr_training(and this module) exceed the 200 LOC cap; extracting helpers for validation, config build, training loop, and finalization will improve maintainability. As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC."acestep/ui/gradio/events/training/dataset_ops.py-271-278 (1)
271-278:⚠️ Potential issue | 🟠 MajorWarning branch doesn’t actually save the dataset.
The message says “Saving anyway…”, but the function returns before saving. Save the dataset and prefix the warning.🔧 Suggested fix
- if labeled_count == 0: - return ( - "⚠️ Warning: No samples have been labeled. Consider auto-labeling first.\nSaving anyway...", - gr.update(value=save_path), - ) + if labeled_count == 0: + status = builder_state.save_dataset(save_path, dataset_name) + return ( + f"⚠️ Warning: No samples have been labeled. Consider auto-labeling first.\n{status}", + gr.update(value=save_path), + )acestep/ui/gradio/events/__init__.py-1-20 (1)
1-20: 🛠️ Refactor suggestion | 🟠 MajorSplit this module to meet the 200‑LOC hard cap.
This file is far beyond the limit; please move generation/results/training wiring into dedicated submodules and keep this file as a thin coordinator. As per coding guidelines "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
acestep/ui/gradio/interfaces/__init__.py-1-289 (1)
1-289:⚠️ Potential issue | 🟠 MajorSplit this module; it exceeds the 200 LOC hard cap.
Line 1-289: the UI builder plus embedded CSS pushes this file well past the hard cap. Please extract CSS into a dedicated constants module and/or split the UI assembly into smaller helpers (header, settings, tabs, handler wiring).
As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
acestep/ui/gradio/events/results/generation_info.py-15-22 (1)
15-22:⚠️ Potential issue | 🟠 MajorAvoid creating directories at import time.
Line 16-22:
os.makedirs(DEFAULT_RESULTS_DIR, exist_ok=True)executes during module import, which can fail on read-only deployments and hides side effects. Defer directory creation to the call site that writes files (e.g., insidelrc_to_vtt_fileor a dedicatedensure_results_dir()helper).As per coding guidelines, "Keep data flow explicit (data in, data out); side effects must be obvious and deliberate".
acestep/ui/gradio/events/results/lrc_utils.py-1-288 (1)
1-288:⚠️ Potential issue | 🟠 MajorSplit this module; it exceeds the 200 LOC hard cap.
Line 1-288: parsing, VTT generation, file I/O, and LRC generation flow are all in one file. Please split into smaller modules (e.g.,
lrc_parsing.py,vtt_io.py,lrc_generation.py).As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
acestep/ui/gradio/events/generation/metadata_loading.py-1-308 (1)
1-308:⚠️ Potential issue | 🟠 MajorSplit this module; it exceeds the 200 LOC hard cap.
Line 1-308: this file combines JSON loading, example selection, LM fallback logic, and simple-mode sampling. Please split into focused modules (e.g.,
metadata_loader.py,example_sampler.py) to meet the size cap.As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
acestep/ui/gradio/events/generation/llm_actions.py-1-440 (1)
1-440:⚠️ Potential issue | 🟠 MajorSplit this module; it exceeds the 200 LOC hard cap.
Line 1-440: this file combines multiple LLM-facing actions and helpers. Please split into smaller modules (e.g.,
analysis.py,formatting.py,sampling.py) to meet the size cap.As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
acestep/ui/gradio/events/results/generation_progress.py-1-343 (1)
1-343:⚠️ Potential issue | 🟠 MajorSplit this module; it exceeds the 200 LOC hard cap.
Line 1-343: this file packs the generator, scoring, LRC logic, and helpers into one module. Please extract helpers (e.g., audio save, scoring/LRC helpers) into separate files.
As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
🟡 Minor comments (34)
acestep/ui/gradio/events/training/training_utils.py-15-15 (1)
15-15:⚠️ Potential issue | 🟡 MinorRemove unused
# noqa: E402.Ruff flags it as unused.
🧹 Proposed cleanup
-import matplotlib.pyplot as plt # noqa: E402 +import matplotlib.pyplot as pltacestep/ui/gradio/help_content_test.py-29-35 (1)
29-35:⚠️ Potential issue | 🟡 MinorRemove unused
# noqa: E402directives.Ruff flags these as unused; they can be dropped to keep lint clean.
🧹 Proposed cleanup
-from acestep.ui.gradio.help_content import ( # noqa: E402 +from acestep.ui.gradio.help_content import ( @@ -from acestep.ui.gradio.i18n import I18n # noqa: E402 +from acestep.ui.gradio.i18n import I18nacestep/ui/gradio/events/training/__init__.py-7-33 (1)
7-33:⚠️ Potential issue | 🟡 MinorRemove unused
# noqa: F401directives.Ruff reports these as unused; they can be removed.
🧹 Proposed cleanup
-from .training_utils import ( # noqa: F401 +from .training_utils import ( @@ -from .dataset_ops import ( # noqa: F401 +from .dataset_ops import ( @@ -from .preprocess import ( # noqa: F401 +from .preprocess import ( @@ -from .lora_training import ( # noqa: F401 +from .lora_training import ( @@ -from .lokr_training import ( # noqa: F401 +from .lokr_training import (acestep/ui/gradio/events/training/training_utils.py-22-62 (1)
22-62:⚠️ Potential issue | 🟡 MinorAdd missing type hints for new helpers.
create_dataset_buildershould declare its return type, and_safe_joinshould acceptOptional[str]since it handlesNone.As per coding guidelines, "Add type hints for new/modified functions when practical in Python".🧾 Suggested typing updates
-from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, TYPE_CHECKING @@ -def create_dataset_builder(): +if TYPE_CHECKING: + from acestep.training.dataset_builder import DatasetBuilder + +def create_dataset_builder() -> "DatasetBuilder": @@ -def _safe_join(base_root: str, user_path: str) -> Optional[str]: +def _safe_join(base_root: str, user_path: Optional[str]) -> Optional[str]:acestep/ui/gradio/events/training/preprocess.py-174-176 (1)
174-176:⚠️ Potential issue | 🟡 MinorMark
output_pathsas intentionally unused.Ruff flags this; prefixing with
_clarifies intent.🧹 Cleanup
- output_paths, status = builder_state.preprocess_to_tensors( + _output_paths, status = builder_state.preprocess_to_tensors(acestep/ui/gradio/events/training/training_facade_test.py-41-56 (1)
41-56:⚠️ Potential issue | 🟡 MinorAdd docstrings for the test methods.
The new test functions are missing docstrings.
✅ Example docstrings
def test_all_symbols_present(self): + """Ensure each expected symbol is exposed on the facade.""" for name in EXPECTED_SYMBOLS: @@ def test_callable_symbols(self): + """Ensure re-exported symbols (except constants) are callable.""" callables = [s for s in EXPECTED_SYMBOLS if s != "SAFE_TRAINING_ROOT"]As per coding guidelines, "Docstrings are mandatory for all new or modified Python modules, classes, and functions".
acestep/ui/gradio/events/training_handlers.py-8-35 (1)
8-35:⚠️ Potential issue | 🟡 MinorRemove unused
# noqa: F401directives.Ruff reports these
noqadirectives as unused becauseF401isn’t enabled, so they add noise without effect.🧹 Suggested cleanup
-from .training.training_utils import ( # noqa: F401 +from .training.training_utils import ( @@ -from .training.dataset_ops import ( # noqa: F401 +from .training.dataset_ops import ( @@ -from .training.preprocess import ( # noqa: F401 +from .training.preprocess import ( @@ -from .training.lora_training import ( # noqa: F401 +from .training.lora_training import ( @@ -from .training.lokr_training import ( # noqa: F401 +from .training.lokr_training import (acestep/ui/gradio/events/training/preprocess.py-19-186 (1)
19-186:⚠️ Potential issue | 🟡 MinorAdd return type hint to
load_existing_dataset_for_preprocess().The function
load_existing_dataset_for_preprocess()(line 19) is missing a return type hint. Per coding guidelines, add type hints for new/modified functions when practical. The docstring documents a complex tuple return; consider using aTupletype hint or aTypedDictfor clarity.acestep/ui/gradio/events/results/batch_queue.py-13-152 (1)
13-152:⚠️ Potential issue | 🟡 MinorAdd type hints for the public batch-queue APIs.
These functions are used by multiple modules (results_handlers, batch_navigation, batch_management, and tests); type hints improve clarity for callers and maintainability. Per coding guidelines, "Add type hints for new/modified functions when practical in Python".
acestep/ui/gradio/events/results/batch_queue.py-79-139 (1)
79-139:⚠️ Potential issue | 🟡 MinorThe hard-coded values are intentional for AutoGen batch generation, but parameters should be removed from the signature.
The docstring explicitly states "Audio codes are cleared so AutoGen batches always generate fresh content." This function is used for next-batch generation in AutoGen workflows, which intentionally bypasses user input to ensure fresh randomization. However, accepting unused parameters (
random_seed_checkboxandtext2music_audio_code_string) in the signature is misleading—they should be removed since the function will never use them. Consider either:
- Remove the unused parameters from the function signature and update callers accordingly.
- If keeping the signature for interface compatibility, add a docstring note: "Parameters
random_seed_checkboxandtext2music_audio_code_stringare intentionally ignored; fresh generation requires randomization and cleared codes."acestep/ui/gradio/events/training/preprocess.py-163-168 (1)
163-168:⚠️ Potential issue | 🟡 MinorDon't swallow exceptions in
progress_callback. At least log the failure so UI issues don't disappear silently.🛠️ Suggested logging
def progress_callback(msg): if progress: try: progress(msg) - except Exception: - pass + except Exception as exc: + logger.warning(f"Progress callback failed: {exc}")Per coding guidelines: "Handle errors explicitly in Python; avoid bare
except" and "Uselogurulogger instead ofprint()for logging (except CLI output)". Theloggeris already imported in the file.acestep/ui/gradio/events/results/scoring.py-13-28 (1)
13-28:⚠️ Potential issue | 🟡 MinorAdd type hints to these public handler functions.
These functions are part of the public UI surface. The first function has 13 parameters and would particularly benefit from type hints for IDE support and readability. The second function's mixed return type (involving
gr.skip()) would also be clarified by explicit type annotations. This aligns with the coding guideline: "Add type hints for new/modified functions when practical in Python."Also applies to: 165–172
acestep/ui/gradio/events/generation/validation.py-80-88 (1)
80-88:⚠️ Potential issue | 🟡 MinorAdd parameter type hint to
_has_reference_audio.The function accepts multiple input types (None, str, list, tuple) and lacks a type hint. Use
Anyto match the pattern established elsewhere in this file (e.g., line 91) and satisfy the coding guideline requirement for type hints on modified functions.🔧 Suggested change
-def _has_reference_audio(reference_audio) -> bool: +def _has_reference_audio(reference_audio: Any) -> bool:acestep/ui/gradio/events/generation/model_config.py-30-45 (1)
30-45:⚠️ Potential issue | 🟡 MinorAdd explicit Optional types and return type hints for public functions.
The function
update_model_type_settingshas no type hints, andget_model_type_ui_settingsusescurrent_mode: str = Nonewhich is implicitly Optional. Both functions should explicitly declare parameter types and returntupleper coding guidelines.✍️ Suggested signature updates
-def update_model_type_settings(config_path, current_mode=None): +def update_model_type_settings( + config_path: str | None, + current_mode: str | None = None, +) -> tuple:-def get_model_type_ui_settings(is_turbo: bool, current_mode: str = None, is_pure_base: bool = False): +def get_model_type_ui_settings( + is_turbo: bool, + current_mode: str | None = None, + is_pure_base: bool = False, +) -> tuple:acestep/ui/gradio/events/results/scoring.py-78-89 (1)
78-89:⚠️ Potential issue | 🟡 MinorDon't swallow exceptions; log and narrow them.
Lines 78-89 and 224-236 contain bare
except Exception:patterns and useprint()instead of logging, violating the coding guidelines that require explicit, narrowed exception handling and use ofloguruinstead ofprint().Suggested fixes
import traceback import gradio as gr +from loguru import logger from acestep.ui.gradio.i18n import t @@ - if bpm is not None and 'bpm' not in metadata: + if bpm is not None and "bpm" not in metadata: try: - metadata['bpm'] = int(bpm) - except Exception: - pass - if caption and 'caption' not in metadata: - metadata['caption'] = caption - if audio_duration is not None and audio_duration > 0 and 'duration' not in metadata: + metadata["bpm"] = int(bpm) + except (TypeError, ValueError) as exc: + logger.debug(f"Invalid bpm '{bpm}': {exc}") + if caption and "caption" not in metadata: + metadata["caption"] = caption + if audio_duration is not None and audio_duration > 0 and "duration" not in metadata: try: - metadata['duration'] = int(audio_duration) - except Exception: - pass - if key_scale and key_scale.strip() and 'keyscale' not in metadata: - metadata['keyscale'] = key_scale.strip() - if vocal_language and vocal_language.strip() and 'language' not in metadata: - metadata['language'] = vocal_language.strip() - if time_signature and time_signature.strip() and 'timesignature' not in metadata: - metadata['timesignature'] = time_signature.strip() + metadata["duration"] = int(audio_duration) + except (TypeError, ValueError) as exc: + logger.debug(f"Invalid duration '{audio_duration}': {exc}") + if key_scale and key_scale.strip() and "keyscale" not in metadata: + metadata["keyscale"] = key_scale.strip() + if vocal_language and vocal_language.strip() and "language" not in metadata: + metadata["language"] = vocal_language.strip() + if time_signature and time_signature.strip() and "timesignature" not in metadata: + metadata["timesignature"] = time_signature.strip() @@ - except Exception as e: - print(f"Error slicing tensor data for score: {e}") + except (IndexError, KeyError, TypeError, AttributeError) as exc: + logger.warning(f"Error slicing tensor data for score: {exc}") extra_tensor_data = Noneacestep/ui/gradio/events/results/batch_management.py-108-112 (1)
108-112:⚠️ Potential issue | 🟡 MinorGuard
batch_size_inputtypes before numeric comparisons.
If the UI passes a string,>= 2will raise aTypeError. Cast once with a safe fallback before comparisons/slicing.🔧 Suggested fix
- if allow_lm_batch and batch_size_input >= 2: - codes_to_store = generated_codes_batch[:int(batch_size_input)] + try: + batch_size_val = int(batch_size_input) + except (TypeError, ValueError): + batch_size_val = 1 + if allow_lm_batch and batch_size_val >= 2: + codes_to_store = generated_codes_batch[:batch_size_val] @@ - batch_size = params.get("batch_size_input", 2) + try: + batch_size = int(params.get("batch_size_input", 2)) + except (TypeError, ValueError): + batch_size = 1 @@ - if allow_lm_batch_val and batch_size >= 2: - codes_to_store = generated_codes_batch[:int(batch_size)] + if allow_lm_batch_val and batch_size >= 2: + codes_to_store = generated_codes_batch[:batch_size]Also applies to: 274-279
acestep/ui/gradio/events/results/audio_transfer.py-136-145 (1)
136-145:⚠️ Potential issue | 🟡 MinorAvoid catch‑all exception handling here.
CatchingExceptionmasks unexpected failures; narrow to expected conversion errors and handle them explicitly.🔧 Suggested fix
- except Exception as e: + except (RuntimeError, ValueError, OSError) as e: gr.Warning(f"Error converting audio to codes: {e}") return gr.skip(), gr.skip()As per coding guidelines, "Handle errors explicitly in Python; avoid bare
except."acestep/ui/gradio/events/generation/mode_ui.py-334-343 (1)
334-343:⚠️ Potential issue | 🟡 MinorAvoid catch‑all exception handling here.
Narrow the exception to expected I/O/parsing errors so unexpected failures aren’t silently masked.🔧 Suggested fix
- except Exception as e: + except (OSError, ValueError, RuntimeError) as e: logger.warning(f"Failed to get audio duration for {mode} mode: {e}")As per coding guidelines, "Handle errors explicitly in Python; avoid bare
except."acestep/ui/gradio/events/training/lokr_training.py-224-227 (1)
224-227:⚠️ Potential issue | 🟡 MinorAvoid blanket exception swallowing when parsing epochs.
CatchingExceptionhides unexpected failures. Narrow toValueError(or let other exceptions surface) to keep errors explicit.🔧 Suggested fix
- except Exception: - continue + except ValueError: + continueAs per coding guidelines, "Handle errors explicitly in Python; avoid bare
except."acestep/ui/gradio/events/generation/ui_helpers.py-126-145 (1)
126-145:⚠️ Potential issue | 🟡 MinorReturn type annotation doesn’t match actual return.
update_instruction_uireturns a string, not a tuple. Align the annotation to avoid misleading type hints.🔧 Suggested fix
-def update_instruction_ui( +def update_instruction_ui( @@ -) -> tuple: +) -> str: @@ - return instruction + return instructionAs per coding guidelines, "Add type hints for new/modified functions when practical in Python."
acestep/ui/gradio/events/training/dataset_ops.py-99-104 (1)
99-104:⚠️ Potential issue | 🟡 MinorDon’t swallow progress callback failures silently.
At minimum, log the exception so UI issues are diagnosable, and avoid a bareexcept.🔧 Suggested fix
-import gradio as gr +import gradio as gr +from loguru import logger @@ - except Exception: - pass + except Exception as exc: + logger.warning("Progress callback failed: {}", exc)As per coding guidelines, "Handle errors explicitly in Python; avoid bare
except" and "Keep logging actionable in Python; avoid noisy logs andacestep/ui/gradio/events/results/batch_management.py-311-319 (1)
311-319:⚠️ Potential issue | 🟡 MinorNarrow the catch‑all in background generation.
CatchingExceptionhides unexpected failures; catch expected errors and log the stack for diagnostics.🔧 Suggested fix
- except Exception as e: + except (RuntimeError, ValueError, OSError) as e: + logger.exception("Background batch generation failed") error_msg = t("messages.batch_failed", error=str(e)) gr.Warning(error_msg)As per coding guidelines, "Handle errors explicitly in Python; avoid bare
except."acestep/ui/gradio/events/__init__.py-16-16 (1)
16-16:⚠️ Potential issue | 🟡 MinorSilence unused
demoparameter to satisfy lint.If
demois required by the signature, mark it intentionally unused.Suggested change
def setup_event_handlers(demo, dit_handler, llm_handler, dataset_handler, dataset_section, generation_section, results_section): """Setup event handlers connecting UI components and business logic""" + _ = demo @@ def setup_training_event_handlers(demo, dit_handler, llm_handler, training_section): """Setup event handlers for the training tab (dataset builder and LoRA training)""" + _ = demoAlso applies to: 1234-1234
acestep/ui/gradio/events/__init__.py-774-840 (1)
774-840:⚠️ Potential issue | 🟡 MinorAdd docstrings to the new helper functions.
make_score_handler,make_lrc_handler, andgeneration_wrappershould include concise docstrings describing purpose/inputs. As per coding guidelines "Docstrings are mandatory for all new or modified Python modules, classes, and functions".acestep/ui/gradio/events/__init__.py-1511-1524 (1)
1511-1524:⚠️ Potential issue | 🟡 MinorAdd docstrings and use the module-level logger in training wrappers.
These wrappers re-import
loggerand lack docstrings; use the existing module logger and add concise docstrings.As per coding guidelines "Docstrings are mandatory for all new or modified Python modules, classes, and functions".Suggested change
- def training_wrapper(tensor_dir, r, a, d, lr, ep, bs, ga, se, sh, sd, od, rc, ts): - from loguru import logger + def training_wrapper(tensor_dir, r, a, d, lr, ep, bs, ga, se, sh, sd, od, rc, ts): + """Stream training progress and capture errors for the UI.""" @@ - def lokr_training_wrapper( + def lokr_training_wrapper( tensor_dir, ldim, lalpha, factor, decompose_both, use_tucker, use_scalar, weight_decompose, lr, ep, bs, ga, se, sh, sd, od, ts, ): - from loguru import logger + """Stream LoKr training progress and capture errors for the UI."""Also applies to: 1581-1599
acestep/ui/gradio/events/results_handlers_facade_test.py-15-51 (1)
15-51:⚠️ Potential issue | 🟡 MinorAvoid mutable class attribute for
_EXPECTED_SYMBOLS.RUF012 flags this; a tuple is sufficient and avoids accidental mutation.
Suggested change
- _EXPECTED_SYMBOLS = [ + _EXPECTED_SYMBOLS = ( # constants "DEFAULT_RESULTS_DIR", "PROJECT_ROOT", # generation info "clear_audio_outputs_for_new_generation", "_build_generation_info", # LRC / VTT "parse_lrc_to_subtitles", "_format_vtt_timestamp", "lrc_to_vtt_file", "update_audio_subtitles_from_lrc", "save_lrc_to_file", "generate_lrc_handler", # batch queue "store_batch_in_queue", "update_batch_indicator", "update_navigation_buttons", "capture_current_params", "restore_batch_parameters", # scoring "calculate_score_handler", "calculate_score_handler_with_selection", # audio transfer "send_audio_to_src_with_metadata", "_extract_metadata_for_editing", "send_audio_to_remix", "send_audio_to_repaint", "convert_result_audio_to_codes", # generation "generate_with_progress", "generate_with_batch_management", "generate_next_batch_background", # navigation "navigate_to_previous_batch", "navigate_to_next_batch", - ] + )acestep/ui/gradio/interfaces/__init__.py-52-54 (1)
52-54:⚠️ Potential issue | 🟡 MinorDrop the unused
i18nassignment or make the side effect explicit.The
i18nvariable is assigned on line 53 but never used in the file. If the call is only for initialization side effects, use_ = get_i18n(language)or justget_i18n(language)to make this intent clear.Suggested fix
- i18n = get_i18n(language) + _ = get_i18n(language)acestep/ui/gradio/events/results/generation_info.py-25-27 (1)
25-27:⚠️ Potential issue | 🟡 MinorAdd a return type annotation to
clear_audio_outputs_for_new_generation.Line 25: The function lacks an explicit return type annotation. Per coding guidelines ("Add type hints for new/modified functions when practical in Python"), add
-> tuple[None, ...]to match the return value of 9 None elements and align with the type hints used elsewhere in the file.Suggested fix
-def clear_audio_outputs_for_new_generation(): +def clear_audio_outputs_for_new_generation() -> tuple[None, ...]:acestep/ui/gradio/interfaces/__init__.py-24-51 (1)
24-51:⚠️ Potential issue | 🟡 MinorAdd type hints to all parameters and switch to double quotes for string defaults.
Line 37: This public API function is missing type hints for all five parameters. Add
Any/Optionalannotations and switch thelanguagedefault from single to double quotes per PEP 8.Suggested fix
+from typing import Any, Optional + import gradio as gr @@ -def create_gradio_interface(dit_handler, llm_handler, dataset_handler, init_params=None, language='en') -> gr.Blocks: +def create_gradio_interface( + dit_handler: Any, + llm_handler: Any, + dataset_handler: Any, + init_params: Optional[dict[str, Any]] = None, + language: str = "en", +) -> gr.Blocks:acestep/ui/gradio/events/generation/metadata_loading.py-123-128 (1)
123-128:⚠️ Potential issue | 🟡 MinorNarrow the broad
except Exceptionblock to explicit exception types.Lines 126-128: The catch-all
Exceptionhandler masks unexpected failures. The operations in the try block (file I/O, JSON parsing, type conversions) can raise specific exceptions:OSError(file operations),ValueError(int/float conversions), andAttributeError(property access). Replace with explicit exception handling—e.g.,except (OSError, ValueError, AttributeError) as e:. If external calls may raise undocumented exceptions, consider catching those separately or re-raising as a last resort.acestep/ui/gradio/events/results/generation_progress.py-350-375 (1)
350-375:⚠️ Potential issue | 🟡 MinorReplace
logger.exceptionin the exception handler.Line 374 uses
print()for logging, which bypasses structured logging. Replace withlogger.exception()to maintain consistency with thelogurulogger already imported at the top of the file.Suggested fix
except Exception as e: - print(f"[Auto Score] Failed to prepare tensor data for sample {sample_idx}: {e}") + logger.exception( + f"[Auto Score] Failed to prepare tensor data for sample {sample_idx}: {e}" + ) return Noneacestep/ui/gradio/events/generation/llm_actions.py-16-28 (1)
16-28:⚠️ Potential issue | 🟡 MinorAdd type hints to all public functions in this module.
The file has multiple public functions without parameter and return type annotations:
analyze_src_audio,transcribe_audio_codes,handle_create_sample,handle_format_sample,handle_format_caption, andhandle_format_lyrics. Also apply to private helper functions_build_user_metadataand_convert_lm_params.Some functions have partial typing (e.g.,
handle_create_samplehas typed parameters but missing return annotation). Complete the coverage to make call sites explicit and improve maintainability.Example fix for analyze_src_audio
+from typing import Any -def analyze_src_audio(dit_handler, llm_handler, src_audio, constrained_decoding_debug=False): +def analyze_src_audio( + dit_handler: Any, + llm_handler: Any, + src_audio: str | None, + constrained_decoding_debug: bool = False, +) -> tuple[Any, ...]:Per coding guidelines: "Add type hints for new/modified functions when practical in Python".
acestep/ui/gradio/events/results/lrc_utils.py-287-289 (1)
287-289:⚠️ Potential issue | 🟡 MinorAvoid broad
except Exceptioningenerate_lrc_handler.The broad exception handler masks programming errors and suppresses intentional re-raises from
get_lyric_timestamp()(which explicitly re-raises unexpected exceptions at line 147 of lyric_timestamp.py). Catch specific exceptions from tensor operations (RuntimeError,ValueError,IndexError) and file I/O (OSError) instead, allowing unexpected failures to propagate for proper debugging.acestep/ui/gradio/events/generation/metadata_loading.py-20-26 (1)
20-26:⚠️ Potential issue | 🟡 MinorAdd type hints to the public loader/sampler functions.
Four public functions in this module lack complete type annotations:
load_metadata(line 20): missing parameter and return type hintsload_random_example(line 139): missingllm_handlerparameter type and return typesample_example_smart(line 227): missingllm_handlerparameter type and return typeload_random_simple_description(line 265): missing return typePer the coding guidelines, add type hints for these new/modified functions where practical. For
load_metadata, useAnyforfile_obj(accepts file object with.nameattribute or string path), and annotate the return type as the union of the list and tuple variants it returns.Suggested fix (example for load_metadata)
-from typing import Optional +from typing import Any, Optional @@ -def load_metadata(file_obj, llm_handler=None): +def load_metadata(file_obj: Any, llm_handler: Any | None = None) -> list[Any] | tuple[Any, ...]:
🧹 Nitpick comments (15)
acestep/training/data_module.py (2)
8-74: Import organization: standard library imports should be at the top.The
torchandtorchaudioimports at lines 61-63 are placed after the helper functions. This breaks PEP 8 import ordering conventions and creates an unusual code layout. Move all imports to the top of the file.♻️ Proposed fix: reorganize imports
import os import json import random from typing import Optional, List, Dict, Any, Tuple from loguru import logger + +import torch +import torchaudio +from torch.utils.data import Dataset, DataLoader + +try: + from lightning.pytorch import LightningDataModule + LIGHTNING_AVAILABLE = True +except ImportError: + LIGHTNING_AVAILABLE = False + logger.warning("Lightning not installed. Training module will not be available.") + # Create a dummy class for type hints + class LightningDataModule: + pass def _resolve_and_validate_dir(user_dir: str) -> str: # ... function body ... def _safe_child_path(base_dir: str, child: str) -> Optional[str]: # ... function body ... -import torch -import torchaudio -from torch.utils.data import Dataset, DataLoader - -try: - from lightning.pytorch import LightningDataModule - LIGHTNING_AVAILABLE = True -except ImportError: - LIGHTNING_AVAILABLE = False - logger.warning("Lightning not installed. Training module will not be available.") - # Create a dummy class for type hints - class LightningDataModule: - pass
1-6: Module exceeds LOC limit: consider splitting in a follow-up.At 569 LOC, this module significantly exceeds the 200 LOC hard cap. Consider splitting into separate modules in a future PR:
path_utils.pyfor_resolve_and_validate_dirand_safe_child_pathpreprocessed_dataset.pyforPreprocessedTensorDataset,PreprocessedDataModule, andcollate_preprocessed_batchraw_audio_dataset.pyfor the legacyAceStepTrainingDataset,AceStepDataModule, andcollate_training_batchSince this PR focuses on security fixes, this refactor can be deferred.
As per coding guidelines: "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
acestep/training/data_module_test.py (2)
71-90: Good integration test, but use context manager for file creation.The test logic is correct. Minor improvement: use a context manager or explicit close pattern for file creation to be consistent with best practices.
♻️ Suggested improvement
- good_pt = os.path.join(d, "good.pt") - open(good_pt, "wb").close() # touch + good_pt = os.path.join(d, "good.pt") + with open(good_pt, "wb"): + pass # touch
1-6: Consider adding a symlink traversal test.The current tests cover
..traversal and absolute paths outside the base, but don't test actual symlink-based attacks. A test that creates a symlink inside the base directory pointing outside would validate therealpathprotection more directly.♻️ Example symlink test (may need platform guards)
def test_symlink_escape_blocked(self): """Symlinks pointing outside base_dir are blocked.""" with tempfile.TemporaryDirectory() as base: with tempfile.TemporaryDirectory() as outside: base = os.path.realpath(base) link_path = os.path.join(base, "escape_link") try: os.symlink(outside, link_path) except OSError: self.skipTest("Symlinks not supported on this platform") result = _safe_child_path(base, "escape_link/file.pt") self.assertIsNone(result)acestep/ui/gradio/events/training/lora_training.py (2)
10-46: Add type hints + complete docstrings for public APIs (and markprogressas intentionally unused).Docstrings should include key inputs/outputs/exceptions, and type hints should be added where practical. Also,
progressis unused; if it’s intentionally part of the Gradio signature, mark it explicitly.Suggested update (example for
start_training)-from typing import Dict, Tuple +from typing import Any, Dict, Generator, Optional, Tuple @@ -def start_training( +def start_training( tensor_dir: str, dit_handler, lora_rank: int, @@ resume_checkpoint_dir: str, - training_state: Dict, - progress=None, -): + training_state: Dict[str, Any], + progress: Optional[object] = None, +) -> Generator[Tuple[str, str, Any, Dict[str, Any]], None, None]: """Start LoRA training from preprocessed tensors. - This is a generator function that yields progress updates as - (status, log_text, plot_figure, training_state) tuples. + Args: + tensor_dir: Preprocessed tensor directory. + dit_handler: Initialized DiT handler. + training_state: Mutable training state used by the UI. + Yields: + (status, log_text, plot_figure, training_state). + Raises: + ImportError: If required training dependencies are missing. """ + _ = progress # Reserved for Gradio progress callback.Please apply similar docstring additions to
stop_trainingandexport_lora.
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)”; “Add type hints for new/modified functions when practical in Python”.Also applies to: 237-255
231-235: Narrow exception handling to explicit failures.Catching
Exceptionmasks unexpected bugs and conflicts with the explicit error-handling guideline. Prefer catching the expected error types and let unexpected exceptions propagate.Example narrowing
-import time +import time +import shutil @@ - except Exception as e: + except (ImportError, RuntimeError, OSError, ValueError) as e: logger.exception("Training error") training_state["is_training"] = False - yield f"❌ Error: {str(e)}", str(e), _training_loss_figure({}, [], []), training_state + yield f"❌ Error: {e}", str(e), _training_loss_figure({}, [], []), training_state @@ - try: - import shutil + try: @@ - except Exception as e: + except (ImportError, OSError, shutil.Error, ValueError) as e: logger.exception("Export error") return t("training.export_failed", error=str(e))As per coding guidelines: “Handle errors explicitly in Python; avoid bare
except”.Also applies to: 294-296
acestep/ui/gradio/__init__.py (1)
2-2: Remove unusednoqadirective.Static analysis indicates F401 is not enabled in this project's Ruff configuration, making the
# noqa: F401comment unnecessary.🧹 Proposed fix
-from acestep.ui.gradio.interfaces import create_gradio_interface # noqa: F401 +from acestep.ui.gradio.interfaces import create_gradio_interfaceacestep/ui/gradio/api/__init__.py (1)
2-2: Remove unusednoqadirective.Same as the other
__init__.pyfiles — F401 is not enabled in the Ruff configuration.🧹 Proposed fix
-from acestep.ui.gradio.api.api_routes import setup_api_routes # noqa: F401 +from acestep.ui.gradio.api.api_routes import setup_api_routesacestep/ui/gradio/events/training/dataset_ops_test.py (1)
6-11: Unused import:save_sample_editis imported but not tested.The function
save_sample_editis imported but has no corresponding test class or methods. Either add test coverage or remove the unused import.Option A: Remove unused import
from acestep.ui.gradio.events.training.dataset_ops import ( get_sample_preview, - save_sample_edit, update_settings, save_dataset, )Option B: Add test coverage (preferred)
class TestSaveSampleEdit(unittest.TestCase): """Tests for save_sample_edit.""" def test_none_builder_returns_error(self): result = save_sample_edit(0, "caption", "genre", "lyrics", 120, "C", "4/4", "en", False, "caption", None) self.assertIn("❌", result) def test_valid_sample_updates_fields(self): sample = MagicMock() builder = MagicMock() builder.samples = [sample] result = save_sample_edit(0, "new caption", "rock", "lyrics", 120, "C", "4/4", "en", False, "caption", builder) self.assertIn("✅", result)As per coding guidelines: "Add or update tests for every behavior change and bug fix."
acestep/ui/gradio/i18n/i18n.py (1)
25-32: Dead code: directory existence check is now unreachable.Since
i18n_diris nowos.path.dirname(__file__)(the directory containing this module), it will always exist when this code executes — the module couldn't have been imported otherwise. Theos.makedirsbranch at lines 31-32 is unreachable.🧹 Remove unreachable code
def _load_all_translations(self): """Load all translation files from i18n directory.""" current_file = os.path.abspath(__file__) # JSON files live alongside this module in the same directory i18n_dir = os.path.dirname(current_file) - - if not os.path.exists(i18n_dir): - os.makedirs(i18n_dir) - return # Load all JSON files in i18n directory for filename in os.listdir(i18n_dir):acestep/ui/gradio/i18n/__init__.py (1)
2-2: Remove unusednoqadirective.Consistent with other
__init__.pyfiles in this PR — F401 is not enabled in the Ruff configuration.🧹 Proposed fix
-from acestep.ui.gradio.i18n.i18n import I18n, get_i18n, t # noqa: F401 +from acestep.ui.gradio.i18n.i18n import I18n, get_i18n, tacestep/ui/gradio/interfaces/training.py (1)
15-834: Advisory: File exceeds module size guideline.At 834 LOC, this file significantly exceeds the 200 LOC hard cap. Consider decomposing
create_training_sectioninto smaller functions in a future iteration — e.g., extract each tab's UI construction into separate helper functions like_create_dataset_builder_tab(),_create_training_tab(), and_create_lokr_tab().This is not blocking for this refactor PR since the import changes are the only modifications here.
acestep/ui/gradio/events/results/batch_navigation.py (1)
21-180: Add type hints for navigation APIs and the helper.These are public handlers; type hints will clarify the generator return contract and parameter shapes.
As per coding guidelines, "Add type hints for new/modified functions when practical in Python".
acestep/ui/gradio/events/results/audio_transfer.py (1)
60-117: Deduplicate Remix/Repaint handlers.
These two functions are nearly identical; extracting a shared helper would reduce drift and keep the logic focused. As per coding guidelines, "Keep functions focused and short in Python; extract helpers instead of nesting complexity."acestep/ui/gradio/events/results_handlers.py (1)
75-111: Keep__all__sorted for stable exports.Line 75-111: Ruff RUF022 flags the unsorted
__all__list. Sorting improves diff stability and reduces merge churn.
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
acestep/training/lora_utils.py (1)
1-10: 🛠️ Refactor suggestion | 🟠 MajorModule exceeds 200 LOC; split into smaller modules.
This file is well beyond the hard cap; consider separating LoRA injection, IO, and info helpers into dedicated modules.As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
🤖 Fix all issues with AI agents
In `@acestep/training/data_module_test.py`:
- Around line 104-117: The two test methods test_nonexistent_file_raises and
test_valid_json in the LoadDatasetFromJsonTests need concise docstrings; add a
one-line docstring to each method describing the behavior under test and the
expected outcome (e.g., test_nonexistent_file_raises: "Ensure
load_dataset_from_json raises ValueError for missing file"; test_valid_json:
"Verify valid JSON yields one sample and correct metadata"), include mention of
raised exception for the first and the key outputs for the second to satisfy the
docstring guidelines.
- Around line 93-96: Replace the hard-coded "/tmp" usage in
test_nonexistent_dir_raises (and the other test that constructs a "/tmp/..."
path) with a temporary directory from Python's tempfile module: create a
TemporaryDirectory (or mkdtemp) as the base, build a guaranteed-nonexistent
child path under that base (e.g., join base with a unique name), and pass that
path to PreprocessedTensorDataset; ensure the temporary directory is cleaned up
(use context manager TemporaryDirectory in the test) so the test is
deterministic and platform-independent.
- Around line 63-96: Add concise docstrings to the three test methods in
PreprocessedTensorDatasetPathSafetyTests: test_manifest_traversal_paths_skipped,
test_fallback_scan_only_finds_pt_files, and test_nonexistent_dir_raises. Each
docstring should state the test purpose, key inputs (e.g., temporary directory
contents or absent manifest), expected outputs/behavior (e.g., only safe paths
retained, only .pt files found, or ValueError raised), and for
test_nonexistent_dir_raises explicitly note the raised ValueError; keep each
docstring one or two short sentences.
- Around line 23-53: Add concise docstrings to each test method in SafePathTests
(test_valid_directory, test_traversal_raises, test_absolute_path_outside_raises,
test_normal_child, test_absolute_path_inside_allowed) that state the purpose of
the test, the key inputs (e.g., temporary directory or path passed as base), the
expected output or behavior (returned safe path or ValueError raised), and any
exceptions expected to be raised; ensure they are brief one- or two-line
docstrings following project docstring conventions.
In `@acestep/training/dataset_builder_modules/csv_metadata.py`:
- Around line 10-18: The function load_csv_metadata now lets safe_path() raise
ValueError for invalid/unsafe directories which breaks callers expecting an
empty dict; wrap the call to safe_path(directory) in a try/except ValueError (or
broader if safe_path documents other exceptions) and on exception return {} to
preserve the previous graceful fallback, then proceed to build csv_files and the
rest of load_csv_metadata as before; reference the validated_dir assignment and
csv_files population so you modify around the validated_dir =
safe_path(directory) line to handle the error and return an empty metadata dict.
In `@acestep/training/path_safety.py`:
- Around line 68-77: The current sanitiser check `if not
normalised.startswith(root + os.sep) and normalised != root:` fails when root is
"/" because `root + os.sep` becomes "//"; change the prefix logic to compute a
safe_prefix (e.g. safe_prefix = root if root == os.sep else root + os.sep) and
use `if not normalised.startswith(safe_prefix) and normalised != root:` (locate
and update the check using the variables normalised, root, and user_path in the
function in path_safety.py).
In `@acestep/ui/gradio/events/training/lokr_training.py`:
- Around line 1-10: This module exceeds the size cap; split responsibilities by
extracting the training runner logic and export helpers into separate modules:
move the code that starts/controls training into a new trainings/runner.py
(functions like start_lokr_training or run_training_loop) and move
checkpoint/export utilities into trainings/export.py (functions like
list_checkpoint_epochs and export_trained_weights), keep only high-level UI
handler wrappers in lokr_training.py and update imports to re-export or call the
new modules; ensure you update any relative imports, add small unit tests or
type hints for the new functions, and adjust package __init__ if needed so
existing callers of start_lokr_training, list_checkpoint_epochs, and
export_trained_weights keep working.
- Around line 41-45: Update the concise docstrings for the generator function
that begins with "Start LoKr training from preprocessed tensors" (likely
start_lokr_training) to include: a short purpose line, explicit parameters with
types and meanings (e.g., tensors, hyperparams, callbacks), the yield format and
types (status, log_text, plot_figure, training_state), any return behavior, and
any exceptions that can be raised; also apply the same pattern to the other two
functions with missing details (docstrings at the regions around lines 203-207
and 262-266) so each docstring lists purpose, inputs (names/types),
outputs/returns or yielded values, and raised exceptions. Ensure wording is
concise and follows project docstring guidelines.
- Around line 238-240: Replace the broad "except Exception" that follows the
epoch parsing (the block that sets epoch_num = int(d.split("_")[1])) with a
narrow exception handler: catch ValueError (and optionally IndexError if the
split may be missing parts) so only expected parse errors are handled; update
both occurrences that parse epoch_num (the int(d.split("_")[1]) sites around the
epoch parsing blocks) to use "except ValueError:" (or "except (ValueError,
IndexError):") and leave other exceptions to propagate.
- Around line 178-194: The generator currently yields a stop message when
training_state.get("should_stop") is true but then continues to the completion
path; after yielding the stop tuple (the line that yields "ℹ️ Stopped
({time_info})", "\n".join(log_lines[-15:]), plot_figure, training_state), return
immediately to prevent falling through to the completion logic—i.e., replace the
break (or add a return) so the function exits right after that yield, ensuring
training_state["is_training"] is set to False and no completion_msg is emitted.
- Around line 114-128: Validate and sanitize the user-controlled lokr_output_dir
with safe_path before passing it into TrainingConfig: ensure you call
safe_path(lokr_output_dir) (or equivalent sanitization) and use the returned
safe path for the output_dir field in the TrainingConfig constructor (refer to
lokr_output_dir and TrainingConfig in this diff); replace the raw
lokr_output_dir with the sanitized value and handle any errors from safe_path
(e.g., reject/raise or fallback) before creating TrainingConfig.
In `@acestep/ui/gradio/events/training/lora_training.py`:
- Around line 302-304: The current broad except in the export path (the except
Exception around logger.exception("Export error") and return
t("training.export_failed", error=str(e))) swallows unexpected errors; change it
to catch the specific filesystem/export-related exceptions you expect (e.g.,
OSError, shutil.Error) and handle them by logging and returning the translated
error, and re-raise any other exceptions so they surface during runtime; update
the exception clause around the export logic (the block that calls
logger.exception and returns t("training.export_failed", error=str(e))) to only
catch those concrete exceptions and let others propagate.
- Around line 237-240: Narrow the broad except in the training loop by catching
only the expected exception types from the trainer/config path (for example
ValueError, RuntimeError, IOError, or the specific TrainerError class if one
exists) instead of "except Exception"; inside each specific except block, call
logger.exception("Training error") and set training_state["is_training"] = False
and yield the error tuple as currently done; for any other unexpected exceptions
re-raise after logging so they surface (i.e., use an except (ExpectedError1,
ExpectedError2) block then a bare "except Exception as e: logger.exception(...);
training_state[...] = False; raise").
- Around line 23-236: start_training is too large and mixes validation,
dependency checks, device-default selection, resume-path resolution, and the
training loop; split it into small helpers: extract input validation into
validate_training_inputs(tensor_dir, dit_handler, training_state) that performs
path/safety/model checks and yields early messages, move the dependency
import/check into ensure_training_dependencies(), move device-config selection
logic (device_type detection and
num_workers/pin_memory/prefetch_factor/mixed_precision/persistent_workers) into
get_device_loader_defaults(dit_handler) and return a dict used to build
TrainingConfig, extract resume_checkpoint resolution into
resolve_resume_path(resume_checkpoint_dir), and extract the loop that iterates
trainer.train_from_preprocessed into run_training_loop(trainer, tensor_dir,
training_state, start_time, step_list, loss_list) which yields progress tuples
and handles stop/training_failed logic; keep small orchestration in
start_training that calls these helpers and constructs
LoRATrainer/TrainingConfig (refer to symbols: start_training,
validate_training_inputs, ensure_training_dependencies,
get_device_loader_defaults, resolve_resume_path, run_training_loop, LoRATrainer,
TrainingConfig, safe_path, _training_loss_figure) and consider moving these
helpers to a sibling module to reduce this file under the 200-LOC cap.
- Around line 205-208: The code appends the original status object to log_lines
and then joins into log_text, which can raise TypeError if status isn't a
string; change the append to use the stringified value (e.g., append status_text
or str(status)) instead of status so log_lines always contains strings before
creating log_text; update the block around log_lines.append(status) to append
the stringified variable and keep the existing trimming logic.
- Around line 217-235: The stop branch currently yields a stopped message then
breaks out of the loop which allows execution to continue and emit the
completion messages; modify the handler around training_state.get("should_stop",
False) so that after setting training_state["is_training"] = False and yielding
the stopped tuple (the same as the current yield that returns "ℹ️ Stopped"), you
immediately return from the function to prevent falling through to the final
completion/failure logic; ensure you still append the stop message to log_lines
and keep use of _format_duration/time_info and plot_figure consistent.
- Around line 23-45: Add precise type hints to start_training (e.g., annotate
tensor_dir: str, dit_handler: appropriate handler type or Protocol, numeric
params as int/float, lora_output_dir/resume_checkpoint_dir: str, training_state:
Dict[str, Any], and progress: Optional[Callable] or Optional[Iterator]) and give
the function a generator return type (e.g., Generator[Tuple[str, str,
Optional[Figure], Dict[str, Any]], None, None]). Expand the docstring to
concisely describe purpose, key inputs (mention tensor_dir, dit_handler,
lora_rank/lora_alpha/lora_dropout, learning_rate, train_epochs,
train_batch_size, gradient_accumulation, save_every_n_epochs, training_shift,
training_seed, lora_output_dir, resume_checkpoint_dir, training_state, and
progress), key outputs (the yielded (status, log_text, plot_figure,
training_state) tuples) and any raised exceptions; finally either wire the
progress parameter (call it with updates) or explicitly state in the docstring
that progress is optional and currently unused to suppress lint warnings.
In `@acestep/ui/gradio/events/training/preprocess.py`:
- Around line 174-179: The progress_callback currently swallows all exceptions
(in function progress_callback referencing progress) which hides errors; change
the except block to capture the exception (e.g., except Exception as e) and log
it at debug level (preferably using the module logger or Python logging with
exc_info=True / logging.exception) instead of silently passing so diagnostic
information is preserved while still not breaking flow.
🧹 Nitpick comments (13)
acestep/ui/gradio/events/training/training_utils.py (3)
7-15: Remove unused imports and unnecessarynoqadirective.
reandtimeare imported but never used in this module. Additionally, the# noqa: E402directive is unnecessary as indicated by static analysis.🧹 Proposed cleanup
import os -import re -import time from typing import Any, Dict, List, Optional import gradio as gr import matplotlib matplotlib.use("Agg") -import matplotlib.pyplot as plt # noqa: E402 +import matplotlib.pyplot as plt
23-29: Consider adding return type hint.The function is missing a return type annotation. Since
DatasetBuilderis lazily imported, a string annotation avoids the import at module level.✨ Proposed fix
-def create_dataset_builder(): +def create_dataset_builder() -> "DatasetBuilder": """Create a new DatasetBuilder instance. Lazy import to avoid heavy module load at startup. """ from acestep.training.dataset_builder import DatasetBuilder return DatasetBuilder()
77-81: Return typeOptional[Any]is misleading—function always returns a figure.The function returns a matplotlib
Figurein all code paths (both the empty plot case and the populated plot case), neverNone. TheOptionalwrapper suggests the caller must handle aNonecase that cannot occur.✨ Proposed fix
def _training_loss_figure( training_state: Dict, step_list: List[int], loss_list: List[float], -) -> Optional[Any]: +) -> Any: """Build a training/validation loss plot (matplotlib Figure) for ``gr.Plot``."""acestep/ui/gradio/events/training/preprocess.py (4)
1-244: Module exceeds 200 LOC hard cap (244 lines).Consider extracting helper functions from
load_existing_dataset_for_preprocessto reduce module size:
- Extract preview tuple construction (lines 99-126) into a
_build_sample_preview()helper.- Extract UI updates tuple construction (lines 128-134) into a
_build_metadata_updates()helper.- Extract the error return tuple construction into a
_error_result()helper to reduce repetition across lines 44-49, 55-60, 66-71, 81-86.As per coding guidelines, "Target module size: optimal <= 150 LOC, hard cap 200 LOC".
44-49: Consider using iterable unpacking for cleaner tuple construction.The repeated tuple concatenation pattern can be simplified using iterable unpacking.
♻️ Proposed refactor using iterable unpacking
- updates = (gr.update(), gr.update(), gr.update(), gr.update(), gr.update()) - return ( - "❌ Please enter a dataset path", - [], - _safe_slider(0, value=0, visible=False), - builder_state, - ) + empty_preview + updates + updates = (gr.update(), gr.update(), gr.update(), gr.update(), gr.update()) + return ( + "❌ Please enter a dataset path", + [], + _safe_slider(0, value=0, visible=False), + builder_state, + *empty_preview, + *updates, + )Apply similar pattern to lines 55-60, 66-71, 81-86, and 136-141.
186-186: Unused variableoutput_pathsshould be prefixed with underscore.The return value is unpacked but only
statusis used.♻️ Proposed fix
- output_paths, status = builder_state.preprocess_to_tensors( + _output_paths, status = builder_state.preprocess_to_tensors(
219-234: Consider restructuring try-except for clarity.Moving the success return to an
elseblock makes the control flow clearer and separates happy path from error handling.♻️ Proposed refactor
try: with open(manifest_path, "r") as f: manifest = json.load(f) - - num_samples = manifest.get("num_samples", 0) - metadata = manifest.get("metadata", {}) - name = metadata.get("name", "Unknown") - custom_tag = metadata.get("custom_tag", "") - - info = f"📂 Loaded preprocessed dataset: {name}\n" - info += f"🔢 Samples: {num_samples} preprocessed tensors\n" - info += f"🏷️ Custom Tag: {custom_tag or '(none)'}" - - return info - except Exception as e: + except (OSError, json.JSONDecodeError) as e: logger.warning(f"Failed to read manifest: {e}") + else: + num_samples = manifest.get("num_samples", 0) + metadata = manifest.get("metadata", {}) + name = metadata.get("name", "Unknown") + custom_tag = metadata.get("custom_tag", "") + + info = f"📂 Loaded preprocessed dataset: {name}\n" + info += f"🔢 Samples: {num_samples} preprocessed tensors\n" + info += f"🏷️ Custom Tag: {custom_tag or '(none)'}" + + return infoacestep/training/trainer.py (1)
506-508: Consider handling ValueError from safe_path in init.
safe_path()onoutput_dirin__init__will raiseValueErrorif the path escapes the safe root. This happens at construction time, which may be unexpected for callers. Consider whether to catch and convert to a more descriptive exception or document this behavior.The CodeQL warnings on lines 552 and 1239 are false positives—paths are validated via
safe_path()before theos.path.isdir()checks.acestep/training/dataset_builder_modules/audio_io.py (1)
10-16: Misleading docstring contradicts implementation.The docstring states the path is "Already-validated" but line 16 calls
safe_path()to validate it. Either remove the redundant validation or update the docstring.📝 Proposed docstring fix
def _read_text_file(path: str) -> Tuple[str, bool]: - """Read a text file; return (content.strip(), True) if present and non-empty. - - Args: - path: Already-validated file path. - """ + """Read a text file; return (content.strip(), True) if present and non-empty.""" validated = safe_path(path)acestep/training/path_safety.py (2)
16-16: Unused import:loggeris never used.The
logurulogger is imported but no logging statements exist in this module.🧹 Remove unused import
-from loguru import logger
30-31: Remove stalenoqacomments.Ruff indicates the
# noqa: PLW0603on line 30 and# noqa: SIM115on line 97 are unused directives.🧹 Proposed cleanup
- global _SAFE_ROOT # noqa: PLW0603 + global _SAFE_ROOT- return open(validated, mode, **kwargs) # noqa: SIM115 + return open(validated, mode, **kwargs)acestep/training/dataset_builder_modules/serialization.py (1)
31-42: Consider specific handling for ValueError in save_dataset.The
safe_path()call is inside the try block, so aValueErrorfrom path validation gets caught by the genericExceptionhandler and produces a less specific error message. Theload_datasetmethod handles this more gracefully.📝 Proposed improvement
+ try: + validated_output = safe_path(output_path) + except ValueError: + return f"❌ Rejected unsafe output path: {output_path}" + try: - validated_output = safe_path(output_path) parent = os.path.dirname(validated_output)acestep/ui/gradio/events/training/lokr_training.py (1)
20-195: Breakstart_lokr_traininginto smaller helpers.
The function handles validation, config construction, runtime setup, training loop, and UI messaging in one block. Extract helpers to reduce complexity and improve testability.As per coding guidelines, "Keep functions focused and short in Python; extract helpers instead of nesting complexity."
| def test_nonexistent_dir_raises(self): | ||
| with self.assertRaises(ValueError): | ||
| PreprocessedTensorDataset("/tmp/nonexistent_xyz_12345") | ||
|
|
There was a problem hiding this comment.
Avoid hard‑coded /tmp paths in tests (breaks on Windows).
These tests can fail on Windows where /tmp doesn't exist. Build missing paths from a temp directory instead.
✅ Suggested fix
@@
- with self.assertRaises(ValueError):
- PreprocessedTensorDataset("/tmp/nonexistent_xyz_12345")
+ with tempfile.TemporaryDirectory() as d:
+ missing_dir = os.path.join(d, "nonexistent_xyz_12345")
+ with self.assertRaises(ValueError):
+ PreprocessedTensorDataset(missing_dir)
@@
- with self.assertRaises(ValueError):
- load_dataset_from_json("/tmp/nonexistent_file.json")
+ with tempfile.TemporaryDirectory() as d:
+ missing_file = os.path.join(d, "nonexistent_file.json")
+ with self.assertRaises(ValueError):
+ load_dataset_from_json(missing_file)Also applies to: 104-107
🧰 Tools
🪛 Ruff (0.15.0)
[error] 95-95: Probable insecure usage of temporary file or directory: "/tmp/nonexistent_xyz_12345"
(S108)
🤖 Prompt for AI Agents
In `@acestep/training/data_module_test.py` around lines 93 - 96, Replace the
hard-coded "/tmp" usage in test_nonexistent_dir_raises (and the other test that
constructs a "/tmp/..." path) with a temporary directory from Python's tempfile
module: create a TemporaryDirectory (or mkdtemp) as the base, build a
guaranteed-nonexistent child path under that base (e.g., join base with a unique
name), and pass that path to PreprocessedTensorDataset; ensure the temporary
directory is cleaned up (use context manager TemporaryDirectory in the test) so
the test is deterministic and platform-independent.
| except Exception as e: | ||
| logger.exception("Training error") | ||
| training_state["is_training"] = False | ||
| yield f"❌ Error: {str(e)}", str(e), _training_loss_figure({}, [], []), training_state |
There was a problem hiding this comment.
Narrow the except Exception to expected failure modes.
Catching all exceptions makes unexpected failures harder to detect and violates the explicit error‑handling guideline. Please catch the expected error types from the trainer/config path (and consider re‑raising unexpected exceptions after logging).
As per coding guidelines, "Handle errors explicitly in Python; avoid bare except".
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 237-237: Do not catch blind exception: Exception
(BLE001)
[warning] 240-240: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In `@acestep/ui/gradio/events/training/lora_training.py` around lines 237 - 240,
Narrow the broad except in the training loop by catching only the expected
exception types from the trainer/config path (for example ValueError,
RuntimeError, IOError, or the specific TrainerError class if one exists) instead
of "except Exception"; inside each specific except block, call
logger.exception("Training error") and set training_state["is_training"] = False
and yield the error tuple as currently done; for any other unexpected exceptions
re-raise after logging so they surface (i.e., use an except (ExpectedError1,
ExpectedError2) block then a bare "except Exception as e: logger.exception(...);
training_state[...] = False; raise").
| except Exception as e: | ||
| logger.exception("Export error") | ||
| return t("training.export_failed", error=str(e)) |
There was a problem hiding this comment.
Avoid broad exception swallowing during export.
except Exception hides unexpected filesystem errors. Prefer catching the concrete OSError/shutil.Error cases you expect and let other issues surface so they can be fixed.
As per coding guidelines, "Handle errors explicitly in Python; avoid bare except".
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 302-302: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In `@acestep/ui/gradio/events/training/lora_training.py` around lines 302 - 304,
The current broad except in the export path (the except Exception around
logger.exception("Export error") and return t("training.export_failed",
error=str(e))) swallows unexpected errors; change it to catch the specific
filesystem/export-related exceptions you expect (e.g., OSError, shutil.Error)
and handle them by logging and returning the translated error, and re-raise any
other exceptions so they surface during runtime; update the exception clause
around the export logic (the block that calls logger.exception and returns
t("training.export_failed", error=str(e))) to only catch those concrete
exceptions and let others propagate.
This project is a local Gradio desktop application, not an internet-facing web service. CodeQL's py/path-injection query flags Gradio UI component values (gr.Textbox etc.) as 'user-provided' taint sources, but: 1. The user IS the local operator — there is no remote attacker surface. 2. All user paths are validated through safe_path() (normpath+startswith), but CodeQL cannot trace sanitisers across function boundaries. Exclude acestep/training/, acestep/ui/, and vendored third-party code from path-injection analysis to suppress these confirmed false positives.
1 participant
Summary by CodeRabbit
New Features
Improvements