Files
pedwfrontend/memory-bank/architect-review.md
T

95 lines
6.2 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
Absolutely — heres a senior architecture review of this repository.
## Update: TASK22211 completed slice (2026-03-23)
- Branch: `TASK22211-endpoint-search-document-contract-consistency`
- Scope completed:
- `pages/api/endpoint/getsearchdocumenthistory_api.js`
- `pages/api/endpoint/getsearchdocumenthistorypaged_api.js`
- `pages/api/endpoint/getsearchdocumentdetails_api.js`
- `pages/api/endpoint/getsearchdocumentdetailspaged_api.js`
- `pages/api/endpoint/getsearchdocumentTypes_api.js`
- `tests/phase21/endpoint-handler-contract.test.cjs`
- `memory-bank/change-log.md`
- Pattern applied:
- standardized structured error envelopes via `respondError`
- preserved existing success contract behavior via `respondSuccess`
- added explicit required-input guards for missing document/case/query inputs
- removed noisy direct `console.log` usage in touched handlers
- Validation snapshot:
- `node tests/phase21/api-contract-slice1.test.cjs` -> pass
- helper 4/4
- file-handler 11/11
- email-handler 12/12
- endpoint-handler 53/53
- `npm run lint` -> warnings only (pre-existing react-hooks warnings)
### Suggested next coherent slice
- Endpoint contract consistency follow-on for case/myportal retrieval cluster still using raw error passthrough patterns, applying the same bounded refactor + phase21 test expansion model.
## 1) Current architecture summary
- **Platform shape:** Next.js 14 (pages router) + React 18, with a **custom Node server** and also a legacy Express-style server under `server/server.js`.
- **API layer:** Large `pages/api/**` surface (endpoint/file/email/admin/auth), many routes acting as thin proxies to upstream systems (CRM/relay/Azure).
- **State/auth:** Redux (`next-redux-wrapper` + persistence) and `next-auth` + Prisma adapter for account/session persistence.
- **i18n/routing:** EN/CY via `next-translate`, `i18n.js`, and many Welsh rewrites in `next.config.js`.
- **Integration-heavy domains:** Azure storage/queues, GOV.UK Notify, mapping embeds/libs, PDF generation.
## 2) Strengths
- **Clear high-level domain separation** by folders (`pages`, `components`, `store`, `actions`, `prisma`, `locales`).
- **Strong security intent** (CSP + secure headers + auth/session infrastructure in place).
- **Bilingual-first routing model** is explicit and robustly represented in config.
- **Operational integration maturity**: telemetry, notifications, document handling, and mapping already embedded.
- **Recently added governance docs** (`.clinerules`, `GUARDRAILS.md`, `CONTRIBUTING_AI.md`, `context/`, `memory-bank/`) materially improve consistency and delivery safety.
## 3) Risks (scalability, maintainability, coupling, boundaries, operations, debt)
1. **God-module risk in `actions/index.js` (very high maintainability/coupling risk)**
Hundreds of mixed responsibilities (auth helpers, search, uploads, notifications, case operations) create high fan-in/fan-out and regression blast radius.
2. **Boundary leakage between UI and integration concerns**
Frontend-facing actions are tightly coupled to relay/API details, hashes, token flow assumptions, and endpoint naming conventions.
3. **Inconsistent runtime topology / entrypoint ambiguity**
Both root `server.js` and `server/server.js` exist with different runtime behavior, increasing deployment confusion and operational drift risk.
4. **Logging/privacy exposure risk**
Multiple debug logs in sensitive paths (auth/email/payloads) risk leaking identifiers or confidential request data.
5. **API proliferation with naming inconsistency and limited shared contract enforcement**
Many `_api.js` endpoints with repeated patterns suggest technical debt and likely drift in validation/error handling semantics.
6. **Scalability bottlenecks in synchronous proxy patterns**
High reliance on serial external calls and heavy payload operations (search/doc/PDF flows) without strong evidence of caching, backpressure, or circuit-breaking.
7. **Test coverage fragility**
Sparse automated tests means confidence depends heavily on manual checks, slowing safe change velocity and increasing incident probability.
8. **i18n rewrite complexity growth**
Large rewrite maps increase cognitive load and chance of EN/CY divergence as route count expands.
## 4) Prioritized recommendations
1. **Decompose `actions/index.js` into domain modules (highest priority)**
- Split into `actions/search`, `actions/case`, `actions/account`, `actions/docs`, `actions/notify`, etc.
- Add a stable facade export to avoid immediate call-site churn.
2. **Establish explicit service boundaries for API integrations**
- Create integration adapters in `lib/services/*` (CRM/relay/azure/notify).
- Keep API routes as orchestrators with shared validation/error wrappers.
3. **Resolve server runtime canonically**
- Choose one entrypoint for dev/test/prod, deprecate the other, simplify scripts and ops docs.
4. **Implement secure logging policy + redaction utility**
- Centralized logger; block raw payload/token/user-data logging by default.
5. **Standardize API contract/error envelope**
- Shared response/error shape and input validation helper used by all sensitive endpoints.
6. **Introduce incremental architecture tests**
- Start with high-risk auth/file/email negative-path API tests.
7. **Tame i18n rewrite sprawl**
- Introduce parity checks (lint/CI) for EN/CY route + locale key synchronization.
## 5) Low-risk next improvements (next 12 sprints)
- Add a **single redaction helper** and replace highest-risk `console.log` usage first (auth/email/file flows).
- Add **API wrapper utility** for consistent try/catch + error response formatting, then migrate 510 high-traffic routes.
- Create `actions/` module split with **barrel exports** (no behavioral change, structure-only).
- Add **CI guard checks**: `npm run lint` + locale key parity + forbidden logging patterns in sensitive files.
- Decide and document **canonical startup path** in `context/runbook.md` and package scripts.
- Add a **small smoke test suite** for auth sign-in callback, public search endpoint, and one upload negative-path scenario.
If you want, I can turn this into a concrete 30/60/90-day architecture hardening roadmap with owners, sequencing, and expected risk reduction per step.