165 lines
14 KiB
Markdown
165 lines
14 KiB
Markdown
# Domain Pitfalls
|
|
|
|
**Domain:** Brownfield runtime/settings workspace UX for a local-first desktop app
|
|
**Researched:** 2026-03-28
|
|
|
|
## Critical Pitfalls
|
|
|
|
### Pitfall 1: Polling overwrites dirty draft state
|
|
**What goes wrong:** A live runtime snapshot is treated as the same thing as the editable form, so background refresh rehydrates in-progress fields like models folder, selected model, API URL, or start policy.
|
|
**Why it happens:** Brownfield code usually starts with "load snapshot into local state" and later adds polling. In this repo, `useRuntimeManager` polls every 3 seconds in `tauri-app/src/features/runtime/useRuntimeManager.ts:33`, while `RuntimeSettingsDrawer` resets local form state whenever `snapshot` changes in `tauri-app/src/features/runtime/components/RuntimeSettingsDrawer.tsx:76`.
|
|
**Consequences:** Users lose folder-picker selections, selected models jump, save writes unexpected values, and trust in the settings surface collapses.
|
|
**Warning signs:**
|
|
- Editing a field and seeing it snap back after a status refresh
|
|
- Folder picker or model dropdown changing without explicit user action
|
|
- Bugs phrased as "save worked but UI changed before I clicked save"
|
|
- `useEffect` blocks that copy server/runtime props into form state whenever snapshot props change
|
|
**Prevention:**
|
|
- Separate `liveSnapshot`, `draft`, and `savedBaseline` as distinct states
|
|
- Freeze draft reinitialization while form is dirty; only rebase on explicit save/cancel/reopen
|
|
- Show live runtime changes as non-destructive notices instead of mutating draft fields
|
|
- Add tests that simulate polling during edits and assert draft preservation
|
|
**Phase should address it:** Workspace state-model phase, before any UI reorganization or control consolidation
|
|
**Detection:** Repro test: open settings, edit `models_dir`, trigger polling/refresh, verify draft stays untouched.
|
|
|
|
### Pitfall 2: Event actions are coupled to render-sync effects instead of explicit user intents
|
|
**What goes wrong:** Save/start/stop behavior is inferred from snapshot changes or effect chains instead of being modeled as explicit commands with predictable sequencing.
|
|
**Why it happens:** Brownfield React screens often grow from separate cards/drawers and then unify later. React's current guidance explicitly warns against using Effects for event-specific logic because it creates stale or duplicated actions.
|
|
**Consequences:** Double start attempts, stop-before-save confusion, operations firing after unrelated refreshes, and hard-to-debug race conditions during "save and start" flows.
|
|
**Warning signs:**
|
|
- One button both mutates draft state and infers an operational action from subsequent render state
|
|
- Bugs that only appear under slow I/O or repeated clicks
|
|
- Operational state described as "sometimes it starts twice" or "clicked stop but save still ran"
|
|
- Shared busy flags masking which operation is in flight
|
|
**Prevention:**
|
|
- Model `save`, `start`, `stop`, and `saveThenStart` as explicit commands with one operation state machine
|
|
- Keep event logic in action handlers, not in snapshot-sync effects
|
|
- Add idempotency/operation guards so a second click cannot enqueue conflicting work
|
|
- Track operation-specific status (`saving`, `starting`, `stopping`) instead of one generic `busy`
|
|
**Phase should address it:** Runtime control orchestration phase
|
|
**Detection:** Integration tests for rapid repeated clicks, slow save + start, and stop while save is pending.
|
|
|
|
### Pitfall 3: Async side effects write back after the user has already moved on
|
|
**What goes wrong:** Model scans, GPU queries, Hugging Face searches, and folder pickers resolve later and overwrite newer UI intent.
|
|
**Why it happens:** These tasks are launched from the settings drawer and update local state independently. In `RuntimeSettingsDrawer`, `scanRuntimeModels`, `queryRuntimeGpu`, `getModelLibraryState`, and search flows all update drawer state after async completion.
|
|
**Consequences:** Wrong model gets auto-selected, stale scan results replace newer folder choices, GPU recommendations overwrite manual values, and users save a configuration they did not mean to create.
|
|
**Warning signs:**
|
|
- Changing folders quickly causes the first scan result to win
|
|
- Search results from an old query appear after entering a new one
|
|
- "Use recommended" values reappear after a user typed something else
|
|
- No request tokens, abort logic, or latest-request guards around async setters
|
|
**Prevention:**
|
|
- Guard async responses with request IDs or abort/cancel semantics
|
|
- Apply async results only if they still match the current draft context
|
|
- Never auto-apply a recommendation after the field becomes dirty
|
|
- Record whether a value is user-authored vs system-suggested before mutating it
|
|
**Phase should address it:** Async data-source hardening phase for model discovery and device/runtime metadata
|
|
**Detection:** Tests that change folders/modes mid-request and assert stale responses are ignored.
|
|
|
|
### Pitfall 4: One surface mixes draft validation, runtime health, and operational errors with no ownership boundaries
|
|
**What goes wrong:** Users see one error banner, but it is unclear whether the issue is an unsaved field problem, a runtime health failure, a file-dialog issue, or a failed start/stop command.
|
|
**Why it happens:** Brownfield runtime screens often merge settings and operations without redesigning the message model. This repo already carries both `localError` and `snapshot.last_error` into the same banner area in `tauri-app/src/features/runtime/components/RuntimeSettingsDrawer.tsx:614`.
|
|
**Consequences:** Users retry the wrong action, overwrite a recoverable draft, or think save failed when only health revalidation failed.
|
|
**Warning signs:**
|
|
- A single banner renders validation, transport, and runtime-health problems interchangeably
|
|
- Support tickets include "I don't know what failed"
|
|
- Start/stop buttons remain visible but their failure cause is ambiguous
|
|
**Prevention:**
|
|
- Split error ownership into validation, draft-save, runtime-operation, and background-health channels
|
|
- Keep background degradation visible without blocking draft editing
|
|
- Attach remediation copy to each error type (`fix field`, `retry save`, `restart runtime`, `recheck health`)
|
|
- Log structured error sources so QA can verify the right message appears for each failure class
|
|
**Phase should address it:** Error-handling and operator feedback phase
|
|
**Detection:** UX acceptance tests that inject each failure type independently and verify distinct copy and CTA behavior.
|
|
|
|
## Moderate Pitfalls
|
|
|
|
### Pitfall 5: Hidden-mode fields leak across local/remote switching
|
|
**What goes wrong:** Switching between local and remote modes either drops meaningful local draft state or silently carries stale values that should be isolated.
|
|
**Why it happens:** Brownfield forms often reuse one object for multiple conceptual modes. `createRuntimeConfigPatch` conditionally preserves some snapshot values based on `form.mode` in `tauri-app/src/features/runtime/runtime-settings-form.ts:76`, which is safer than wiping everything, but still requires explicit draft semantics to avoid confusion.
|
|
**Consequences:** Users think a local model choice was saved when it was only temporarily hidden, or remote credentials and local runtime state appear partially applied.
|
|
**Warning signs:**
|
|
- Toggling local/remote changes the primary CTA wording but preserves unexplained hidden values
|
|
- Save behavior differs depending on prior mode history
|
|
- Bugs mention "switching back brought old values back" or "my local setup disappeared"
|
|
**Prevention:**
|
|
- Maintain mode-specific draft slices with a shared shell state
|
|
- Show when hidden values are preserved vs discarded
|
|
- Require explicit save to commit a mode switch and its dependent fields
|
|
- Add tests for local -> remote -> local round trips without data loss or accidental carryover
|
|
**Phase should address it:** Settings information architecture phase
|
|
**Detection:** Round-trip tests across mode switches with unsaved changes.
|
|
|
|
### Pitfall 6: Polling ignores editing context and transient operation states
|
|
**What goes wrong:** Background refresh keeps firing during `starting`, `stopping`, picker dialogs, and active editing, producing UI jitter and misleading status transitions.
|
|
**Why it happens:** A fixed interval poller is simple to add in brownfield code, but runtime workspaces need context-aware refresh rules.
|
|
**Consequences:** Status flashes between stale and optimistic values, controls disable unexpectedly, and users mistrust whether the runtime is actually ready.
|
|
**Warning signs:**
|
|
- Same poll interval regardless of `starting`, `ready`, `stopping`, or drawer-open state
|
|
- Snapshot refresh racing against optimistic local status updates
|
|
- Intermittent test failures tied to timers
|
|
**Prevention:**
|
|
- Make refresh policy state-aware: pause or downshift polling while editing, during modal OS dialogs, and while a command is in flight
|
|
- Prefer explicit revalidation on operation completion over constant polling during transitions
|
|
- Distinguish optimistic local operation state from confirmed backend snapshot state
|
|
**Phase should address it:** Runtime observability and refresh-policy phase
|
|
**Detection:** Timer-based tests covering idle, editing, starting, stopping, and degraded states.
|
|
|
|
### Pitfall 7: Unsaved-change boundaries stop at Save/Cancel and ignore close/navigation paths
|
|
**What goes wrong:** Closing the drawer, switching surfaces, or triggering a mode change drops dirty edits without warning.
|
|
**Why it happens:** Brownfield teams add explicit Save buttons first, then forget every other exit path.
|
|
**Consequences:** Data loss reports continue even after form-state bugs are "fixed."
|
|
**Warning signs:**
|
|
- Dirty tracking exists only to enable/disable the save button
|
|
- Drawer close/backdrop click has no unsaved-change interception
|
|
- Bug reports say "I never clicked cancel"
|
|
**Prevention:**
|
|
- Centralize dirty-state tracking and route all dismiss/navigation actions through it
|
|
- Prompt on destructive exits; allow explicit discard
|
|
- Suppress the warning only for confirmed save completion or explicit cancel
|
|
**Phase should address it:** Draft-safety and navigation-guard phase
|
|
**Detection:** Interaction tests for backdrop close, Escape, mode switch, and app-surface navigation with dirty drafts.
|
|
|
|
## Minor Pitfalls
|
|
|
|
### Pitfall 8: Brownfield tests cover happy-path flows but not interleavings
|
|
**What goes wrong:** The app looks correct in straightforward tests yet still fails under timer ticks, stale async responses, and cross-surface transitions.
|
|
**Why it happens:** Brownfield teams usually add tests for save/start success, but not for overlapping events. Current tests already cover important happy paths and degraded polling, but not the full draft-vs-polling conflict space.
|
|
**Consequences:** Regressions return during refactors because the highest-risk state interleavings remain unpinned.
|
|
**Warning signs:**
|
|
- Tests assert final text only, not intermediate states
|
|
- No fake-timer coverage for edits during polling
|
|
- No tests for stale async completion after newer user input
|
|
**Prevention:**
|
|
- Add interleaving-focused tests: edit + poll, pick folder + rescan + save, save + start + poll, mode toggle + stale search result
|
|
- Treat runtime workspace as a small state machine and test transitions, not just screens
|
|
- Keep fixtures for degraded health, missing binary, dirty draft, and mode-switch edge cases
|
|
**Phase should address it:** QA/regression phase, but test scaffolding should begin in the first state-model phase
|
|
**Detection:** Red-team test matrix covering timing, cancellation, and multi-action overlap.
|
|
|
|
## Phase-Specific Warnings
|
|
|
|
| Phase Topic | Likely Pitfall | Mitigation |
|
|
|-------------|---------------|------------|
|
|
| Workspace state model | Polling clobbers drafts | Split live snapshot, saved baseline, and editable draft before changing UI layout |
|
|
| Runtime control orchestration | Save/start/stop commands race or double-fire | Introduce explicit command state machine and per-operation statuses |
|
|
| Settings IA and mode switching | Local/remote values leak or vanish | Use mode-scoped draft slices and explicit mode-commit behavior |
|
|
| Async model discovery | Stale scans/searches overwrite newer intent | Use request IDs/abort logic and apply only matching results |
|
|
| Error handling | One banner hides multiple failure sources | Separate validation, save, operation, and health error channels |
|
|
| Refresh policy | Polling during edits and transitions causes jitter | Make polling context-aware and favor event-driven revalidation |
|
|
| QA/regression | Happy-path tests miss timing bugs | Add interleaving tests with fake timers and stale-response scenarios |
|
|
|
|
## Sources
|
|
|
|
- Project context and current architecture: `D:\Documents\Github\ForgeLM\.planning\PROJECT.md`, `D:\Documents\Github\ForgeLM\.planning\codebase\ARCHITECTURE.md`
|
|
- Current runtime implementation: `tauri-app/src/features/runtime/useRuntimeManager.ts`, `tauri-app/src/features/runtime/components/RuntimeSettingsDrawer.tsx`, `tauri-app/src/features/runtime/runtime-settings-form.ts`, `tauri-app/src/features/runtime/components/RuntimeSurface.test.tsx`
|
|
- React official docs, "You Might Not Need an Effect" (official, high confidence): `https://react.dev/learn/you-might-not-need-an-effect`
|
|
- React official docs, `useSyncExternalStore` (official, high confidence): `https://react.dev/reference/react/useSyncExternalStore`
|
|
- React official docs, "Preserving and Resetting State" (official, high confidence): `https://react.dev/learn/preserving-and-resetting-state`
|
|
- Ecosystem evidence for recurring unsaved-change and race-condition bugs (community/GitHub, medium confidence):
|
|
- `https://github.com/calcom/cal.com/issues/28179`
|
|
- `https://github.com/calcom/cal.com/pull/22485`
|
|
- `https://github.com/formbricks/formbricks/pull/6199`
|
|
- `https://github.com/TanStack/form/issues/1457`
|
|
- `https://github.com/payloadcms/payload/issues/12691`
|