Skip to content

Localization 、training and More#19

Open
sdbds wants to merge 15 commits intofspecii:mainfrom
sdbds:localization
Open

Localization 、training and More#19
sdbds wants to merge 15 commits intofspecii:mainfrom
sdbds:localization

Conversation

@sdbds
Copy link

@sdbds sdbds commented Feb 5, 2026

No description provided.

sdbds added 7 commits February 4, 2026 17:41
…calization

# Conflicts:
#	App.tsx
#	components/CreatePanel.tsx
#	components/Player.tsx
#	components/SettingsModal.tsx
#	server/src/services/acestep.ts
#	server/src/services/cleanup.ts
@fspecii
Copy link
Owner

fspecii commented Feb 5, 2026

Hi @sdbds, thanks for this impressive contribution! There's a lot of great work here — i18n support, training UI, model selection, and many UI improvements. Really appreciate the effort.

I've done a thorough review and wanted to share some findings before we merge. The PR is quite large (7,000+ lines, 38 files), so I'd like to work through a few things together.


Critical Items to Address

1. Hardcoded Chinese string in Player.tsx

There are two places where '正常' (meaning "normal") is used directly instead of going through the i18n system:

{rate === 1.0 ? '正常' : `${rate}x`}

This will show Chinese text to English/Japanese/Korean users. Should use t('normal') or similar.

2. use_adg: false hardcoded in generate.ts

use_adg: false, // Disable ADG to avoid dimension mismatch with LoRA

This disables ADG for all generation requests, not just when LoRA is loaded. This silently changes behavior for every user. Could we make this conditional on whether LoRA is actually active?

3. Default language set to Chinese

return stored === 'zh' || stored === 'en' || ... ? stored : 'zh';

The fallback default is 'zh'. Since the existing userbase is primarily English-speaking, this would switch everyone's UI to Chinese on their first visit. Would you be open to changing the default to 'en'?

4. Duplicate getSamples in services/api.ts

The trainingApi object defines getSamples twice — the second definition silently overwrites the first. They have different return types, so one of them may not work as intended.

5. Memory leak in handleAutoLabel

The polling interval created in handleAutoLabel is never cleaned up if the component unmounts during labeling. The cleanup function is returned from the async handler but never captured. A small useEffect cleanup or AbortController pattern would fix this.


Suggestions

Generation API architecture change

The switch from Python subprocess spawning to HTTP proxy (port 8001) is a significant architecture change. The existing codebase has a fallback to subprocess when the API isn't available — this PR removes that fallback entirely. Could we keep the fallback for robustness?

Path sanitization on training/LoRA routes

The training and LoRA routes proxy client-provided filesystem paths directly to the backend without any validation. For a local single-user setup this is fine, but adding basic path validation (e.g., ensuring paths stay within the project directory) would be a good safety measure.

Batch size / inference steps limits

Batch size max was raised from 4 to 8, and inference steps from 32 to 200. These higher values could cause OOM on consumer GPUs. Maybe we could keep the UI limits a bit more conservative, or add a warning when users select very high values?


Recommendation

This is a really substantial contribution with a lot of value. To make it easier to review and merge safely, would you consider splitting it into 2-3 smaller PRs? For example:

  1. i18n system — translations + context + component updates (with default language set to en)
  2. Training + LoRA — training panel, LoRA routes, dataset management
  3. Model selection + UI improvements — model dropdown, genre selector, sidebar improvements, etc.

This way we can merge the non-controversial parts quickly and give the bigger changes (like the generation API rewrite) the focused attention they deserve.

Either way, thanks again for all this work — looking forward to getting it merged! 🙏

@sdbds
Copy link
Author

sdbds commented Feb 6, 2026

Thank you for your reply!
This PR is indeed very large, because initially I only intended to do some localization work, but I found some bugs during use and fixed them, and then started modifying the training part.
It is indeed possible to separate them. The training part involves backend modifications (API part), so I think the merging might be slower. We can finish the localization translation part first.

Critical Items to Address
1、2、4、5 fixed
3. Because it was my local environment, I forgot to change it; it has now been changed to en.

Generation API architecture change
I want to specifically point out this, because the local api_server initializes the model by default upon startup, and I found that using a Python subprocess (simple_generate) resulted in higher GPU memory usage. From this perspective, I think switching entirely to remote 8001 settings might be better than creating another warper locally; otherwise, we would have to unload the initialized model every time the api_server starts.
Furthermore, I believe that front-end and back-end separation is necessary, so that if the community builds a public server, it may be available to more people.

Path sanitization on training/LoRA routes
I'm currently modifying the official code for this part, so it may take some time.

Batch size / inference steps limits
After the api_server starts, it will automatically detect the GPU and provide recommended configurations. Perhaps we can accept the recommended configurations from the backend and use them.

Additionally, this project currently only has one main branch. Considering this, we hope to create a new dev branch for testing this pull request and for further modifications.

sdbds added 7 commits February 6, 2026 12:24
…calization

# Conflicts:
#	App.tsx
#	components/CreatePanel.tsx
#	components/LibraryView.tsx
#	components/SongDropdownMenu.tsx
#	components/SongList.tsx
#	server/scripts/format_sample.py
#	server/scripts/simple_generate.py
#	server/src/routes/generate.ts
#	server/src/routes/songs.ts
#	services/api.ts
@jonas-klesen
Copy link

jonas-klesen commented Feb 6, 2026

When using "scan" or "load" in the LoRa training, I get a 500 with {"error":"Request failed"}.

api.log says:
INFO: 127.0.0.1:55198 - "GET /v1/training/status HTTP/1.1" 404 Not Found INFO: 127.0.0.1:55198 - "POST /v1/dataset/scan HTTP/1.1" 404 Not Found

Using the newest AceStep repo version..


Also, all language options have a "vocal" prefix, e.g. vocalEnglish

@sdbds
Copy link
Author

sdbds commented Feb 6, 2026

When using "scan" or "load" in the LoRa training, I get a 500 with {"error":"Request failed"}.

api.log says: INFO: 127.0.0.1:55198 - "GET /v1/training/status HTTP/1.1" 404 Not Found INFO: 127.0.0.1:55198 - "POST /v1/dataset/scan HTTP/1.1" 404 Not Found

Using the newest AceStep repo version..

Also, all language options have a "vocal" prefix, e.g. vocalEnglish

Please use this forked repository, because the official repository does not have a training API, so I had to rewrite this part and fix some memory leak issues.
https://github.com/sdbds/ACE-Step-1.5-for-windows/tree/qinglong

Also, all language options have a "vocal" prefix, e.g. vocalEnglish
Those don't affect the display because the voice language and translation language need to be separated.

@sdbds
Copy link
Author

sdbds commented Feb 6, 2026

Okay, after thinking about it, I wrote a fallback for the local warp, so that if there is a problem with the API server, it will fall back to using the local warp.

@sdbds sdbds marked this pull request as ready for review February 7, 2026 05:07
fspecii added a commit that referenced this pull request Feb 9, 2026
… control, inline title editing, and ConfirmDialog for deletions

Phase 4: i18n all 15 components from PR #19. Sidebar gains collapse/expand
toggle. Player gets playback speed selector (0.25x-2.0x) fixing bug B1
(hardcoded Chinese '正常'). SongList adds inline title editing and model
version badge. Delete actions now use ConfirmDialog instead of
window.confirm. Volume persisted to localStorage. Training nav item
deferred to Phase 7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants