OSIT-AE-App-Svelte/documentation/REFERENCE__Common_Agent_Mistakes.md

# Aether SvelteKit — Common Agent Mistakes (Reference)

This is the detailed mistake catalog referenced by `BOOTSTRAP__AI_Agent_Quickstart.md`.
Use it as a troubleshooting and prevention guide, not as first-pass onboarding.

Review policy: balanced curation.
- Keep: high-impact or recurring mistakes.
- Archive: one-off or lower-signal historical incidents.

---

## Active Mistakes (Curated)

### 1) IDAA content exposed publicly
**Impact:** Sev-1 privacy breach.

**What happened:** A route guard was removed on an IDAA BB route.

**Rule:** All `/idaa/` content is private. Never remove auth guards on IDAA routes.

**Verify:** Confirm route/layout auth checks still gate all IDAA pages before data load.

### 2) Object ID included in PATCH body (`data_kv`)
**Impact:** 400 errors (`Unknown column in SET`).

**What happened:** Object ID (for URL path) was also sent in `data_kv`.

**Rule:** Keep object ID in the URL only; `data_kv` contains changed fields only.

**Verify:** In `update_ae_obj__*` calls, confirm `data_kv` excludes `*_id` fields.

### 3) Coarse-store `$effect` reactivity loops
**Impact:** repeated fetches, duplicate side effects, noisy state churn.

**What happened:** `$effect` read fields from `svelte-persisted-store` stores (`$ae_loc`, `$idaa_loc`), subscribing to entire store.

**Rule:** Be minimal about coarse-store reads in `$effect`; move transient triggers to session state and keep duplicate guards in executor paths.

**Verify:** Check effect dependencies and ensure unrelated writes do not retrigger critical effects.

### 4) Dexie `.get()` used with string object ID
**Impact:** false misses (`undefined`) despite cached data.

**What happened:** `.get()` queried primary key `id`, but V3 records rely on object string IDs (e.g. `person_id`).

**Rule:** Use `.where('<obj_id>').equals(value).first()` for object lookups.

**Verify:** Search for `.get(` against object IDs and replace with indexed `where` query.

### 5) Misunderstanding `$effect` and auth gates
**Impact:** security confusion and wrong mitigations.

**What happened:** Child-component `$effect` blocks were treated as possible bypass vectors.

**Rule:** Child effects cannot bypass parent layout auth gates; pre-gate risk is in universal `+page.ts` / `+layout.ts` loads and prefetch.

**Verify:** Keep private-module data loads out of universal load files unless explicitly authenticated.

### 6) Using query `key` like `x-no-account-id: bypass`
**Impact:** dropped `x-account-id`, unexpected 403 on scoped requests.

**What happened:** `key` was treated as bypass intent and account context was stripped.

**Rule:** `key` is business data, not bypass intent. Only explicit `x-no-account-id: bypass` drops account context.

**Verify:** Check request headers for account-scoped calls and preserve `x-account-id` unless bypass is explicitly required.

### 7) Pre-stringifying `*_json` before API wrappers
**Impact:** double encoding risk and inconsistent payloads.

**What happened:** Callers stringified JSON fields manually before wrapper serialization.

**Rule:** Pass plain objects for `*_json` fields; wrappers handle serialization.

**Verify:** Remove `JSON.stringify(...)` around `cfg_json`, `data_json`, and related fields before wrapper calls.

### 8) Broad Dexie result windows silently clipped
**Impact:** “All” views show fewer rows than narrower filters.

**What happened:** page limits or API revalidation replaced local broad result sets.

**Rule:** For empty text search, local IDB result set should drive visible results; server refresh updates cache without shrinking display.

**Verify:** Compare empty-search “All” view vs narrower filter counts and inspect revalidation merge behavior.

### 9) Missing `IDB_CONTENT_VERSIONS` bump after `properties_to_save` changes
**Impact:** long-lived stale cache bugs (e.g. IDAA “no meetings found”).

**What happened:** shape persisted in IDB changed, but table content version was not bumped.

**Rule:** When persisted object shape/behavior changes, bump matching `IDB_CONTENT_VERSIONS` entry in `src/lib/stores/store_versions.ts`.

**Verify:** For relevant object changes, confirm both version increment and table-wiring path are in the same change set.

### 10) API retry loop broken by returning transient errors
**Impact:** no retries on common WiFi/network blips.

**What happened:** transient error paths returned `false` instead of throwing, bypassing retry loop.

**Rule:** Keep retry classification strict:
- transient `TypeError` and timeout aborts -> `throw`
- navigation abort -> `return false`
- deterministic 4xx -> `return false`
- 5xx -> `throw`

**Verify:** Simulate transient network failure and confirm retry/backoff attempts occur.

### 11) Account-scoped trigger fired before bootstrap account is ready
**Impact:** wrong-account API fetch and cached cross-account data.

**What happened:** trigger effects ran before account bootstrap settled, reading stale context.

**Rule:** Gate account-scoped load triggers on `$slct.account_id` readiness, not persisted `$ae_loc.account_id`.

**Verify:** Ensure trigger effects require browser + account_id + api readiness before fetch trigger assignment.

### 12) `tmp_sort_*` comparator direction inverted
**Impact:** priority ordering reversed.

**What happened:** descending comparator used with `build_tmp_sort` encoding (which expects ascending).

**Rule:** Use ascending compare for `build_tmp_sort` outputs, with documented exceptions for legacy encodings.

**Verify:** Confirm comparator direction per module encoding and avoid `collection.reverse().sortBy(...)` assumptions.

### 13) `$` sigil used on plain prop values
**Impact:** runtime `store_invalid_shape` errors.

**What happened:** child component treated normal prop values as Svelte stores.

**Rule:** In Svelte 5, props passed as values are plain values. Do not use `$prop` unless prop is an actual store.

**Verify:** In migrated components, replace `$lq__...` prop reads with plain `lq__...` prop access.

### 14) Null JSON blob fields not guarded
**Impact:** read/write crashes (`cannot access property of null`).

**What happened:** `*_json` / `*_kv_json` DB columns were null before first write.

**Rule:** Use optional chaining for reads and `?? {}` initialization before object spread writes.

**Verify:** Audit reads/writes for `cfg_json`, `data_json`, `poc_kv_json`, and similar nullable blob fields.

### 15) Incomplete cache clearing — SW and Cache Storage omitted

**Impact:** Sev-1 class. Users stuck on old JS for weeks after deployments. “Fixed” bugs
reappear because the affected user never loaded the fix. Nearly cost the IDAA client after
4+ months of recurring reports that were repeatedly declared resolved.

**What happened (two separate gaps, both required to fix):**

**Gap A — Clear buttons did not clear the SW or Cache Storage.**
The app has a service worker that caches every JS/CSS/HTML asset in its own Cache Storage
(separate from IndexedDB and localStorage). All “clear caches” buttons only cleared IDB,
localStorage, and sessionStorage — leaving the SW and its Cache Storage intact. On the next
reload, the SW intercepted the request and returned the OLD cached JS bundle. The user appeared
to reload but was still running old code. Clicking “Clear Storage and Reload” appeared to work
but did nothing to break the stuck state.

**Gap B — No `controllerchange` listener in the layout.**
`service-worker.js` correctly calls `skipWaiting()` on install (new SW activates immediately)
and `clients.claim()` on activate (new SW takes control of all open tabs). But the page-side
half of this contract was missing: there was no `controllerchange` event listener to trigger
a reload when the new SW took over. Result: every deployment correctly installed and activated
a new SW, but any tab that was already open kept running the old JS bundle from memory —
indefinitely, until the user happened to close and reopen the browser.

**Who was affected:** Windows users who leave Chrome/Edge running for days or weeks with the
tab pinned or restored via “continue where you left off.” Most users were never affected
because normal browser habits (restart, close/reopen tab) naturally clear the stuck state.
The pattern looked random but was actually a long-session-duration problem.

**The fix (2026-06-22):**
1. Every “clear” button and automated heal path now clears SW registrations + Cache Storage
   BEFORE clearing IDB/localStorage. Order matters: SW → Cache Storage → IDB → storage.
2. `controllerchange` listener added to `+layout.svelte` effect 4 — reloads the page when
   a new SW takes control, so open tabs automatically get fresh JS after any deployment.
3. `idaa/clear-caches/+page.svelte` (IDAA iframe tool) updated with the same fix.

**Rule:** Any code that clears caches MUST clear all four layers in this order:
1. `navigator.serviceWorker.getRegistrations()` → unregister each
2. `caches.keys()` → delete each key
3. `indexedDB.databases()` → delete each database
4. `localStorage.clear()` + `sessionStorage.clear()`

Clearing IDB/localStorage WITHOUT clearing SW and Cache Storage is not a fix — it is a
placebo that makes the user think something changed while the old JS bundle is served again
on the very next reload.

**Verify:** After any future changes to cache-clearing paths, confirm all four layers are
cleared. The `controllerchange` listener in `+layout.svelte` effect 4 must not be removed.
`/testing/fix-sw` is the emergency escape route for users who are stuck on pre-fix code.

### 16) Local "shadow field" silently bypasses the admin-synced master field
**Impact:** an admin config toggle appears to do nothing — the UI updates fine when a local
per-browser preference is clicked, but the actual admin setting has zero effect, at any
permission level. Looks like a permissions bug; isn't one.

**What happened:** A list/table column's visibility prop was computed from only a local-only,
never-synced preference field (e.g. `hide__session_li_location_field`), while a
similarly-named, admin-synced field (`hide__session_location`) existed and was correctly
synced — just never read at that particular render call site. Found twice in the same
session (Pres Mgmt POC column, then Location column).

**Rule:** When a remote config field and a local-only preference field could plausibly both
affect the same visible thing, combine them explicitly (`admin_field || local_field`) at
*every* call site that renders that thing — don't assume one supersedes the other, and don't
assume fixing it once means every other render site is also fixed.

**Verify:** `grep` every consumer of the field name (and near-miss siblings, e.g. `_li_`/`_field`
suffixed variants) before trusting that a config toggle does what its label says.

### 17) SWR `await` after a write does not mean dependent caches are fresh
**Impact:** a value you just saved appears stale or "one save behind" immediately after
saving — looks like inverted/random behavior when toggling a boolean back and forth, since
being one step behind on a 2-state toggle looks exactly like being inverted.

**What happened:** A save handler `await`ed `load_ae_obj_id__event()` (SWR) and assumed that
meant Dexie now had the fresh record. In reality the fast path returns the stale cache
immediately and refreshes Dexie via a non-awaited background fetch. A *different* page's own
sync `$effect` read Dexie before that background fetch landed, re-syncing from the stale
record and overwriting a value that had just been set correctly moments earlier.

**Rule:** After a write whose result other reactive code depends on, don't rely on an awaited
SWR reload to mean downstream caches are updated — for any call with a populated cache, it
almost never is. Update dependent local state directly from the data you already have (what
you just saved), not by waiting on a refetch. See the "try_cache: false Bug" section of
`GUIDE__SvelteKit2_Svelte5_DexieJS.md` for the related, equally counter-intuitive case where
disabling caching *also* disables the very write you wanted.

### 18) Conditional ("locked") sync gates are effectively undebuggable
**Impact:** a field's local value silently depends on the *history* of an unrelated flag's
state across past syncs, not its current value — looks like inconsistent behavior tied to
permission level, browser, or test sequence, none of which is the actual cause.

**What happened:** `sync_config__event_pres_mgmt()` only applied a subset of fields when a
separate `lock_config` flag was `true` *at that exact sync call*. Toggling `lock_config` off
even briefly during testing froze those fields at stale values in every browser that synced
during that window, with nothing in the UI indicating staleness. Removed entirely — every
event already had it set to `true` in production; the conditional gated nothing real.

**Rule:** Avoid making one config field's sync conditional on another field's *current*
value unless that's a genuine, deliberate design choice — and if it is, surface the
dependency in the UI (disable the inputs, show a warning), don't bury it in a silent
function-level gate.

**Verify:** Before adding `if (some_flag) { ...many field syncs... }`, check whether removing
the gate (sync unconditionally) loses any real behavior — query production data for whether
the gate is ever actually in the "off" state before assuming it needs to exist.

### 19) `param?: Type` in `.svelte` function or snippet parameter positions

**Impact:** `vite build` SSR step fails with "Expected ',', got '?'" — Docker builds and
production deploys break. Dev server is unaffected (different code path), so the error is
invisible in local development and only surfaces in CI/CD or the first clean Docker build.

**What happened:** Source files used TypeScript optional-parameter syntax (`param?: Type`)
inside `.svelte` `<script lang="ts">` functions and `{#snippet}` parameter lists. esbuild
(Vite's dep pre-bundler/SSR transform) strips `: Type` from the type annotation but leaves
the `?` marker, producing `param?` — which is invalid JavaScript. Rollup then fails to parse
the compiled output, showing the original `.svelte` line number via source maps.

Five occurrences were found and fixed in 2026-06 across:
`reports_file_downloads.svelte`, `ae_comp__journal_entry_obj_id_view.svelte`,
`ae_comp__modal_journal_entry_config.svelte`, `launcher_file_cont.svelte`,
`ae_comp__badge_print_controls.svelte` (Svelte 5 `{#snippet}` param).

**Rule:** Never write `param?: Type` in function or snippet parameter positions inside
`.svelte` files. Use `param: Type | undefined = undefined` instead — this preserves the
same runtime behavior (calling without the argument passes `undefined`) and produces valid
JavaScript after TypeScript stripping.

```ts
// ❌ Breaks vite build (SSR environment):
function clean_part(s: any, max?: number): string { ... }
{#snippet field_actions(on_save: () => void, on_revert?: () => void)}

// ✅ Correct:
function clean_part(s: any, max: number | undefined = undefined): string { ... }
{#snippet field_actions(on_save: () => void, on_revert: (() => void) | undefined = undefined)}
```

This restriction applies **only inside `.svelte` files** — TypeScript `.ts` files are
processed by the TypeScript compiler directly and handle `?: Type` correctly. Interface and
type alias blocks inside `<script lang="ts">` are also fine (the Svelte compiler handles
those natively); the restriction is only for runtime function/snippet parameter lists.

Note: `scripts/postinstall.mjs` applies the same fix to flowbite-svelte's published dist
files, which ship with TypeScript optional params in function signatures. That script needs
to be maintained whenever flowbite-svelte is updated — check with:
`grep -rn "?:" node_modules/flowbite-svelte/dist/ | grep "function\|=>"`.

**Verify:** After writing any new function or `{#snippet}` in a `.svelte` file, scan for
`\w\?:` in the parameter list. Run `npm run build:dev` locally before any Docker build or
deploy — it will catch this within seconds.

---

## Archived Historical Items (Pruned)

These are retained as project memory but removed from the active mistake list because they are lower-signal or one-off.

1. **Bad `.d.ts` module declaration masking errors** (important incident, low recurrence recently).
2. **Launcher `file_purpose == 'admin'` filtering gap** (specific historical enum-audit miss).
3. **Deleting files with `rm`** (still a workflow rule, now maintained in bootstrap File Safety section rather than as a mistake entry).

---

## Related Docs

- `documentation/BOOTSTRAP__AI_Agent_Quickstart.md`
- `documentation/TODO__Agents.md`
- `documentation/GUIDE__SvelteKit2_Svelte5_DexieJS.md`
- `documentation/GUIDE__AE_API_V3_for_Frontend.md`
- `documentation/PROJECT__Stores_Svelte5_Migration.md`