Skip to content

feat: persist provider connection status#26

Merged
LRainner merged 4 commits into
masterfrom
feat/provider-status-file
Jun 22, 2026
Merged

feat: persist provider connection status#26
LRainner merged 4 commits into
masterfrom
feat/provider-status-file

Conversation

@LRainner

Copy link
Copy Markdown
Owner

Summary

  • Persist provider connection test status in a backend provider_status.json file.
  • Load/reset ASR and LLM test status from the desktop backend instead of localStorage.

🤖 Generated with Claude Code

Store provider connection test results in a separate backend status file and reload them from the desktop UI. Reset stale status entries when provider connection fields change.

Co-Authored-By: Claude <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces connection testing capabilities for ASR and LLM providers, adding UI elements to the settings page, localization strings, and backend Tauri commands to execute and persist test results. The review feedback highlights a critical compilation error on stable Rust caused by the use of unstable let_chains syntax. Additionally, the reviewer recommends refactoring the status management to utilize an in-memory store within AppState to prevent race conditions and disk I/O overhead, and suggests awaiting configuration saves prior to testing connections to ensure draft settings are correctly applied.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread apps/desktop/src/lib.rs
Comment on lines +595 to +605
// 2. Reset stale provider test status when connection settings changed
let mut provider_status = load_provider_status(&state.provider_status_path);
if reset_changed_provider_status(&old_config, &config, &mut provider_status)
&& let Err(e) = save_provider_status(&state.provider_status_path, &provider_status)
{
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Avoid using the unstable let_chains syntax (&& let Err(e) = ...) which will cause a compilation error on stable Rust. Instead, use a standard nested if let block. Additionally, use the in-memory provider_status store from AppState to prevent race conditions.

Suggested change
// 2. Reset stale provider test status when connection settings changed
let mut provider_status = load_provider_status(&state.provider_status_path);
if reset_changed_provider_status(&old_config, &config, &mut provider_status)
&& let Err(e) = save_provider_status(&state.provider_status_path, &provider_status)
{
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}
// 2. Reset stale provider test status when connection settings changed
let mut provider_status = state.provider_status.lock().unwrap();
if reset_changed_provider_status(&old_config, &config, &mut provider_status) {
if let Err(e) = save_provider_status(&state.provider_status_path, &provider_status) {
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}
}

Comment thread apps/desktop/src/lib.rs
struct AppState {
config: Mutex<AppConfig>,
config_path: std::path::PathBuf,
provider_status_path: std::path::PathBuf,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Add provider_status to AppState to keep the connection test statuses in memory. This allows safe, synchronized access and avoids concurrent file read/write race conditions.

Suggested change
provider_status_path: std::path::PathBuf,
provider_status_path: std::path::PathBuf,
provider_status: Mutex<ProviderStatusStore>,

Comment thread apps/desktop/src/lib.rs
Comment on lines +461 to +470
fn get_provider_connection_status(
state: tauri::State<AppState>,
kind: String,
connection_id: String,
) -> Option<ProviderConnectionStatus> {
let store = load_provider_status(&state.provider_status_path);
provider_status_for_kind(&store, kind.trim())
.and_then(|statuses| statuses.get(connection_id.trim()).cloned())
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Read the connection test status directly from the in-memory provider_status store in AppState instead of loading it from disk on every request. This is much faster and safer.

Suggested change
fn get_provider_connection_status(
state: tauri::State<AppState>,
kind: String,
connection_id: String,
) -> Option<ProviderConnectionStatus> {
let store = load_provider_status(&state.provider_status_path);
provider_status_for_kind(&store, kind.trim())
.and_then(|statuses| statuses.get(connection_id.trim()).cloned())
}
#[tauri::command]
fn get_provider_connection_status(
state: tauri::State<AppState>,
kind: String,
connection_id: String,
) -> Option<ProviderConnectionStatus> {
let store = state.provider_status.lock().unwrap();
provider_status_for_kind(&store, kind.trim())
.and_then(|statuses| statuses.get(connection_id.trim()).cloned())
}

Comment thread apps/desktop/src/lib.rs
Comment on lines +471 to +496
#[tauri::command]
async fn test_asr_connection(
app: tauri::AppHandle,
request: ConnectionTestRequest,
) -> ConnectionTestResult {
let status_path = provider_status_path(&app);
let connection_id = request
.connection_id
.clone()
.map(|id| id.trim().to_string())
.filter(|id| !id.is_empty())
.unwrap_or_else(|| "default".into());
let provider = request.provider.trim().to_string();
let result = match provider.as_str() {
"mock" => Ok("Mock ASR is available.".to_string()),
"openai-compatible" | "" => test_openai_compatible_asr(request).await,
other => Err(anyhow::anyhow!("unsupported ASR provider: {}", other)),
};
let mut result = connection_test_result(provider, result);
if result.ok && result.provider == "mock" {
result.message_key = Some("settings.mock_asr_ok".into());
}
result.tested_at = Some(chrono::Utc::now().to_rfc3339());
save_connection_test_status(&status_path, "asr", &connection_id, &result);
result
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use tauri::State<'_, AppState> to access the in-memory provider_status store and eliminate the race condition on concurrent connection tests.

#[tauri::command]
async fn test_asr_connection(
    state: tauri::State<'_, AppState>,
    request: ConnectionTestRequest,
) -> ConnectionTestResult {
    let connection_id = request
        .connection_id
        .clone()
        .map(|id| id.trim().to_string())
        .filter(|id| !id.is_empty())
        .unwrap_or_else(|| "default".into());
    let provider = request.provider.trim().to_string();
    let result = match provider.as_str() {
        "mock" => Ok("Mock ASR is available.".to_string()),
        "openai-compatible" | "" => test_openai_compatible_asr(request).await,
        other => Err(anyhow::anyhow!("unsupported ASR provider: {}", other)),
    };
    let mut result = connection_test_result(provider, result);
    if result.ok && result.provider == "mock" {
        result.message_key = Some("settings.mock_asr_ok".into());
    }
    result.tested_at = Some(chrono::Utc::now().to_rfc3339());
    save_connection_test_status(&state, "asr", &connection_id, &result);
    result
}

Comment thread apps/desktop/src/lib.rs
Comment on lines +498 to +523
#[tauri::command]
async fn test_llm_connection(
app: tauri::AppHandle,
request: ConnectionTestRequest,
) -> ConnectionTestResult {
let status_path = provider_status_path(&app);
let connection_id = request
.connection_id
.clone()
.map(|id| id.trim().to_string())
.filter(|id| !id.is_empty())
.unwrap_or_else(|| "default".into());
let provider = request.provider.trim().to_string();
let result = match provider.as_str() {
"mock" | "" => Ok("Mock LLM is available.".to_string()),
"openai-compatible" => test_openai_compatible_llm(request).await,
other => Err(anyhow::anyhow!("unsupported LLM provider: {}", other)),
};
let mut result = connection_test_result(provider, result);
if result.ok && result.provider == "mock" {
result.message_key = Some("settings.mock_llm_ok".into());
}
result.tested_at = Some(chrono::Utc::now().to_rfc3339());
save_connection_test_status(&status_path, "llm", &connection_id, &result);
result
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use tauri::State<'_, AppState> to access the in-memory provider_status store and eliminate the race condition on concurrent connection tests.

#[tauri::command]
async fn test_llm_connection(
    state: tauri::State<'_, AppState>,
    request: ConnectionTestRequest,
) -> ConnectionTestResult {
    let connection_id = request
        .connection_id
        .clone()
        .map(|id| id.trim().to_string())
        .filter(|id| !id.is_empty())
        .unwrap_or_else(|| "default".into());
    let provider = request.provider.trim().to_string();
    let result = match provider.as_str() {
        "mock" | "" => Ok("Mock LLM is available.".to_string()),
        "openai-compatible" => test_openai_compatible_llm(request).await,
        other => Err(anyhow::anyhow!("unsupported LLM provider: {}", other)),
    };
    let mut result = connection_test_result(provider, result);
    if result.ok && result.provider == "mock" {
        result.message_key = Some("settings.mock_llm_ok".into());
    }
    result.tested_at = Some(chrono::Utc::now().to_rfc3339());
    save_connection_test_status(&state, "llm", &connection_id, &result);
    result
}

Comment thread apps/desktop/src/lib.rs
Comment on lines +525 to +542
fn save_connection_test_status(
path: &Path,
kind: &str,
connection_id: &str,
result: &ConnectionTestResult,
) {
let mut store = load_provider_status(path);
if let Some(statuses) = provider_status_for_kind_mut(&mut store, kind) {
statuses.insert(connection_id.to_string(), test_result_to_status(result));
if let Err(e) = save_provider_status(path, &store) {
tracing::warn!(
"failed to save provider status to {}: {}",
path.display(),
e
);
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Update save_connection_test_status to lock and write to the in-memory provider_status store in AppState instead of performing unsafe concurrent file reads and writes.

Suggested change
fn save_connection_test_status(
path: &Path,
kind: &str,
connection_id: &str,
result: &ConnectionTestResult,
) {
let mut store = load_provider_status(path);
if let Some(statuses) = provider_status_for_kind_mut(&mut store, kind) {
statuses.insert(connection_id.to_string(), test_result_to_status(result));
if let Err(e) = save_provider_status(path, &store) {
tracing::warn!(
"failed to save provider status to {}: {}",
path.display(),
e
);
}
}
}
fn save_connection_test_status(
state: &AppState,
kind: &str,
connection_id: &str,
result: &ConnectionTestResult,
) {
let mut store = state.provider_status.lock().unwrap();
if let Some(statuses) = provider_status_for_kind_mut(&mut store, kind) {
statuses.insert(connection_id.to_string(), test_result_to_status(result));
if let Err(e) = save_provider_status(&state.provider_status_path, &store) {
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}
}
}

Comment thread apps/desktop/src/lib.rs
.setup(move |app| {
// Load or create config
let cfg_path = config_path(app.handle());
let status_path = provider_status_path(app.handle());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Load the provider status from disk during application startup so it can be kept in memory.

            let status_path = provider_status_path(app.handle());
            let provider_status = load_provider_status(&status_path);

Comment thread apps/desktop/src/lib.rs
app.manage(AppState {
config: Mutex::new(config),
config_path: cfg_path.clone(),
provider_status_path: status_path,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Initialize the provider_status mutex in AppState during setup.

Suggested change
provider_status_path: status_path,
provider_status_path: status_path,
provider_status: Mutex::new(provider_status),

Comment on lines +1836 to +1851
async function testAsrConnection() {
if (!validateSettings()) return;
saveAsrConnectionFields();
setConnectionTestStatus('asr', 'testing', typexI18n.t('settings.connection_testing'), new Date().toISOString());
try {
const result = await invoke('test_asr_connection', { request: getAsrConnectionDraft() });
renderConnectionTestStatus('asr', result);
} catch (e) {
renderConnectionTestStatus('asr', {
ok: false,
provider: document.getElementById('asr-provider').value || 'mock',
message: String(e || typexI18n.t('settings.connection_failed')),
tested_at: new Date().toISOString(),
});
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Await saveConfig() before running the connection test. This ensures that the draft settings are persisted to disk and any stale status is correctly reset before the test runs, preventing UI state mismatches.

Suggested change
async function testAsrConnection() {
if (!validateSettings()) return;
saveAsrConnectionFields();
setConnectionTestStatus('asr', 'testing', typexI18n.t('settings.connection_testing'), new Date().toISOString());
try {
const result = await invoke('test_asr_connection', { request: getAsrConnectionDraft() });
renderConnectionTestStatus('asr', result);
} catch (e) {
renderConnectionTestStatus('asr', {
ok: false,
provider: document.getElementById('asr-provider').value || 'mock',
message: String(e || typexI18n.t('settings.connection_failed')),
tested_at: new Date().toISOString(),
});
}
}
async function testAsrConnection() {
if (!validateSettings()) return;
if (!await saveConfig()) return;
setConnectionTestStatus('asr', 'testing', typexI18n.t('settings.connection_testing'), new Date().toISOString());
try {
const result = await invoke('test_asr_connection', { request: getAsrConnectionDraft() });
renderConnectionTestStatus('asr', result);
} catch (e) {
renderConnectionTestStatus('asr', {
ok: false,
provider: document.getElementById('asr-provider').value || 'mock',
message: String(e || typexI18n.t('settings.connection_failed')),
tested_at: new Date().toISOString(),
});
}
}

Comment on lines +1853 to +1868
async function testLlmConnection() {
if (!validateSettings()) return;
saveLlmConnectionFields();
setConnectionTestStatus('llm', 'testing', typexI18n.t('settings.connection_testing'), new Date().toISOString());
try {
const result = await invoke('test_llm_connection', { request: getLlmConnectionDraft() });
renderConnectionTestStatus('llm', result);
} catch (e) {
renderConnectionTestStatus('llm', {
ok: false,
provider: document.getElementById('llm-provider').value || 'mock',
message: String(e || typexI18n.t('settings.connection_failed')),
tested_at: new Date().toISOString(),
});
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Await saveConfig() before running the connection test. This ensures that the draft settings are persisted to disk and any stale status is correctly reset before the test runs, preventing UI state mismatches.

Suggested change
async function testLlmConnection() {
if (!validateSettings()) return;
saveLlmConnectionFields();
setConnectionTestStatus('llm', 'testing', typexI18n.t('settings.connection_testing'), new Date().toISOString());
try {
const result = await invoke('test_llm_connection', { request: getLlmConnectionDraft() });
renderConnectionTestStatus('llm', result);
} catch (e) {
renderConnectionTestStatus('llm', {
ok: false,
provider: document.getElementById('llm-provider').value || 'mock',
message: String(e || typexI18n.t('settings.connection_failed')),
tested_at: new Date().toISOString(),
});
}
}
async function testLlmConnection() {
if (!validateSettings()) return;
if (!await saveConfig()) return;
setConnectionTestStatus('llm', 'testing', typexI18n.t('settings.connection_testing'), new Date().toISOString());
try {
const result = await invoke('test_llm_connection', { request: getLlmConnectionDraft() });
renderConnectionTestStatus('llm', result);
} catch (e) {
renderConnectionTestStatus('llm', {
ok: false,
provider: document.getElementById('llm-provider').value || 'mock',
message: String(e || typexI18n.t('settings.connection_failed')),
tested_at: new Date().toISOString(),
});
}
}

Keep provider connection status in memory while persisting it to disk, and preserve the testing UI state while saving pending settings before a connection test.

Co-Authored-By: Claude <noreply@anthropic.com>
@LRainner

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces connection testing functionality for ASR and LLM providers, adding UI elements, localized strings, and backend Tauri commands to test connections and persist their status. The review feedback highlights three key areas for improvement: checking for NaN when parsing timestamps in the frontend to prevent incorrect relative time formatting, refactoring unstable let-chains in Rust to ensure compatibility with stable compilers, and offloading synchronous file operations to tokio::task::spawn_blocking to avoid blocking the async executor threads.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +1740 to +1746
function formatRelativeTestTime(timestamp) {
if (!timestamp) return typexI18n.t('settings.connection_not_tested');
const time = typeof timestamp === 'number' ? timestamp : Date.parse(timestamp);
const elapsedMs = Date.now() - time;
if (!Number.isFinite(elapsedMs) || elapsedMs < 60_000) {
return typexI18n.t('settings.connection_last_tested_just_now');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If timestamp is an invalid date string, Date.parse(timestamp) returns NaN. Since Number.isFinite(NaN) is false, the condition !Number.isFinite(elapsedMs) evaluates to true, causing the function to incorrectly return "Last tested just now" instead of handling the parsing failure. Adding a check for Number.isNaN(time) prevents this bug.

Suggested change
function formatRelativeTestTime(timestamp) {
if (!timestamp) return typexI18n.t('settings.connection_not_tested');
const time = typeof timestamp === 'number' ? timestamp : Date.parse(timestamp);
const elapsedMs = Date.now() - time;
if (!Number.isFinite(elapsedMs) || elapsedMs < 60_000) {
return typexI18n.t('settings.connection_last_tested_just_now');
}
function formatRelativeTestTime(timestamp) {
if (!timestamp) return typexI18n.t('settings.connection_not_tested');
const time = typeof timestamp === 'number' ? timestamp : Date.parse(timestamp);
if (Number.isNaN(time)) return typexI18n.t('settings.connection_not_tested');
const elapsedMs = Date.now() - time;
if (elapsedMs < 60_000) {
return typexI18n.t('settings.connection_last_tested_just_now');
}

Comment thread apps/desktop/src/lib.rs
Comment on lines +597 to +606
let mut provider_status = state.provider_status.lock().unwrap();
if reset_changed_provider_status(&old_config, &config, &mut provider_status)
&& let Err(e) = save_provider_status(&state.provider_status_path, &provider_status)
{
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The use of && let (let-chains) is an unstable Rust feature (RFC 2497) and requires #![feature(let_chains)] to compile. To ensure compatibility with stable Rust and improve readability, this should be refactored into nested if statements.

Suggested change
let mut provider_status = state.provider_status.lock().unwrap();
if reset_changed_provider_status(&old_config, &config, &mut provider_status)
&& let Err(e) = save_provider_status(&state.provider_status_path, &provider_status)
{
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}
let mut provider_status = state.provider_status.lock().unwrap();
if reset_changed_provider_status(&old_config, &config, &mut provider_status) {
if let Err(e) = save_provider_status(&state.provider_status_path, &provider_status) {
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}
}

Comment thread apps/desktop/src/lib.rs
Comment on lines +532 to +542
let mut store = state.provider_status.lock().unwrap();
if let Some(statuses) = provider_status_for_kind_mut(&mut store, kind) {
statuses.insert(connection_id.to_string(), test_result_to_status(result));
if let Err(e) = save_provider_status(&state.provider_status_path, &store) {
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Performing synchronous file I/O (save_provider_status) directly inside an async context (called by the async commands test_asr_connection and test_llm_connection) can block the Tokio executor threads. Offloading this blocking operation to tokio::task::spawn_blocking is highly recommended to maintain responsiveness.

Suggested change
let mut store = state.provider_status.lock().unwrap();
if let Some(statuses) = provider_status_for_kind_mut(&mut store, kind) {
statuses.insert(connection_id.to_string(), test_result_to_status(result));
if let Err(e) = save_provider_status(&state.provider_status_path, &store) {
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}
}
let mut store = state.provider_status.lock().unwrap();
if let Some(statuses) = provider_status_for_kind_mut(&mut store, kind) {
statuses.insert(connection_id.to_string(), test_result_to_status(result));
let store_clone = store.clone();
let path_clone = state.provider_status_path.clone();
tokio::task::spawn_blocking(move || {
if let Err(e) = save_provider_status(&path_clone, &store_clone) {
tracing::warn!(
"failed to save provider status to {}: {}",
path_clone.display(),
e
);
}
});
}

Treat unparsable provider status timestamps as not tested instead of showing them as just tested.

Co-Authored-By: Claude <noreply@anthropic.com>
@LRainner

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces connection testing functionality for both ASR and LLM providers in the desktop application. It adds UI elements, CSS styling, and localization keys for testing connections, alongside Tauri backend commands to perform the actual connection tests (including mock and OpenAI-compatible endpoints) and persist their status. Feedback on the backend implementation highlights two critical issues in apps/desktop/src/lib.rs: first, using String::truncate with a hardcoded byte limit on error messages can cause panics on multi-byte UTF-8 characters; second, the use of the unstable let_chains feature (&& let) will cause compilation failures on stable Rust and should be refactored into nested if let statements.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread apps/desktop/src/lib.rs
Comment on lines +160 to +163
if message.len() > 500 {
message.truncate(500);
message.push('…');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using String::truncate with a hardcoded byte limit can cause a panic if the truncation index falls in the middle of a multi-byte UTF-8 character (e.g., Chinese characters or emojis in error messages). It is safer to find the nearest valid character boundary before truncating.

    if message.len() > 500 {
        let mut limit = 500;
        while !message.is_char_boundary(limit) {
            limit -= 1;
        }
        message.truncate(limit);
        message.push('…');
    }

Comment thread apps/desktop/src/lib.rs
Comment on lines +598 to +606
if reset_changed_provider_status(&old_config, &config, &mut provider_status)
&& let Err(e) = save_provider_status(&state.provider_status_path, &provider_status)
{
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The let_chains feature (&& let Err(e) = ...) is currently unstable in Rust. To ensure compatibility with stable Rust compilers, it is recommended to use nested if let statements instead.

Suggested change
if reset_changed_provider_status(&old_config, &config, &mut provider_status)
&& let Err(e) = save_provider_status(&state.provider_status_path, &provider_status)
{
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}
if reset_changed_provider_status(&old_config, &config, &mut provider_status) {
if let Err(e) = save_provider_status(&state.provider_status_path, &provider_status) {
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}
}

Avoid truncating provider connection error messages in the middle of a UTF-8 character boundary.

Co-Authored-By: Claude <noreply@anthropic.com>
@LRainner

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces connection testing capabilities for ASR and LLM providers, allowing users to verify their configuration settings directly from the desktop application's settings view. It adds the necessary UI elements, localization keys, and backend Tauri commands to trigger and track connection tests. The feedback focuses on improving application responsiveness and robustness by offloading synchronous file I/O to a blocking thread and adding timeouts to the network requests for OpenAI-compatible providers to prevent the UI from hanging indefinitely.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread apps/desktop/src/lib.rs
Comment on lines +539 to +545
if let Err(e) = save_provider_status(&state.provider_status_path, &store) {
tracing::warn!(
"failed to save provider status to {}: {}",
state.provider_status_path.display(),
e
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The save_provider_status function performs synchronous file I/O (std::fs::write). Calling this directly on the async executor thread in save_connection_test_status blocks the thread, which can degrade application responsiveness. Since ProviderStatusStore implements Clone, we can clone the store, drop the lock guard early, and perform the file write inside tokio::task::spawn_blocking to keep the async executor thread free.

        let path = state.provider_status_path.clone();
        let store_clone = store.clone();
        drop(store);
        tokio::task::spawn_blocking(move || {
            if let Err(e) = save_provider_status(&path, &store_clone) {
                tracing::warn!(
                    "failed to save provider status to {}: {}",
                    path.display(),
                    e
                );
            }
        });

Comment thread apps/desktop/src/lib.rs
Comment on lines +488 to +492
let result = match provider.as_str() {
"mock" => Ok("Mock ASR is available.".to_string()),
"openai-compatible" | "" => test_openai_compatible_asr(request).await,
other => Err(anyhow::anyhow!("unsupported ASR provider: {}", other)),
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The connection test for the OpenAI-compatible ASR provider does not have a timeout. If the configured endpoint is unresponsive or hangs, the test will block indefinitely, keeping the UI in a 'testing' state. Wrapping the async call in tokio::time::timeout ensures the test fails gracefully after a reasonable duration (e.g., 30 seconds).

Suggested change
let result = match provider.as_str() {
"mock" => Ok("Mock ASR is available.".to_string()),
"openai-compatible" | "" => test_openai_compatible_asr(request).await,
other => Err(anyhow::anyhow!("unsupported ASR provider: {}", other)),
};
let result = match provider.as_str() {
"mock" => Ok("Mock ASR is available.".to_string()),
"openai-compatible" | "" => {
match tokio::time::timeout(std::time::Duration::from_secs(30), test_openai_compatible_asr(request)).await {
Ok(res) => res,
Err(_) => Err(anyhow::anyhow!("Connection timed out after 30 seconds")),
}
}
other => Err(anyhow::anyhow!("unsupported ASR provider: {}", other)),
};

Comment thread apps/desktop/src/lib.rs
Comment on lines +515 to +519
let result = match provider.as_str() {
"mock" | "" => Ok("Mock LLM is available.".to_string()),
"openai-compatible" => test_openai_compatible_llm(request).await,
other => Err(anyhow::anyhow!("unsupported LLM provider: {}", other)),
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The connection test for the OpenAI-compatible LLM provider does not have a timeout. If the configured endpoint is unresponsive or hangs, the test will block indefinitely, keeping the UI in a 'testing' state. Wrapping the async call in tokio::time::timeout ensures the test fails gracefully after a reasonable duration (e.g., 30 seconds).

Suggested change
let result = match provider.as_str() {
"mock" | "" => Ok("Mock LLM is available.".to_string()),
"openai-compatible" => test_openai_compatible_llm(request).await,
other => Err(anyhow::anyhow!("unsupported LLM provider: {}", other)),
};
let result = match provider.as_str() {
"mock" | "" => Ok("Mock LLM is available.".to_string()),
"openai-compatible" => {
match tokio::time::timeout(std::time::Duration::from_secs(30), test_openai_compatible_llm(request)).await {
Ok(res) => res,
Err(_) => Err(anyhow::anyhow!("Connection timed out after 30 seconds")),
}
}
other => Err(anyhow::anyhow!("unsupported LLM provider: {}", other)),
};

@LRainner LRainner merged commit fc15ca8 into master Jun 22, 2026
7 checks passed
@LRainner LRainner deleted the feat/provider-status-file branch June 22, 2026 08:45
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.

1 participant