|
|
|
|
|
---
|
|
|
|
|
|
title: arts/ audit + fixes (ex-sium)
|
|
|
|
|
|
type: notes
|
|
|
|
|
|
audience: human + agent
|
|
|
|
|
|
status: current
|
|
|
|
|
|
source: directed grep/Read audit, each finding verified against the code
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
# arts/ audit — 2026-06-14
|
|
|
|
|
|
|
|
|
|
|
|
Audit of the runtime artifacts under `src/arts/` (excluding `sium`, audited and
|
|
|
|
|
|
extended separately). Goal: contract violations, dead/duplicated code, bad
|
|
|
|
|
|
patterns, doc drift. Method: directed grep/Read, **every finding verified by
|
|
|
|
|
|
reading the code** (no agent fan-out). Scope agreed: deep-read of the priority
|
|
|
|
|
|
arts (connection, prefs, color, bus, http) + cross-cutting sweep of the rest.
|
|
|
|
|
|
|
|
|
|
|
|
## Applied (this pass)
|
|
|
|
|
|
|
|
|
|
|
|
All outside `sium`; applied with explicit permission. Verified:
|
|
|
|
|
|
`npx vitest run src/arts/{connection,http,color}` → 183/183; type-check clean on
|
|
|
|
|
|
the touched files; touched files Prettier-clean.
|
|
|
|
|
|
|
|
|
|
|
|
- **`connection/index.ts` — barrel hygiene.** Removed `export * from './helpers.ts'`
|
|
|
|
|
|
(helpers is 100% internal — message/key builders, `createConnectionIdFactory`,
|
|
|
|
|
|
`timerKey`, `loggerScope` — leaked into the public `$connection` surface) and
|
|
|
|
|
|
replaced `export * from './errors.ts'` with a **named** export of the public
|
|
|
|
|
|
error surface (codes + classes + guards + `CONNECTION_ERROR_MESSAGES` +
|
|
|
|
|
|
`CONNECTION_ERROR_PREFIX`); the internal `CONNECTION_ERROR_MSG_*` strings no
|
|
|
|
|
|
longer leak. Safe: nothing imports the `$connection` barrel (active-app uses
|
|
|
|
|
|
deep imports `$connection/active-connections.svelte` + `$connection/types`).
|
|
|
|
|
|
- **`http/consts.ts` + `http/diagnostics.ts` — inline diagnostic message.** The
|
|
|
|
|
|
`ATTEMPT_COMPLETED` catalog entry was the only inline message literal in any
|
|
|
|
|
|
arts diagnostics catalog. Extracted to a named builder
|
|
|
|
|
|
`attemptCompletedLogMessage(method, url, attempt, ok, durationMs)` (matches the
|
|
|
|
|
|
sibling builders); message output preserved exactly.
|
|
|
|
|
|
- **`color/index.ts` — stale comment.** Removed "Phase 0 … not consumed yet"
|
|
|
|
|
|
(`uix.color` is consumed today by Eidos' color engine).
|
|
|
|
|
|
- **`connection` — removed `EngineConnections.close()`.** Exact redundant alias of
|
|
|
|
|
|
`closeConnection(name)` (the canonical name, consistent with `openConnection` /
|
|
|
|
|
|
`reconnectConnection` / `closeAll`). Removed from the interface (`types.ts`) +
|
|
|
|
|
|
impl (`engine-connections.ts`); `ActiveConnections` inherits the change via
|
|
|
|
|
|
`Omit`. **Correction:** the first-pass "nothing calls it" was wrong — a dedicated
|
|
|
|
|
|
test (`keeps close() safe when destructured`) exercised it (caught by the test
|
|
|
|
|
|
run). The destructure-safety guard was **retargeted to `closeConnection`**; that
|
|
|
|
|
|
property holds for every engine method (they are closures, not `this`-bound).
|
|
|
|
|
|
|
|
|
|
|
|
## Deferred / not actioned (with reason)
|
|
|
|
|
|
|
|
|
|
|
|
- **`connection/index.ts` consts curation.** `export * from './consts.ts'` still
|
|
|
|
|
|
leaks ~100 consts, mixing public (states, events, defaults, reasons) with
|
|
|
|
|
|
internal (`*_LOG_MSG_*`, `*_METHOD_*`, `*_TIMER_KEY_*`, `*_WEBSOCKET_*`,
|
|
|
|
|
|
`*_BROWSER_EVENT_*`, the session-event mirror). Curating public-vs-internal is
|
|
|
|
|
|
an **API-surface decision the maintainer should own**, not a mechanical fix —
|
|
|
|
|
|
deferred.
|
|
|
|
|
|
- **`bus/engine-bus.ts:48` — `IS_DEV` via `process.env.NODE_ENV`.** Real gap (the
|
|
|
|
|
|
dev-only `assertPayloadCloneable` guard never fires in browser-dev, where
|
|
|
|
|
|
`process` is undefined). **NOT changed**: the proposed `import.meta.env.DEV`
|
|
|
|
|
|
is Vite-only and would break the art under plain Node/Bun/Deno/Workers — the
|
|
|
|
|
|
arts are framework-agnostic, so the guarded `process.env` is the more portable
|
|
|
|
|
|
choice. The browser-dev gap is an accepted trade-off.
|
|
|
|
|
|
- **`color/*` (5×) + `prefs/dimensions/*` (3×) inline throws.** `color` is pure
|
|
|
|
|
|
math with no `errors.ts`; the prefs throws are factory-argument guards
|
|
|
|
|
|
(`TypeError('catalog must not be empty')`), which `prefs/errors.ts` explicitly
|
|
|
|
|
|
scopes out ("exceptions reserved for engine misuse / data errors"). Both are
|
|
|
|
|
|
**defensible** as idiomatic programmer-error guards; a typed error would be
|
|
|
|
|
|
overkill. Left as-is.
|
|
|
|
|
|
- **Diagnostics factory naming** (`createAuthClientDiagnostics` /
|
|
|
|
|
|
`createActiveCacheDiagnostics` / `createOrcaDiagnostics`). Homogeneity nit; a
|
|
|
|
|
|
cross-art rename touches every factory + caller for low value — deferred.
|
|
|
|
|
|
|
|
|
|
|
|
## Not actioned — clean
|
|
|
|
|
|
|
|
|
|
|
|
Light-scan (cross-cutting) of auth · cache · session · storage · timer · orca ·
|
|
|
|
|
|
langs · format · motion · clipboard · adom · active-app: **no cross-cutting
|
|
|
|
|
|
violations** — named barrels, throws only in tests, zero TODO/FIXME/HACK, casts
|
|
|
|
|
|
and `eslint-disable`s justified, diagnostics use named consts. `arts/` is, in
|
|
|
|
|
|
general, very well maintained. `connection` was the only barrel violator.
|
|
|
|
|
|
|
|
|
|
|
|
## Bonus — QrCode morfo bug, fixed
|
|
|
|
|
|
|
|
|
|
|
|
Separately, the QrCode component failed morfo validation: `MorfoElement`
|
|
|
|
|
|
(`uix/morfo/types.ts:79`) lists `'path'` but the runtime `elementSchema`
|
|
|
|
|
|
(`uix/morfo/schema.ts`) was missing `literal('path')` — a type↔validator drift
|
|
|
|
|
|
hidden by the `as Schema<MorfoElement, MorfoElement>` cast on that union. Added
|
|
|
|
|
|
`literal('path')`; `npm run morfo:check` → `PASS qr-code`. (combobox + words
|
|
|
|
|
|
still fail morfo:check — pre-existing, unrelated; words is an excluded track.)
|
|
|
|
|
|
Hardening worth doing: drop the `as` cast so TS catches the next drift.
|
