TorbatYar/frontend/docs/refactor-risk-analysis.md

# TorbatYar Frontend — Refactor Risk Analysis

**Date:** 2026-07-26  
**Scope:** Risk assessment for proposed architectural improvements  
**Audience:** Engineering leads planning frontend refactors

---

## 1. Risk Matrix Overview

| Refactor area | Impact | Effort | Risk | Priority |
|---------------|--------|--------|------|----------|
| Fix Button type error | Low | S | 🟢 Low | P0 — immediate |
| Delete dead files | Low | S | 🟢 Low | P0 |
| Remove unused npm deps | Low | S | 🟢 Low | P0 |
| ESLint boundary rules | Medium | M | 🟢 Low | P1 |
| Unify ui/ → ds/ | Medium | M | 🟡 Medium | P1 |
| Merge beauty owner routes | Medium | M | 🟡 Medium | P1 |
| Extract portal infrastructure | High | L | 🟡 Medium | P2 |
| Split API client monoliths | High | L | 🟡 Medium | P2 |
| Extract accounting route logic | High | XL | 🔴 High | P3 |
| Add middleware.ts | Medium | M | 🟡 Medium | P3 |
| Split accounting screen aggregators | High | L | 🔴 High | P3 |
| Adopt TanStack Table | Medium | M | 🟡 Medium | P4 — defer |
| Implement Recharts analytics | Medium | L | 🟢 Low | P4 — defer |
| Extract module to separate app | Very High | XL | 🔴 Very High | Defer |

**Legend:** S = days, M = 1–2 weeks, L = 2–4 weeks, XL = 1–2 months

---

## 2. P0 — Stabilization Risks

### 2.1 Fix DS Button `loading` prop

| Factor | Assessment |
|--------|-------------|
| **Change** | Add `loading?: boolean` to `ds/Button.tsx` OR replace `loading` with `disabled` + spinner in accounting |
| **Files affected** | 1–5 (grep for `loading=` on Button) |
| **Regression risk** | 🟢 Low — localized UI change |
| **Test coverage** | None — manual verify submit buttons |
| **Rollback** | Trivial |

**Recommendation:** Add `loading` prop to DS Button (matches ui/Button API). Lower risk than changing all accounting call sites.

### 2.2 Delete dead files

| File | Risk if deleted |
|------|-----------------|
| `lib/api-client.ts` | 🟢 None — zero imports |
| `lib/optional-sso.ts` | 🟢 None — zero imports |
| `lib/beauty-business-nav.ts` | 🟢 None — superseded |
| `components/HealthStatus.tsx` | 🟢 None — zero imports |
| `components/beauty/pages/mobile.tsx` | 🟢 None — zero imports |

**Caution:** Run full grep before delete — dynamic imports would not appear in static analysis.

### 2.3 Remove unused npm packages

| Package | Risk |
|---------|------|
| `@tanstack/react-table` | 🟢 Safe if zero imports confirmed |
| `recharts` | 🟡 Check if planned for current sprint |

---

## 3. P1 — Boundary & UI Unification Risks

### 3.1 ESLint cross-domain import rules

| Factor | Assessment |
|--------|-------------|
| **Regression risk** | 🟢 Low — additive enforcement |
| **False positives** | Platform admin importing admin components — needs override config |
| **Migration** | Zero code changes if currently compliant |

### 3.2 Unify `components/ui/` → `components/ds/`

| Factor | Assessment |
|--------|-------------|
| **Files to migrate** | ~10 |
| **Risk** | 🟡 Medium — visual regression on marketing pages |
| **Button API diff** | ui uses `variant`; ds uses CVA variants — prop mapping needed |
| **Mitigation** | Migrate one page at a time; visual QA on login/dashboard/home |

### 3.3 Merge beauty `(owner)/` and `owner/` routes

| Factor | Assessment |
|--------|-------------|
| **Current state** | Two URL trees for same portal |
| **Risk** | 🟡 Medium — bookmark/URL breakage |
| **Affected URLs** | `/beauty/owner/analytics` vs `/beauty/organizations` (different trees) |
| **Mitigation** | Add redirects from old paths; merge into single `(owner)/` group |
| **SEO impact** | Low — authenticated app routes |

**Note:** These are NOT duplicate URLs — they serve different pages. The issue is organizational (two folders for one portal), not URL collision. Risk is developer confusion, not user-facing breakage.

---

## 4. P2 — Infrastructure Extraction Risks

### 4.1 Shared portal infrastructure

| Factor | Assessment |
|--------|-------------|
| **Scope** | Merge 2× createPortalLayout, 2× portal shell, 2× public layout, 2× shared.tsx |
| **Regression risk** | 🟡 Medium — touches all beauty AND healthcare portals (~200 routes) |
| **Testing burden** | High — 7 beauty portals + 8 healthcare portals |
| **Mitigation** | Extract base first without changing behavior; swap one portal at a time |
| **Rollback** | Keep old files until all portals migrated |

**Highest-risk portal migrations:**
1. Healthcare patient portal (uses `useHealthcarePatientContext`)
2. Healthcare doctor portal (uses `useHealthcareDoctorContext`)
3. Beauty owner portal (split across two route trees)

### 4.2 API client decomposition

| Factor | Assessment |
|--------|-------------|
| **Scope** | 3 files, ~4,600 lines, ~470 methods, 72 import sites |
| **Regression risk** | 🟡 Medium — import path changes only if barrel maintained |
| **Strategy risk** | 🟢 Low if deprecated barrels re-export everything |
| **Mitigation** | One subdomain per PR; keep `accountingApi.method()` surface stable |

**Danger:** Partial migration leaving two import paths for same method.

---

## 5. P3 — Accounting Refactor Risks

### 5.1 Extract route logic to components

| Factor | Assessment |
|--------|-------------|
| **Scope** | ~130 route files, many 200–660 lines |
| **Regression risk** | 🔴 High — accounting is largest module with complex forms |
| **Business criticality** | 🔴 Very high — financial data |
| **Test coverage** | None — no frontend test suite detected |
| **Mitigation** | Extract one page at a time; start with read-only pages (reports, monitoring) |

**Safest extraction order:**
1. Monitoring pages (read-only, ~22 lines each)
2. Compliance pages (delegate to DomainScreens already)
3. Integration pages (delegate to DomainScreens already)
4. Settings pages (moderate complexity)
5. Core CRUD (chart-of-accounts, vouchers, treasury) — last

**Most dangerous extractions:**
1. `vouchers/[id]/page.tsx` (550 lines, useFieldArray, posting logic)
2. `chart-of-accounts/page.tsx` (644 lines, tree manipulation)
3. `purchase/returns/page.tsx` (417 lines, current build failure location)

### 5.2 Split screen aggregators

| File | Lines | Split risk |
|------|------:|------------|
| `BusinessDocsPage.tsx` | 1,328 | 🔴 High — many document types interleaved |
| `WorkflowScreens.tsx` | 1,132 | 🔴 High — shared state between workflows |
| `DomainScreens.tsx` | 652 | 🟡 Medium — already parameterized |
| `OperationalDocumentPage.tsx` | 996 | 🔴 High — voucher editing core |

**Recommendation:** Split DomainScreens first (lowest coupling); defer BusinessDocsPage until accounting tests exist.

### 5.3 Add middleware.ts

| Factor | Assessment |
|--------|-------------|
| **Risk** | 🟡 Medium — can break auth flows if misconfigured |
| **Complexity** | Public routes (`/beauty/site/*`, `/healthcare/site/*`, `/login`) need allowlist |
| **Duplication** | Currently AuthGuard in layouts — middleware replaces client-side redirect with edge redirect |
| **Mitigation** | Start with logging-only middleware; enable redirects after validation |

---

## 6. P4 — Deferred Changes

### 6.1 Adopt TanStack Table

| Factor | Assessment |
|--------|-------------|
| **Need** | Current DataTable lacks sorting, filtering, pagination |
| **Risk** | 🟡 Medium — visual/behavior change across 50+ tables |
| **Recommendation** | Defer until explicit product requirement for sortable columns |

### 6.2 Implement Recharts

| Factor | Assessment |
|--------|-------------|
| **Need** | Analytics routes exist without charts |
| **Risk** | 🟢 Low — additive feature |
| **Recommendation** | Implement per-page when analytics data available |

### 6.3 Module extraction to separate Next.js app

| Factor | Assessment |
|--------|-------------|
| **Risk** | 🔴 Very High |
| **Reasons to defer** | Shared auth, tenant, theme, DS; build already monolithic; team size |
| **Trigger to revisit** | Build > 3 min, independent release cycles needed, 5+ modules |

---

## 7. Risk by Module

| Module | Refactor sensitivity | Reason |
|--------|---------------------|--------|
| **Accounting** | 🔴 Very High | Financial data, complex forms, no tests, inline route logic |
| **Healthcare patient/doctor** | 🟡 Medium | Context hooks, PHI display |
| **Beauty owner CRUD** | 🟡 Medium | Active development, form mutations |
| **Beauty/healthcare portals (display)** | 🟢 Low | Thin routes, TanStack Query |
| **Platform admin** | 🟡 Medium | Tenant management |
| **Public sites** | 🟢 Low | Mostly static display |

---

## 8. Regression Testing Gaps

| Area | Current state | Risk implication |
|------|---------------|------------------|
| Unit tests | None detected in frontend | All refactors are manual QA |
| Integration tests | None | API client splits untested |
| E2E tests | None | Portal extraction untested |
| Visual regression | None | UI unification untested |
| Type checking | Failing | Cannot catch prop mismatches in CI |

**Recommendation:** Before P3 accounting refactors, establish at minimum:
1. Green `npm run build`
2. Smoke test checklist per module (manual)
3. Consider Playwright for top 10 critical flows

---

## 9. Change Blast Radius

### Single PR safe changes (< 10 files, low risk)

- Button loading prop fix
- Dead file deletion
- Unused npm removal
- ESLint config addition
- Single accounting page extraction (read-only)

### Multi-PR required (medium risk)

- Portal infrastructure extraction
- API client splitting
- ui/ → ds/ migration
- Middleware addition

### Program-level effort (high risk — dedicated sprint)

- Full accounting route extraction
- Screen aggregator splitting
- Beauty owner route consolidation

---

## 10. Decision Framework

Before any refactor, answer:

```
1. Does it fix a build blocker or security issue?
   → YES: Do immediately (P0)

2. Does it reduce duplication without changing behavior?
   → YES: Extract with snapshot tests / manual QA (P2)

3. Does it move business logic between layers?
   → YES: One file at a time, start with read-only (P3)

4. Does it change user-visible URLs or auth flows?
   → YES: Add redirects, test all portals (P1–P3)

5. Does it introduce new abstractions?
   → YES: Wait until 3rd module needs it (Rule of Three)
```

---

## 11. Recommended Execution Order (Risk-Adjusted)

```
Week 1:  P0 fixes (build green, dead code removed)
Week 2:  P1 ESLint boundaries + Button unification
Week 3:  P1 beauty owner organization (no URL changes)
Week 4–6: P2 portal base extraction (beauty admin first — simplest)
Week 7–8: P2 portal extraction (remaining portals, one per PR)
Week 9–12: P2 API client split (accounting vouchers first)
Week 13+: P3 accounting extraction (monitoring → settings → core CRUD)
Ongoing: P3 middleware + error boundaries
Defer: P4 table/charts, module extraction
```

---

## 12. Summary

| Category | Count |
|----------|------:|
| 🔴 High/Very High risk refactors | 4 |
| 🟡 Medium risk refactors | 7 |
| 🟢 Low risk refactors | 5 |
| Current build blockers | 1 |
| Dead files safe to delete | 5 |
| Modules with zero test coverage | All |

The codebase is **safe to maintain and extend** within existing patterns. **Large-scale refactoring** (especially accounting) carries significant regression risk without automated tests. Prioritize stabilization (P0) and boundary enforcement (P1) before structural changes (P2–P3).

---

## Related Documents

- [frontend-migration-plan.md](./frontend-migration-plan.md)
- [frontend-architecture-audit.md](./frontend-architecture-audit.md)
- [module-boundaries.md](./module-boundaries.md)
- [dependency-report.md](./dependency-report.md)