-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add generation profile management (#71) #141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| CREATE TABLE IF NOT EXISTS generation_profiles ( | ||
| id TEXT PRIMARY KEY, | ||
| name TEXT NOT NULL UNIQUE, | ||
| created_at TEXT NOT NULL, | ||
| updated_at TEXT NOT NULL, | ||
| model_variant TEXT, | ||
| duration_seconds REAL, | ||
| audio_format TEXT, | ||
| thinking INTEGER, | ||
| inference_steps INTEGER, | ||
| guidance_scale REAL, | ||
| bpm INTEGER, | ||
| key_scale TEXT, | ||
| time_signature TEXT, | ||
| vocal_language TEXT, | ||
| lm_backend TEXT | ||
| ); | ||
|
|
||
| CREATE INDEX IF NOT EXISTS idx_generation_profiles_name ON generation_profiles(name); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,211 @@ | ||||||
| use crate::{ | ||||||
| cli::{cli_error, human_output, json_output, spec::ProfileCommand}, | ||||||
| models::{ | ||||||
| errors::{AppError, AppResult}, | ||||||
| profile::CreateProfileRequest, | ||||||
| }, | ||||||
| }; | ||||||
|
|
||||||
| use super::AppState; | ||||||
|
|
||||||
| pub fn execute(state: &AppState, json: bool, command: ProfileCommand) -> AppResult<()> { | ||||||
| match command { | ||||||
| ProfileCommand::List => cmd_list(state, json), | ||||||
| ProfileCommand::Create { | ||||||
| name, | ||||||
| model, | ||||||
| duration, | ||||||
| format, | ||||||
| thinking, | ||||||
| steps, | ||||||
| guidance, | ||||||
| bpm, | ||||||
| key, | ||||||
| time_signature, | ||||||
| language, | ||||||
| lm_backend, | ||||||
| } => cmd_create( | ||||||
| state, | ||||||
| json, | ||||||
| &name, | ||||||
| model, | ||||||
| duration, | ||||||
| format, | ||||||
| thinking, | ||||||
| steps, | ||||||
| guidance, | ||||||
| bpm, | ||||||
| key, | ||||||
| time_signature, | ||||||
| language, | ||||||
| lm_backend, | ||||||
| ), | ||||||
| ProfileCommand::Rename { id, name } => cmd_rename(state, json, &id, &name), | ||||||
| ProfileCommand::Delete { id, yes } => cmd_delete(state, json, &id, yes), | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| fn cmd_list(state: &AppState, json: bool) -> AppResult<()> { | ||||||
| let profiles = state.db.list_profiles()?; | ||||||
|
|
||||||
| if json { | ||||||
| let output = | ||||||
| serde_json::to_string_pretty(&profiles).map_err(|e| cli_error(e.to_string()))?; | ||||||
| json_output(&output); | ||||||
| } else { | ||||||
| if profiles.is_empty() { | ||||||
| human_output("No profiles."); | ||||||
| return Ok(()); | ||||||
| } | ||||||
| println!( | ||||||
| "{:<12} {:<20} {:<8} {:<8} {:<8}", | ||||||
| "ID", "Name", "Model", "Steps", "Format" | ||||||
| ); | ||||||
| println!("{}", "-".repeat(60)); | ||||||
| for profile in &profiles { | ||||||
| let short_id = &profile.id[..8.min(profile.id.len())]; | ||||||
| let name = if profile.name.len() > 18 { | ||||||
| format!("{}…", &profile.name[..17]) | ||||||
|
Comment on lines
+67
to
+68
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Pre-existing byte-index string slicing pattern is replicated from project/list modules The byte-index slicing pattern ( Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||
| } else { | ||||||
| profile.name.clone() | ||||||
| }; | ||||||
| println!( | ||||||
| "{:<12} {:<20} {:<8} {:<8} {:<8}", | ||||||
| short_id, | ||||||
| name, | ||||||
| profile.model_variant.as_deref().unwrap_or("-"), | ||||||
| profile | ||||||
| .inference_steps | ||||||
| .map(|s| s.to_string()) | ||||||
| .unwrap_or_else(|| "-".to_string()), | ||||||
| profile.audio_format.as_deref().unwrap_or("-"), | ||||||
| ); | ||||||
| } | ||||||
| } | ||||||
| Ok(()) | ||||||
| } | ||||||
|
|
||||||
| #[allow(clippy::too_many_arguments)] | ||||||
| fn cmd_create( | ||||||
| state: &AppState, | ||||||
| json: bool, | ||||||
| name: &str, | ||||||
| model: Option<String>, | ||||||
| duration: Option<f64>, | ||||||
| format: Option<String>, | ||||||
| thinking: Option<String>, | ||||||
| steps: Option<i64>, | ||||||
| guidance: Option<f64>, | ||||||
| bpm: Option<i64>, | ||||||
| key: Option<String>, | ||||||
| time_signature: Option<String>, | ||||||
| language: Option<String>, | ||||||
| lm_backend: Option<String>, | ||||||
| ) -> AppResult<()> { | ||||||
| let thinking_bool = match thinking.as_deref() { | ||||||
| Some("on") | Some("true") => Some(true), | ||||||
| Some("off") | Some("false") => Some(false), | ||||||
| Some(other) => { | ||||||
| return Err(AppError::validation_failed(format!( | ||||||
| "Invalid thinking value '{other}' (expected on/off)" | ||||||
| ))); | ||||||
| } | ||||||
| None => None, | ||||||
| }; | ||||||
|
|
||||||
| let request = CreateProfileRequest { | ||||||
| name: name.to_string(), | ||||||
| model_variant: model, | ||||||
| duration_seconds: duration, | ||||||
| audio_format: format, | ||||||
| thinking: thinking_bool, | ||||||
| inference_steps: steps, | ||||||
| guidance_scale: guidance, | ||||||
| bpm, | ||||||
| key_scale: key, | ||||||
| time_signature, | ||||||
| vocal_language: language, | ||||||
| lm_backend, | ||||||
| }; | ||||||
|
|
||||||
| let profile = state.db.create_profile(&request)?; | ||||||
| if json { | ||||||
| let output = | ||||||
| serde_json::to_string_pretty(&profile).map_err(|e| cli_error(e.to_string()))?; | ||||||
| json_output(&output); | ||||||
| } else { | ||||||
| human_output(&format!( | ||||||
| "Created profile '{}' (id: {})", | ||||||
| profile.name, profile.id | ||||||
| )); | ||||||
| } | ||||||
| Ok(()) | ||||||
| } | ||||||
|
|
||||||
| fn cmd_rename(state: &AppState, json: bool, id: &str, name: &str) -> AppResult<()> { | ||||||
| let resolved = resolve_profile_by_prefix(&state.db, id)?; | ||||||
| let profile = state.db.rename_profile(&resolved, name)?; | ||||||
| if json { | ||||||
| let output = | ||||||
| serde_json::to_string_pretty(&profile).map_err(|e| cli_error(e.to_string()))?; | ||||||
| json_output(&output); | ||||||
| } else { | ||||||
| human_output(&format!("Renamed profile {} to '{}'", resolved, name)); | ||||||
| } | ||||||
| Ok(()) | ||||||
| } | ||||||
|
|
||||||
| fn cmd_delete(state: &AppState, json: bool, id: &str, yes: bool) -> AppResult<()> { | ||||||
| let resolved = resolve_profile_by_prefix(&state.db, id)?; | ||||||
|
|
||||||
| if !yes && !json { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Profile deletion skips safety confirmation when JSON output is requested, unlike project deletion The delete-confirmation prompt is skipped when Impact: Users running the CLI with Inconsistency with the project delete commandThe analogous project delete at
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||
| use std::io::Write; | ||||||
| let profiles = state.db.list_profiles()?; | ||||||
| let display_name = profiles | ||||||
| .iter() | ||||||
| .find(|p| p.id == resolved) | ||||||
| .map(|p| p.name.as_str()) | ||||||
| .unwrap_or(&resolved); | ||||||
| print!("Delete profile '{}'? [y/N] ", display_name); | ||||||
| std::io::stdout().flush().ok(); | ||||||
| let mut input = String::new(); | ||||||
| std::io::stdin() | ||||||
| .read_line(&mut input) | ||||||
| .map_err(|e| cli_error(e.to_string()))?; | ||||||
| let trimmed = input.trim(); | ||||||
| if !["y", "Y", "yes", "Yes"].contains(&trimmed) { | ||||||
| human_output("Cancelled."); | ||||||
| return Ok(()); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| state.db.delete_profile(&resolved)?; | ||||||
| if json { | ||||||
| json_output(&format!("{{\"deleted\":\"{}\"}}", resolved)); | ||||||
| } else { | ||||||
| human_output(&format!("Deleted profile {}", resolved)); | ||||||
| } | ||||||
| Ok(()) | ||||||
| } | ||||||
|
|
||||||
| fn resolve_profile_by_prefix( | ||||||
| db: &crate::services::db::Database, | ||||||
| prefix: &str, | ||||||
| ) -> AppResult<String> { | ||||||
| let profiles = db.list_profiles()?; | ||||||
| let matches: Vec<_> = profiles | ||||||
| .iter() | ||||||
| .filter(|p| p.id.starts_with(prefix) || p.name == prefix) | ||||||
| .collect(); | ||||||
| match matches.len() { | ||||||
| 0 => Err(AppError::not_found( | ||||||
| "Profile", | ||||||
| format!("No profile matches '{prefix}'"), | ||||||
| )), | ||||||
| 1 => Ok(matches[0].id.clone()), | ||||||
| _ => Err(AppError::validation_failed(format!( | ||||||
| "Ambiguous profile prefix '{prefix}' matched {} profiles", | ||||||
| matches.len() | ||||||
| ))), | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| use tauri::State; | ||
|
|
||
| use crate::{ | ||
| models::{ | ||
| errors::AppResult, | ||
| profile::{CreateProfileRequest, GenerationProfile, RenameProfileRequest}, | ||
| }, | ||
| AppState, | ||
| }; | ||
|
|
||
| #[tauri::command] | ||
| pub fn list_profiles(state: State<'_, AppState>) -> AppResult<Vec<GenerationProfile>> { | ||
| state.db.list_profiles() | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn create_profile( | ||
| state: State<'_, AppState>, | ||
| request: CreateProfileRequest, | ||
| ) -> AppResult<GenerationProfile> { | ||
| state.db.create_profile(&request) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn rename_profile( | ||
| state: State<'_, AppState>, | ||
| id: String, | ||
| request: RenameProfileRequest, | ||
| ) -> AppResult<GenerationProfile> { | ||
| state.db.rename_profile(&id, &request.name) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn delete_profile(state: State<'_, AppState>, id: String) -> AppResult<()> { | ||
| state.db.delete_profile(&id) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| pub mod backend; | ||
| pub mod errors; | ||
| pub mod generation; | ||
| pub mod profile; | ||
| pub mod project; | ||
| pub mod settings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Profile name truncation crashes on non-ASCII characters
A profile name containing multi-byte characters (e.g., Chinese) is sliced by raw byte index (
&profile.name[..17]atsrc-tauri/src/cli/profiles.rs:68) instead of by character boundary, so listing profiles with such names panics the CLI.Impact: The
openloop profiles listcommand crashes when any profile has a non-ASCII name longer than 18 bytes.Byte-index slicing on UTF-8 strings causes a panic at a non-char-boundary
At
src-tauri/src/cli/profiles.rs:67,profile.name.len()returns the byte length, and at line 68,&profile.name[..17]slices by byte offset. For a Chinese name like"我的音乐生成配置档名称"(each character is 3 UTF-8 bytes), byte index 17 falls in the middle of a character, triggering a panic:byte index 17 is not a char boundary. The app ships with zh-CN locale support, making this a realistic scenario.The fix should use
char_indices()or.chars().take(17)to truncate by character count rather than byte offset.Was this helpful? React with 👍 or 👎 to provide feedback.