Architectural decisions and constraints

This page records decisions that are already made. They are not up for re-litigation inside a feature task; if one needs revisiting, that is its own scoped piece of work with its own spec.

Standing decisions

D1 — Versions are pinned; do not upgrade opportunistically

Decision. Angular 12.2.15, TypeScript 4.3.5, NgRx 4.1.1, RxJS 6.5.4, AG Grid 23.x, Node 12.21.0, Karma/Jasmine as the test runner. No dependency bumps, no framework migration, no new state library, no swapping the test runner unless a task explicitly asks for it.

Why. The blockers compound:

NgRx 4 → modern NgRx is a rewrite of every action, reducer, effect and selector in twelve slices.
rxjs-compat is load-bearing — legacy deep imports (rxjs/observable/forkJoin, rxjs/observable/ErrorObservable) appear across dozens of files.
Node 12.21.0 constrains every build tool, and modern tooling (including Antora) will not run on it.
AG Grid 23 → current is several breaking majors and touches every grid.
@angular/http (HttpModule) is still imported in AppModule.
Application code imports Node’s util (isUndefined, isArray, …), which depends on the webpack 4 Node polyfill.

Consequence. You will meet APIs that look outdated. Match them.

D2 — Feature-first, fractal folder structure

Decision. Organise by feature, recursively, rather than by file type. Reject Angular’s "as flat as possible" guidance.

Why. The repository’s own conventions state it: a flat structure "leads to designs that are very difficult to maintain", and the folder tree should reflect the business and UI structure. With ~420 components, a flat layout is unnavigable.

Consequence. model/, services/, store/, shared/ recur at several depths. Placement is decided by scope — see the module map.

D3 — Selective state, not total state

Decision. Only cross-screen state goes in the NgRx store. Feature-local state stays in the component. Do not add a slice for something one screen owns.

Why. NgRx 4 boilerplate is expensive per slice, and the twelve existing slices already carry the shared concerns (auth, current supplier, CRM target, layout, reference data, settings).

Consequence. Most of the app is plain components calling services. Shared state also lives in BehaviorSubject caches inside services (UsersService, FeatureFlagsService) and in the TableEventsService bus — check those before adding a slice.

D4 — Everything is eagerly loaded

Decision. One bundle; no loadChildren in the live route tables.

Why. Internal tool, small and stable user population, no cold-start pressure on first paint from a marketing perspective. Lazy loading was never introduced, and route guards (FeatureFlagGuard implements CanLoad) hint that it was considered.

Consequence. The AOT build needs a raised heap (--max_old_space_size 8192 locally, 12288 in deployment/build.sh). The initial bundle is large. Do not add loadChildren to a single module in isolation — the gain is nil until the pattern is applied broadly, and it would change guard semantics.

D5 — Hash routing

Decision. useHash: true.

Why. Static hosting on S3 behind CloudFront without server-side rewrite rules for deep links.

Consequence. URL manipulation in code has to strip /#/ — see RouteAccessResolver.handleLogout(). Changing this requires hosting changes and invalidates every bookmark.

D6 — Authorisation is advisory in the client, authoritative in the backend

Decision. The front end hides and gates UI with ngx-permissions, but every protected operation is enforced server-side.

Why. A JavaScript bundle cannot keep a secret. Client-side gating is a usability and least-surprise measure.

Consequence. Never treat a guard as a security control. A missing guard is a UX bug; a missing backend check is a security bug. See authorisation.

D7 — Feature flags on everything new, by default

Decision. New modules and routes are flag-gated. The repository README states it in capitals: flags "SHOULD BE USED ON ALL MODULES BY DEFAULT".

Why. Trunk-based deployment to many environments from develop, with production released from release. Flags decouple deploy from release.

Consequence. Three flag layers (build, global settings, session) with a defined precedence, plus a guard and a structural directive. See feature flags.

D8 — Configuration is generated, not committed by hand

Decision. src/environments/environment.ts is produced by deployment/setenv.sh → deployment/set-env.ts from shell variables in setup_template-{dev,production}.cfg. It is committed, but as generated output.

Why. One template drives every environment; the build stamps in the git hash and build date; secrets and per-environment hosts stay out of application source.

Consequence. Hand-editing environment.ts works locally and then silently fails in CI. Adding a config key means touching four files. See environments.

D9 — EVA is embedded, not merged

Decision. Parts of the supplier workspace are rendered by a separate front end in iframes, integrated over a typed postMessage protocol.

Why. Independent deployability for the newer UI without rewriting Internal Admin.

Consequence. A hard seam with explicit two-way state synchronisation, origin checks, document.domain assignment, a shared-domain auth cookie, and iframe resizing driven by messages. The contract is MESSAGE_TYPE; changing it is a cross-repository change. See iframe messaging.

D10 — Spec before code

Decision. Non-trivial changes are specified in .specs/*.md before implementation, and reviewed as a spec before any code is written. Four agent phases: planner → plan reviewer → implementer → implementation reviewer.

Why. Stated in CLAUDE.md/AGENTS.md: small verifiable steps, a cheap generation→verification loop, and no "vibe" diffs in a codebase this large and this close to money movement.

Consequence. If reality contradicts the spec mid-implementation, stop and update the spec. See the spec-driven workflow.

D11 — Abstract service + mock sibling as the test seam

Decision. Services that specs need to stub get an abstract base class bound by token in AppModule, and/or a *-mock.service.ts sibling.

Why. Predates and substitutes for heavier DI/testing machinery; keeps specs from needing HttpTestingController everywhere.

Consequence. ~11 abstract bases and ~30 mock services. Mocks must never be reachable from production paths. See service abstractions.

Known debt

Recorded so that nobody rediscovers it as a surprise. None of these are invitations to a drive-by fix inside an unrelated task.

Area Debt Risk

Area	Debt	Risk
Unfinished migrations	Four table generations (`data-table`, `data-table-grid`, `data-table-ag-grid`, `pyb-ag-grid`); event rules v1 and v2; `supplier-underwriting` and `supplier-underwriting-beta`; three document backends plus an S3 provider; old and new search engines.	Medium — duplicated behaviour, unclear "which one" for newcomers.
Committed build residue	Stale `.js` / `.js.map` files sit beside their `.ts` sources throughout `src/app`.	Low — but they pollute every grep and diff.
Immutability violations	`supplier.reducers.ts` mutates nested models (`supplierDetailsModel.hide = !…`, `delete state.supplierModel.marketplaces`) inside otherwise-spread state.	Medium — blocks any move to `OnPush` and makes memoised selectors unreliable.
Getters lost on spread	Model getters (`isTarget`, `isAmazonSeller`) silently become `undefined` when the model is spread into a plain object.	Medium — fails quietly.
Global serialisation state	cerialize’s `DeserializeKeysFrom/`SerializeKeysTo` are module-global and set per call site.	Medium — ordering-sensitive.
Hardcoded credentials and keys	`MonitorService` embeds `Authorization: Token <literal>` and instance id `8003`; `main.ts` embeds an expired AG Grid licence key; `googleAuthClientId` is the same string in dev and prod templates.	Medium.
Devtools in production	`StoreDevtoolsModule.instrument()` is unconditional, and `FetchedRequestsInterceptor` records the last 50 requests (bodies included) into state.	Medium — exposes internal request shapes in a production build.
Sentry configured but unused	`sentryUrl` exists in every environment; no SDK is installed and no code reads it, yet `AppErrorHandler` tells users to check Sentry.	Low — misleading.
Route ordering hazard	In `/processes`, `:processKey/:paymentKey` and `:supKey` precede `approval`, which is why Balance Approval is linked through the misspelled `/proceses` table.	Medium — brittle; a reorder breaks live links.
Dead code paths	`SearchEngineService.search()` guarded by `if (1 === 1)`; `EnableStoreDebugging = false` requiring a source edit; `SupplierDetailsModel.isEFS` always `true`; an unreachable `return state` after a `return` in `supplier.reducers.ts`; `backgroundReports` effects file empty; `CachingInterceptor` implemented but never registered.	Low individually; collectively they mislead readers.
Unmaintained E2E suite	Cypress 7.x suite is explicitly marked not maintained in the repository README, and `test:ci` is commented out of `buildspec.yml`.	High for regression safety.
`SupplierService` breadth	One service covers details, tokens, reports, balances, advances, health and statements.	Medium — a change magnet.
`shared/` → `suppliers/` coupling	`EvaCommunicationService` and `LookupRepository` (in `shared/`) import supplier models, inverting the intended dependency direction.	Low — contained, but do not extend.
Legacy dependencies	`@angular/http`, `ngx-resource`, `@ultimate/ngxerrors`, jQuery + slimscroll, Bootstrap 4 beta, `core-js` 2, `rxjs-compat`, the unused webpack loader set.	Medium — each is an upgrade blocker.

Unfinished migrations

Four table generations (data-table, data-table-grid, data-table-ag-grid, pyb-ag-grid); event rules v1 and v2; supplier-underwriting and supplier-underwriting-beta; three document backends plus an S3 provider; old and new search engines.

Medium — duplicated behaviour, unclear "which one" for newcomers.

Committed build residue

Stale .js / .js.map files sit beside their .ts sources throughout src/app.

Low — but they pollute every grep and diff.

Immutability violations

supplier.reducers.ts mutates nested models (supplierDetailsModel.hide = !…, delete state.supplierModel.marketplaces) inside otherwise-spread state.

Medium — blocks any move to OnPush and makes memoised selectors unreliable.

Getters lost on spread

Model getters (isTarget, isAmazonSeller) silently become undefined when the model is spread into a plain object.

Medium — fails quietly.

Global serialisation state

cerialize’s `DeserializeKeysFrom/SerializeKeysTo are module-global and set per call site.

Medium — ordering-sensitive.

Hardcoded credentials and keys

MonitorService embeds Authorization: Token <literal> and instance id 8003; main.ts embeds an expired AG Grid licence key; googleAuthClientId is the same string in dev and prod templates.

Medium.

Devtools in production

StoreDevtoolsModule.instrument() is unconditional, and FetchedRequestsInterceptor records the last 50 requests (bodies included) into state.

Medium — exposes internal request shapes in a production build.

Sentry configured but unused

sentryUrl exists in every environment; no SDK is installed and no code reads it, yet AppErrorHandler tells users to check Sentry.

Low — misleading.

Route ordering hazard

In /processes, :processKey/:paymentKey and :supKey precede approval, which is why Balance Approval is linked through the misspelled /proceses table.

Medium — brittle; a reorder breaks live links.

Dead code paths

SearchEngineService.search() guarded by if (1 === 1); EnableStoreDebugging = false requiring a source edit; SupplierDetailsModel.isEFS always true; an unreachable return state after a return in supplier.reducers.ts; backgroundReports effects file empty; CachingInterceptor implemented but never registered.

Low individually; collectively they mislead readers.

Unmaintained E2E suite

Cypress 7.x suite is explicitly marked not maintained in the repository README, and test:ci is commented out of buildspec.yml.

High for regression safety.

SupplierService breadth

One service covers details, tokens, reports, balances, advances, health and statements.

Medium — a change magnet.

shared/ → suppliers/ coupling

EvaCommunicationService and LookupRepository (in shared/) import supplier models, inverting the intended dependency direction.

Low — contained, but do not extend.

Legacy dependencies

@angular/http, ngx-resource, @ultimate/ngxerrors, jQuery + slimscroll, Bootstrap 4 beta, core-js 2, rxjs-compat, the unused webpack loader set.

Medium — each is an upgrade blocker.

When you think a decision should change

Write it up in .specs/ as its own change, with the blast radius enumerated.
Get the plan reviewed before any code — that is what the plan-reviewer phase is for.
Prefer strangling over rewriting: introduce the new pattern beside the old, migrate call sites incrementally, and finish the migration. The debt table above is mostly a record of migrations that were started and abandoned.