Skip to content

feat(dashboard): auto-save config with 500ms debounce #199

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.

Sign up for GitHub

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

Merged
BillChirico merged 5 commits into from
Mar 2, 2026
+446 −31
Merged

feat(dashboard): auto-save config with 500ms debounce #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5210f9b
feat(dashboard): replace manual save with auto-save (500ms debounce)
BillChirico Mar 2, 2026
eb894ad
test(dashboard): add auto-save tests for ConfigEditor
BillChirico Mar 2, 2026
b8300bd
fix(dashboard): prevent fetchConfig from overwriting saveStatus after…
BillChirico Mar 2, 2026
af6ff5c
test(dashboard): use fake timers, restore vi.stubGlobal, fix assertio…
BillChirico Mar 2, 2026
61d6f1d
Merge remote-tracking branch 'origin/main' into feat/issue-189
Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 129 additions & 31 deletions web/src/components/dashboard/config-editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use client';

import { Loader2, Save } from 'lucide-react';
import { AlertCircle, CheckCircle2, Loader2, RefreshCw } from 'lucide-react';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { toast } from 'sonner';
import { Button } from '@/components/ui/button';
Expand Down Expand Up @@ -136,6 +136,15 @@ export function ConfigEditor() {

const abortRef = useRef<AbortController | null>(null);

/** Auto-save status indicator. */
const [saveStatus, setSaveStatus] = useState<'idle' | 'saving' | 'saved' | 'error'>('idle');
/** Debounce timer for auto-save. */
const autoSaveTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
/** True while the initial config load is in progress — suppresses auto-save. */
const isLoadingConfigRef = useRef(false);
/** Ref tracking latest draftConfig for race-condition detection during save. */
const draftConfigRef = useRef<GuildConfig | null>(null);

const updateDraftConfig = useCallback((updater: (prev: GuildConfig) => GuildConfig) => {
setDraftConfig((prev) => updater((prev ?? {}) as GuildConfig));
}, []);
Expand Down Expand Up @@ -169,7 +178,7 @@ export function ConfigEditor() {
}, []);

// ── Load config when guild changes ─────────────────────────────
const fetchConfig = useCallback(async (id: string) => {
const fetchConfig = useCallback(async (id: string, { skipSaveStatusReset = false } = {}) => {
if (!id) return;

abortRef.current?.abort();
Expand All @@ -178,6 +187,7 @@ export function ConfigEditor() {

setLoading(true);
setError(null);
isLoadingConfigRef.current = true;

try {
const res = await fetch(`/api/guilds/${encodeURIComponent(id)}/config`, {
Expand Down Expand Up @@ -209,10 +219,19 @@ export function ConfigEditor() {
}
setSavedConfig(data);
setDraftConfig(structuredClone(data));
// Reset status after a successful reload; clear any stale error
if (!skipSaveStatusReset) {
setSaveStatus('idle');
}
// Use a macrotask so the state setter for draftConfig fires before we clear the flag
setTimeout(() => {
isLoadingConfigRef.current = false;
}, 0);
setDmStepsRaw((data.welcome?.dmSequence?.steps ?? []).join('\n'));
setProtectRoleIdsRaw((data.moderation?.protectRoles?.roleIds ?? []).join(', '));
} catch (err) {
if ((err as Error).name === 'AbortError') return;
isLoadingConfigRef.current = false;
Comment on lines +226 to +234
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

isLoadingConfigRef is cleared via setTimeout(..., 0) without checking that the completing request is still the latest one. If the guild changes quickly (or multiple fetchConfig calls overlap), an earlier request can flip the ref to false while a newer load is still in progress, allowing auto-save to run during load. Consider tracking a request id / comparing abortRef.current === controller inside the timeout (and in the error/abort paths) before clearing the flag.

Suggested change
// Use a macrotask so the state setter for draftConfig fires before we clear the flag
setTimeout(() => {
isLoadingConfigRef.current = false;
}, 0);
setDmStepsRaw((data.welcome?.dmSequence?.steps ?? []).join('\n'));
setProtectRoleIdsRaw((data.moderation?.protectRoles?.roleIds ?? []).join(', '));
} catch (err) {
if ((err as Error).name === 'AbortError') return;
isLoadingConfigRef.current = false;
// Use a macrotask so the state setter for draftConfig fires before we clear the flag.
// Only clear the loading flag if this request is still the active one.
setTimeout(() => {
if (abortRef.current === controller) {
isLoadingConfigRef.current = false;
}
}, 0);
setDmStepsRaw((data.welcome?.dmSequence?.steps ?? []).join('\n'));
setProtectRoleIdsRaw((data.moderation?.protectRoles?.roleIds ?? []).join(', '));
} catch (err) {
if ((err as Error).name === 'AbortError') return;
// Only clear the loading flag if this request is still the active one.
if (abortRef.current === controller) {
isLoadingConfigRef.current = false;
}

Copilot uses AI. Check for mistakes.
const msg = (err as Error).message || 'Failed to load config';
setError(msg);
toast.error('Failed to load config', { description: msg });
Expand All @@ -226,6 +245,11 @@ export function ConfigEditor() {
return () => abortRef.current?.abort();
}, [guildId, fetchConfig]);

// Keep draftConfigRef in sync for race-condition detection in saveChanges
useEffect(() => {
draftConfigRef.current = draftConfig;
}, [draftConfig]);
Comment on lines +248 to +251
Copy link

@greptile-apps greptile-apps bot Mar 2, 2026

Choose a reason for hiding this comment

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

ref update timing issue: draftConfigRef.current is updated asynchronously in this effect after renders, but saveChanges (line 389) checks it synchronously after PATCH completes. If the server responds before this effect runs (e.g., user types → render scheduled → PATCH completes → ref still has old value), the check incorrectly concludes no changes occurred, calls fetchConfig, and overwrites the user's pending edits.

Consider updating the ref synchronously when setDraftConfig is called, or use a different approach like comparing against the latest state in a ref callback.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: web/src/components/dashboard/config-editor.tsx
Line: 248-251

Comment:
ref update timing issue: `draftConfigRef.current` is updated asynchronously in this effect after renders, but `saveChanges` (line 389) checks it synchronously after PATCH completes. If the server responds before this effect runs (e.g., user types → render scheduled → PATCH completes → ref still has old value), the check incorrectly concludes no changes occurred, calls `fetchConfig`, and overwrites the user's pending edits.

Consider updating the ref synchronously when `setDraftConfig` is called, or use a different approach like comparing against the latest state in a ref callback.

How can I resolve this? If you propose a fix, please make it concise.


// ── Derived state ──────────────────────────────────────────────
const hasChanges = useMemo(() => {
if (!savedConfig || !draftConfig) return false;
Expand Down Expand Up @@ -272,10 +296,10 @@ export function ConfigEditor() {
}

const patches = computePatches(savedConfig, draftConfig);
if (patches.length === 0) {
toast.info('No changes to save.');
return;
}
if (patches.length === 0) return;

// Snapshot draft before saving to detect changes made during in-flight save
const draftSnapshot = draftConfig;

// Group patches by top-level section for batched requests
const bySection = new Map<string, Array<{ path: string; value: unknown }>>();
Expand All @@ -290,6 +314,7 @@ export function ConfigEditor() {
}

setSaving(true);
setSaveStatus('saving');

// Shared AbortController for all section saves - aborts all in-flight requests on 401
const saveAbortController = new AbortController();
Expand Down Expand Up @@ -354,29 +379,64 @@ export function ConfigEditor() {
return updated;
});
}
setSaveStatus('error');
toast.error('Some sections failed to save', {
description: `Failed: ${failedSections.join(', ')}`,
});
} else {
toast.success('Config saved successfully!');
// Full success: reload to get the authoritative version from the server
await fetchConfig(guildId);
setSaveStatus('saved');
// Full success: check if draft changed during save (user edited while saving)
if (deepEqual(draftConfigRef.current, draftSnapshot)) {
// No changes during save — safe to reload authoritative version
await fetchConfig(guildId, { skipSaveStatusReset: true });
Comment on lines +387 to +391
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

On full success, this sets status to saved and then calls fetchConfig, which toggles the global loading state and re-fetches the entire config. With auto-save, this can introduce frequent extra GETs and briefly replace the editor with the loading screen after each save. Consider avoiding the full reload (update savedConfig from draftSnapshot / server response) or adding a background-refresh mode that doesn’t flip loading/hide the editor.

Copilot uses AI. Check for mistakes.
} else {
// Draft changed during save — don't overwrite user's new changes
// Update savedConfig to reflect what was successfully persisted
setSavedConfig(structuredClone(draftSnapshot));
}
}
} catch (err) {
const msg = (err as Error).message || 'Failed to save config';
setSaveStatus('error');
toast.error('Failed to save config', { description: msg });
} finally {
setSaving(false);
}
}, [guildId, savedConfig, draftConfig, hasValidationErrors, fetchConfig]);

// ── Auto-save: debounce 500ms after draft changes ──────────────
useEffect(() => {
// Don't auto-save during initial config load or if no changes
if (isLoadingConfigRef.current || !hasChanges || hasValidationErrors || saving) return;

// Clear any pending timer
if (autoSaveTimerRef.current) {
clearTimeout(autoSaveTimerRef.current);
}

autoSaveTimerRef.current = setTimeout(() => {
void saveChanges();
}, 500);
Comment on lines +408 to +419
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

When saveStatus is saved, making a new edit doesn’t clear it, so the UI can still display “Saved” during the 500ms debounce window even though there are unsaved changes. Consider resetting saveStatus back to idle (or a dedicated “pending changes” state) as soon as hasChanges becomes true, so the indicator doesn’t misrepresent the current persistence state.

Copilot uses AI. Check for mistakes.

return () => {
if (autoSaveTimerRef.current) {
clearTimeout(autoSaveTimerRef.current);
}
};
}, [draftConfig, hasChanges, hasValidationErrors, saving, saveChanges]);

// ── Keyboard shortcut: Ctrl/Cmd+S to save ──────────────────────
useEffect(() => {
function onKeyDown(e: KeyboardEvent) {
if ((e.metaKey || e.ctrlKey) && e.key === 's') {
e.preventDefault();
if (hasChanges && !saving && !hasValidationErrors) {
saveChanges();
// Cancel any pending debounce timer and save immediately
if (autoSaveTimerRef.current) {
clearTimeout(autoSaveTimerRef.current);
autoSaveTimerRef.current = null;
}
void saveChanges();
}
}
}
Expand Down Expand Up @@ -697,38 +757,24 @@ export function ConfigEditor() {
Manage AI, welcome messages, and other settings.
</p>
</div>
<div className="flex items-center gap-2">
<div className="flex items-center gap-3">
{/* Auto-save status indicator */}
<AutoSaveStatus status={saveStatus} onRetry={saveChanges} />
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

AutoSaveStatus is rendered solely from saveStatus, so it can remain visible (e.g., Save failed or Saved) even after the user discards changes and hasChanges becomes false. Consider resetting saveStatus to idle when discarding/when hasChanges becomes false, and wrap onRetry to also clear any pending debounce timer (similar to the Ctrl+S handler) to avoid double-saves.

Suggested change
<AutoSaveStatus status={saveStatus} onRetry={saveChanges} />
<AutoSaveStatus status={hasChanges ? saveStatus : 'idle'} onRetry={saveChanges} />

Copilot uses AI. Check for mistakes.
<DiscardChangesButton
onReset={discardChanges}
disabled={saving || !hasChanges}
sectionLabel="all unsaved changes"
/>
<Button
onClick={saveChanges}
disabled={saving || !hasChanges || hasValidationErrors}
aria-keyshortcuts="Control+S Meta+S"
>
{saving ? (
<Loader2 className="mr-2 h-4 w-4 animate-spin" aria-hidden="true" />
) : (
<Save className="mr-2 h-4 w-4" aria-hidden="true" />
)}
{saving ? 'Saving...' : 'Save Changes'}
</Button>
</div>
</div>

{/* Unsaved changes banner */}
{hasChanges && (
{/* Validation error banner */}
{hasChanges && hasValidationErrors && (
<output
aria-live="polite"
className="rounded-md border border-yellow-500/30 bg-yellow-500/10 px-4 py-3 text-sm text-yellow-200"
className="rounded-md border border-destructive/30 bg-destructive/10 px-4 py-3 text-sm text-destructive"
>
You have unsaved changes.{' '}
<kbd className="rounded border border-yellow-500/30 bg-yellow-500/10 px-1.5 py-0.5 font-mono text-xs">
Ctrl+S
</kbd>{' '}
to save.
Fix validation errors before changes can be saved.
</output>
)}

Expand Down Expand Up @@ -2241,6 +2287,58 @@ export function ConfigEditor() {
);
}

// ── Auto-Save Status Indicator ────────────────────────────────

interface AutoSaveStatusProps {
status: 'idle' | 'saving' | 'saved' | 'error';
onRetry: () => void;
}

/**
* Displays the current auto-save status with an icon and text.
*
* Shows nothing when idle, a spinner while saving, a check icon on success,
* and an error state with a retry button on failure.
*/
function AutoSaveStatus({ status, onRetry }: AutoSaveStatusProps) {
if (status === 'idle') return null;

if (status === 'saving') {
return (
<output className="flex items-center gap-1.5 text-sm text-muted-foreground">
<Loader2 className="h-4 w-4 animate-spin" aria-hidden="true" />
Saving...
</output>
);
}

if (status === 'saved') {
return (
<output className="flex items-center gap-1.5 text-sm text-green-500">
<CheckCircle2 className="h-4 w-4" aria-hidden="true" />
Saved
</output>
);
}

// error state
return (
<output className="flex items-center gap-1.5 text-sm text-destructive">
<AlertCircle className="h-4 w-4" aria-hidden="true" />
Save failed
<button
type="button"
onClick={onRetry}
className="ml-1 inline-flex items-center gap-1 rounded px-1.5 py-0.5 text-xs font-medium underline-offset-2 hover:underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
aria-label="Retry save"
Comment on lines +2326 to +2333
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

AutoSaveStatus uses <output> for the status text, but it isn’t marked as a live region, and the error variant nests an interactive <button> inside <output>. For better screen reader behavior, consider using role="status"/aria-live="polite" on a non-output wrapper (e.g., a <div>/<span>) and keep the Retry button as a sibling element.

Copilot uses AI. Check for mistakes.
>
<RefreshCw className="h-3 w-3" aria-hidden="true" />
Retry
</button>
</output>
);
}

// ── Toggle Switch ───────────────────────────────────────────────

interface ToggleSwitchProps {
Expand Down
Loading
Loading