fix review feedback: restart race, listener cleanup, stable keys, accessibility

This commit is contained in:
Jamie Pine
2026-03-16 13:06:57 -07:00
parent a8469b39f1
commit 2ad4776a76
9 changed files with 66 additions and 54 deletions
@@ -57,8 +57,8 @@ function renderMarkdown(md: string): React.ReactNode[] {
}
elements.push(
<ul key={elements.length} className="space-y-1 my-2">
{items.map((item) => (
<li key={item} className="text-sm text-muted-foreground flex gap-2">
{items.map((item, idx) => (
<li key={idx} className="text-sm text-muted-foreground flex gap-2">
<span className="text-muted-foreground/50 select-none shrink-0">&bull;</span>
<span>{inlineMarkdown(item)}</span>
</li>
@@ -96,9 +96,9 @@ function renderTable(tableLines: string[], keyBase: number): React.ReactNode {
<table className="text-sm w-full">
<thead>
<tr className="border-b">
{headers.map((h) => (
{headers.map((h, hIdx) => (
<th
key={h}
key={hIdx}
className="text-left py-1.5 pr-4 text-muted-foreground font-medium text-xs"
>
{inlineMarkdown(h)}
@@ -107,10 +107,10 @@ function renderTable(tableLines: string[], keyBase: number): React.ReactNode {
</tr>
</thead>
<tbody>
{rows.map((row) => (
<tr key={row.join('|')} className="border-b border-border/50">
{row.map((cell) => (
<td key={cell} className="py-1.5 pr-4 text-muted-foreground">
{rows.map((row, rowIdx) => (
<tr key={rowIdx} className="border-b border-border/50">
{row.map((cell, cellIdx) => (
<td key={cellIdx} className="py-1.5 pr-4 text-muted-foreground">
{inlineMarkdown(cell)}
</td>
))}
@@ -133,6 +133,13 @@ export function GeneralPage() {
setKeepServerRunningOnClose(checked);
platform.lifecycle.setKeepServerRunning(checked).catch((error) => {
console.error('Failed to sync setting to Rust:', error);
setKeepServerRunningOnClose(!checked);
toast({
title: 'Failed to update setting',
description: 'Could not sync setting to backend.',
variant: 'destructive',
});
return;
});
toast({
title: 'Setting updated',
+28 -37
View File
@@ -171,17 +171,21 @@ export function GpuPage() {
};
}, [cudaDownloading, serverUrl, refetchCudaStatus]);
const clearHealthPolling = useCallback(() => {
if (healthPollRef.current) {
clearInterval(healthPollRef.current);
healthPollRef.current = null;
}
}, []);
const startHealthPolling = useCallback(() => {
if (healthPollRef.current) return;
clearHealthPolling();
healthPollRef.current = setInterval(async () => {
try {
const result = await apiClient.getHealth();
if (result.status === 'healthy') {
if (healthPollRef.current) {
clearInterval(healthPollRef.current);
healthPollRef.current = null;
}
clearHealthPolling();
setRestartPhase('ready');
queryClient.invalidateQueries();
setTimeout(() => setRestartPhase('idle'), 2000);
@@ -190,7 +194,23 @@ export function GpuPage() {
// Server still down, keep polling
}
}, 1000);
}, [queryClient]);
}, [queryClient, clearHealthPolling]);
const restartServerWithPolling = useCallback(
async (errorMessage: string) => {
setRestartPhase('stopping');
try {
await platform.lifecycle.restartServer();
setRestartPhase('waiting');
startHealthPolling();
} catch (e: unknown) {
clearHealthPolling();
setRestartPhase('idle');
throw new Error(e instanceof Error ? e.message : errorMessage);
}
},
[platform, startHealthPolling, clearHealthPolling],
);
const handleDownload = async () => {
setError(null);
@@ -209,24 +229,9 @@ export function GpuPage() {
const handleRestart = async () => {
setError(null);
setRestartPhase('stopping');
try {
setRestartPhase('waiting');
startHealthPolling();
await platform.lifecycle.restartServer();
if (healthPollRef.current) {
clearInterval(healthPollRef.current);
healthPollRef.current = null;
}
setRestartPhase('ready');
queryClient.invalidateQueries();
setTimeout(() => setRestartPhase('idle'), 2000);
await restartServerWithPolling('Restart failed');
} catch (e: unknown) {
setRestartPhase('idle');
if (healthPollRef.current) {
clearInterval(healthPollRef.current);
healthPollRef.current = null;
}
setError(e instanceof Error ? e.message : 'Restart failed');
}
};
@@ -236,22 +241,8 @@ export function GpuPage() {
setRestartPhase('stopping');
try {
await apiClient.deleteCudaBackend();
setRestartPhase('waiting');
startHealthPolling();
await platform.lifecycle.restartServer();
if (healthPollRef.current) {
clearInterval(healthPollRef.current);
healthPollRef.current = null;
}
setRestartPhase('ready');
queryClient.invalidateQueries();
setTimeout(() => setRestartPhase('idle'), 2000);
await restartServerWithPolling('Failed to switch to CPU');
} catch (e: unknown) {
setRestartPhase('idle');
if (healthPollRef.current) {
clearInterval(healthPollRef.current);
healthPollRef.current = null;
}
setError(e instanceof Error ? e.message : 'Failed to switch to CPU');
refetchCudaStatus();
}
+1 -1
View File
@@ -96,7 +96,7 @@ export function LogsPage() {
)}
</div>
) : (
entries.map((entry, i) => <LogLine key={`${entry.timestamp}-${i}`} entry={entry} />)
entries.map((entry) => <LogLine key={entry.id} entry={entry} />)
)}
</div>
</div>
+1 -1
View File
@@ -48,7 +48,7 @@ export function SettingRow({
<div className="min-w-0">
<label
htmlFor={htmlFor}
className="text-sm font-medium leading-none cursor-pointer select-none"
className={`text-sm font-medium leading-none select-none ${htmlFor ? 'cursor-pointer' : ''}`}
>
{title}
</label>
+1 -1
View File
@@ -25,7 +25,7 @@ export function Toaster() {
<ToastClose />
</Toast>
))}
<ToastViewport className={isPlayerOpen ? 'sm:bottom-32' : ''} />
<ToastViewport className={isPlayerOpen ? 'sm:bottom-44' : ''} />
</ToastProvider>
);
}
+1
View File
@@ -26,6 +26,7 @@ const Toggle = React.forwardRef<HTMLButtonElement, ToggleProps>(
}}
className={cn(
'relative inline-flex h-5 w-9 shrink-0 items-center rounded-full transition-colors',
'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2',
checked ? 'bg-accent' : 'bg-muted-foreground/25',
disabled ? 'opacity-50 cursor-not-allowed' : 'cursor-pointer',
className,
+4 -2
View File
@@ -3,7 +3,10 @@ import type { ServerLogEntry } from '@/platform/types';
const MAX_LOG_ENTRIES = 2000;
let nextLogEntryId = 0;
export interface LogEntry extends ServerLogEntry {
id: number;
timestamp: number;
}
@@ -17,9 +20,8 @@ export const useLogStore = create<LogStore>((set) => ({
entries: [],
addEntry: (entry) =>
set((state) => {
const newEntry: LogEntry = { ...entry, timestamp: Date.now() };
const newEntry: LogEntry = { ...entry, id: nextLogEntryId++, timestamp: Date.now() };
const entries = [...state.entries, newEntry];
// Cap buffer size
if (entries.length > MAX_LOG_ENTRIES) {
return { entries: entries.slice(entries.length - MAX_LOG_ENTRIES) };
}
+15 -4
View File
@@ -88,16 +88,27 @@ class TauriLifecycle implements PlatformLifecycle {
}
subscribeToServerLogs(callback: (entry: ServerLogEntry) => void): () => void {
let disposed = false;
let unlisten: (() => void) | null = null;
listen<ServerLogEntry>('server-log', (event) => {
void listen<ServerLogEntry>('server-log', (event) => {
callback(event.payload);
}).then((fn) => {
unlisten = fn;
});
})
.then((fn) => {
if (disposed) {
fn();
return;
}
unlisten = fn;
})
.catch((error) => {
console.error('Failed to subscribe to server logs:', error);
});
return () => {
disposed = true;
unlisten?.();
unlisten = null;
};
}
}