⭐ Code Review Audit - Terrence721/platform-main GitHub Wiki

A per-module, per-file code review of all 13 added modules, run for real — a GitHub issue, a branch, and a merged PR for every source file, not a simulated pass. Parent tracking issue #32, one sub-issue per module (#33–#45). One module worked at a time, in the order modules were originally added.

Status: store complete — 27/27 files, 3 real bugs found and fixed, 1 test-coverage gap closed. entity complete — 9/9 files, 0 real bugs, 2 structural observations recorded. effects complete — 18/18 files, 0 real bugs found. router-store complete — 12/12 files, 7 real bugs found and fixed (type-level declared-vs-actual mismatches, a false JSDoc claim caught by un-skipping a disabled test, and a field silently sourced from the wrong object). store-devtools complete — 11/11 files, 6 real bugs found and fixed plus 1 minor cleanup, including a crash reachable from a public API method (toggleAction()) that could have taken down an app's entire reactive state stream, not just its devtools panel, when the reducer's history was auto-committed out from under it. component-store complete — 4/4 files, 1 real gap found and fixed (selectSignal()'s SelectSignalOptions/SignalsProjector types were never exported, so the public barrel never re-exported them despite the public method needing them). component complete — 10/10 files, 1 real bug found and fixed plus 2 barrel-export gaps fixed (a shared render-event dedup layer was silently swallowing a second, genuinely new error notification when two different sources errored with an equal value back-to-back — caught while reviewing push.pipe.ts, also fixed the same bug in let.directive.ts; the barrel gaps were LetViewContext/PushPipeResult never reaching index.ts). operators complete — 4/4 files, 0 real bugs, 2 barrel-export gaps fixed (MapResponseObserver/TapResponseObserver were never exported from their own files, so the barrel never re-exported them). schematics-core complete — 16/16 files, 9 real bugs found and fixed (a stale prerelease version floor; a missing-project ng add error that surfaced a raw TypeError instead of a clear message; findModuleFromOptions never actually auto-detected a module when --module was omitted, because every real caller guarded the call itself — confirmed via a real pre-existing snapshot, fixed for store/reducer/entity/feature/most ng-add schematics, deliberately left as-is for effect/component-store; a missing-workspace-file error that surfaced a confusing Could not find (undefined) instead of a clear message; addReducerToActionReducerMap crashing on a reducers/index.ts with any typed declaration ordered before the ActionReducerMap-typed one; visitDecorator handing callers a ClassExpression typed as a ClassDeclaration; a cross-file config path built with OS-native separators that broke on Windows; and replaceImport leaving a dangling comma when removing the last import specifier) plus 2 test-coverage gaps closed, 13 barrel-export gaps closed across 6 files (index.ts — the largest barrel-gap finding of any module in this audit), and 2 dead-code/vestigial-comment cleanups. signals complete — 18/18 files, 14 defects fixed across 11 files (deep-signal.ts 1 severe real bug, the most severe found in the whole audit — a state property named set/update/asReadonly silently and permanently broke patchState with no error, because the deep-signal cache was written directly onto the real underlying signal via Object.defineProperty, fixed with a side WeakMap cache instead; signal-store-assertions.ts a false-positive dev-mode warning fixed in its withProps/withMethods callers; signal-store-feature.ts a parameter-name typo in a public overload; signal-store-models.ts a symbol-key gap in MethodsDictionary; signal-store.ts an untested providedIn: 'platform' option covered; state-source.ts a quadratic getState and three watchState registration bugs; deep-computed.ts, signal-method.ts and signal-state.ts no findings). schematics complete — 25/25 files (standalone-app failures, generated code that did not compile, vacuous tests). data complete — 61/61 files, 42 needing fixes (among them a collection-reducer error that stopped the store under NgRx's default runtime checks). eslint-plugin complete — 55/55 files, 38 needing fixes (an aliased Store import silently turned off 14 rules; auto-fixes that broke working code; functional effects, standalone providers and stores held in variables now covered). The audit is complete: all 13 modules, 270 files — #32 closed 2026-09-30. The type-level gap that let router-store's bugs through was significant enough to spawn its own tracking work — parent issue #162 with one child issue per module (#163–#175), adding type-check tests alongside this file-by-file review rather than as an afterthought.

All 3 real bugs found in store share one shape: a function's top-level entry point missing a null-state guard its own recursive/nested-value logic already had (freeze(), getUnserializable(), combineReducers()) — inherited from the original upstream import, not introduced by this repo, each reproduced with a failing regression test before being fixed. Full findings index, file by file, is docs/code-review.md; live per-file tracking table is in todo.md.

After the audit — the migrations/ folders (#106, complete 2026-10-01): the 31 files ng update runs, outside the audit's src/ scope, reviewed the same way with one issue and PR each. 27 had defects and were fixed, 4 had none. The recurring defects: aliased, type and namespace imports missed or corrupted; @ngrx/store matching @ngrx/store-devtools by prefix; formatting and comments lost; object keys renamed along with real references. Two fixes landed in shared schematics-core helpers every migration uses. Per-file details: docs/code-review.md.

After the audit — type-level coverage (#162, complete 2026-10-01): every module now has type tests of its public API, imported through the real package name. They found 11 declared types that did not match what the code returns, and writing them found 3 runtime bugs (MockStore.select with a key path lost keys; component-store's select(fn, {}) threw; ng add @ngrx/data could not turn effects off). Per-module table: docs/code-review.md.

Then the Helpdesk app (#1001, complete 2026-10-09): all 123 source files of its five projects, bottom-up through the dependency chain (contract, server, API, app, e2e), each with its own sub-issue and PR. helpdesk-contract 11 files, 4 bugs fixed; helpdesk-server 23 files, 7 bugs and gaps fixed in their own PRs and 3 more in the files' own; helpdesk-api 24 files, 7 bugs and gaps fixed, among them the development secret accepted for production sessions and a sign-in with no attempt limit; helpdesk 63 files, 15 with no findings; helpdesk-e2e 2 files, a bug fixed in each. A bug fix came with a test that failed without it, or a check on the real Docker stack or in the browser where no unit test could show it. Findings: docs/helpdesk-code-review.md.