Add strictNullChecks ratchet + fix the core service layer (#6)

[mairas]

First slice of #6. Enabling strictNullChecks surfaces 959 latent null-safety issues across the app — too many to fix in one PR, and most concentrated in the ~46 widgets. This PR builds an incremental ratchet and clears the highest-value slice: the core service layer, where Signal K null actually flows.

Approach

strictNullChecks stays off in the app build (flipping it globally would fail on 959 errors). Instead:

• tsconfig.strict.json enables the flag for npm run snc and for the ratchet.
• betterer snapshots the error set to a committed .betterer.results baseline and fails CI (strict-null-checks job) only on new issues. The count ratchets down per PR; nothing can regress.
• tsconfig.betterer.json gives ts-node a commonjs config so it can load .betterer.ts (the app's bundler moduleResolution is incompatible with the commonjs ts-node forces).

This slice: core service layer (116 fixed, 959 → 843)

Types are made honest, not silenced — zero non-null assertions, casts, or suppressions. Genuinely-nullable values get widened types + handling at use; lazy initialisers get real defaults.

• SK pipeline — widen ISkPathData.type/pathTimestamp and make ISkMetadata.units optional to match real delta/meta ordering; endpoint/version/request fields typed to their genuine pre-connect null states, with explicit throws where a missing HTTP body would otherwise fail obscurely.
• Config/persistence — ConfigurationUpgradeService now skips-and-logs configs missing their app/theme section instead of throwing TypeError on real device data; lying non-null return types corrected.
• Dashboards/app — screensPayload keeps its undefined/null "clear" sentinel honestly; toSignal reads guard the undefined-before-first-emit case; nav fallbacks preserve prior numeric-coercion behaviour.

Two out-of-scope call sites (data-inspector, path-control-config) picked up the widened SK types and are handled in kind — path-control-config's guard fixes a latent compare(null) crash the honest typing surfaced.

Remaining (follow-up PRs under #6)

The ~46 widgets and remaining components (843 issues) migrate in later slices, each ratcheting the baseline down. widget-data-chart (164) and widget-windtrends-chart (100) dominate.

Verification

npm run lint, npm run build:dev (AOT + strictTemplates), and betterer:ci all pass. No file regressed vs the baseline.

Refs #6

[mairas]

Code review — PR #208 (strictNullChecks ratchet + core service layer)

Scope: 17 changed source files + 4 infra files vs main (base 6ded2346). Mode: interactive, 7 persona reviewers (correctness, testing, maintainability, project-standards, api-contract, reliability, adversarial), each in its own fresh context.

Intent: enable strictNullChecks incrementally via a betterer ratchet and fix the core service layer's null-handling, preserving behavior — no !, casts, or suppressions.

Headline: the ratchet mechanism and the type-safety work are sound — correctness and api-contract both returned clean, and every widening was confirmed to match pre-existing runtime nullability (these are internal interfaces, not an external API). The findings are two behavior divergences introduced by over-eager agent "improvements", a documentation/consistency gap in the ratchet infra, and a pervasive lack of tests for the behavior this PR made explicit.

P2 — should fix

#	File	Issue	Reviewers	Conf	Route
1	signalk-connection.service.ts:316	processEndpointResponse now fails loud where base degraded gracefully. New if (!httpUrl || !wsUrl) throw. On plain-HTTP + non-proxy, a server exposing v1 signalk-http but no signalk-ws used to yield a usable app (config/REST loads, only live data dead); it now fails the whole bootstrap → operation:3 → StorageService never ready → app-init waitUntilReady times out. Regression for that input class.	adversarial, reliability, correctness, testing	0.70	gated — product decision (fail-fast vs partial-connect) + add test
2	configuration-upgrade.service.ts:220	startFresh retire loop: abort → skip-and-continue. A partial/app-less legacy slot used to abort the whole retire loop with a loud "Error fetching"; it now skips that slot and continues, retiring more slots and emitting a quieter per-slot pushError. Real divergence in end state + surfaced errors. Related: the sibling legacy runUpgrade(undefined)/transformConfig path still derefs config.app unguarded and its catch never resets upgrading, so it can wedge the upgrade overlay (pre-existing).	adversarial (0.70), reliability, testing	0.70	gated — confirm best-effort intended; fold in the wedge guard
3	CLAUDE.md	Ratchet + baseline-regeneration footgun is undocumented. Nothing documents betterer, that .betterer.results must be regenerated after fixing a file or merging main (author already hit this — commit 2a27734e), or that CI runs on Node 24. Next dev hits spurious CI failures with no pointer.	maintainability	0.85	safe — add a CLAUDE.md section

P3 — fix if straightforward

#	File	Issue	Reviewers	Conf	Route
4	.betterer.ts:5	betterer's file scope diverges from npm run snc. The glob .include('./src/**/*.ts').exclude(/\.spec\.ts$/) pulls in src/test.ts (the vitest setup file), which tsconfig.strict.json excludes — confirmed: .betterer.results has a src/test.ts entry snc never shows. The four compilerOptions/scope settings are also duplicated between the two files and can silently drift.	maintainability	0.66	safe — add src/test.ts to betterer exclude, regen baseline
5	signalk-delta.service.spec.ts:334	updates win test doesn't pin the no-context emission. parseUpdates widened to string | undefined; the only no-context test asserts count only, not context: undefined. A refactor coercing undefined→'' would pass every test.	testing (0.80), correctness, maintainability	0.80	manual — extend the assertion
6	configuration-upgrade.service.ts:~189	transformApp/transformTheme | null returns aren't guarded before JSON.stringify on the v10 path. Unreachable today (defaults substituted) but inconsistent with the sibling startFresh guard; a future null input persists the string "null" and boots config-less.	api-contract, maintainability, reliability	0.62	gated — add the symmetric guard

Testing (cross-cutting)

The PR adds zero tests, and the delta message-drop regression (caught by vitest, not type-checking) already proved the ratchet guards types, not behavior. Behavior this PR made explicit is uncovered: processEndpointResponse throw matrix ({proxy,non-proxy}×{http,https} × ws-absent), configuration-upgrade skip-and-log guards, buildWebSocketUrl throw, dashboard null-active nav fallback, and the no-context DataService path key. notifications.getNotificationSeverity's new guard is provably unreachable (caller pre-continues) and needs no test.

Pre-existing (action required)

File	Issue	Route
data.service.ts:614	Context-less delta messages are misfiled under a "undefined.<path>" key (setPathContext(undefined, …) → undefined !== _selfUrn → `${undefined}.${path}`); widgets subscribed to self.<path> never see that data. Confirmed byte-identical to base — the honest typing surfaced but did not fix it. Real SK servers always send context, so it's a latent quirk.	file-issue

Coverage

• 7/7 reviewers returned. No P0/P1. Correctness and api-contract clean.
• Suppressed (<0.60): setMeta storing undefined vs null (0.4, verified safe); a couple of unreachable-guard notes.
• No security persona selected — the diff touches no auth/permission/input-trust boundary, only null typing.

---

Verdict: Ready with fixes. The ratchet and type-safety goal are solid; nothing here undermines them. Before merge: decide on the two behavior divergences (#1 processEndpointResponse fail-loud, #2 startFresh abort→skip) — both are agent "improvements" that trade the PR's behavior-preservation constraint for arguably-better behavior — apply the safe doc/scope fixes (#3, #4), and file an issue for the pre-existing misfiling. The testing gap is the main residual risk given the ratchet can't catch behavior drift.

[mairas]

Review findings addressed (commits 1bda878, 69b6b5, 3740807)

Decisions

• Replace icon set with the SKip compass logo #1 processEndpointResponse fail-loud — kept as-is (deliberate). A SK server exposing HTTP but no WebSocket can't drive a live dashboard, so a clean "did not become ready" failure beats a half-working app. Now covered by a test pinning the contract.
• Fix CI: repair vitest test suite (inherited from upstream) #2 startFresh abort→skip — accepted the more-robust skip-and-continue semantics; folded in the sibling legacy-path wedge fix.

Applied

• Fix CI: repair vitest test suite (inherited from upstream) #2 wedge: the version-less legacy upgrade path now resets upgrading() in its catch (was leaving the overlay stuck) and transformConfig skips app-less slots instead of throwing TypeError.
• [PCS-01] Missing-config recovery broken: 200 {} from SK triggers no recovery prompt #3 docs: CLAUDE.md now documents the ratchet + the content-hash baseline-regeneration footgun.
• [RED-01] Electrical widget family: copy-pasted implementation across 6 widgets (~5,000 LOC) #4 betterer scope: excluded src/test.ts to match npm run snc; tsconfig.strict.json is now the single source of the strict options. Baseline 843 → 842.
• Tests (the cross-cutting gap): new signalk-connection.service.spec.ts (processEndpointResponse operation-2 + both throw paths); configuration-upgrade skip-and-log + wedge cases; delta updates win now pins context: undefined.

Deferred

• Pre-existing "undefined.<path>" misfiling → filed as DataService misfiles context-less delta values under an "undefined.<path>" key #209 (low priority; real servers always send context).
• [TS-01] No strictNullChecks: ~65 type annotations lie, hiding null-safety bugs at runtime #6 transformApp/Theme JSON.stringify(null): unreachable by construction (the as IAppConfig cast guarantees non-null input → non-null output; loadConfigFromLocalStorage substitutes defaults). Left as-is per YAGNI rather than guard an impossible branch.

All local gates green: lint, build:dev (AOT/strictTemplates), betterer:ci, and the three affected specs.