evandro/ForgeLM
1
0 Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Initial ForgeLM import

Browse Source
This commit is contained in:
Evandro Fonseca Junior committed 2026-03-29 19:10:57 -03:00
commit a392b9dc60
4296 files changed
+1212675
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

.planning/research/PITFALLS.md
+164
View File
@@ -0,0 +1,164 @@
# 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`