Files
ui/_docs/02_document/architecture_compliance_baseline.md
T
Oleksandr Bezdieniezhnykh c368f60853 [AZ-511] classColors carve-out to src/class-colors/ (closes F3)
Move src/features/annotations/classColors.ts to its own component directory
src/class-colors/ with a proper barrel; update the 4 consumer imports to go
through the barrel; remove the F3-pending exemption from STC-ARCH-01 and from
the architecture test fixture; clean up the 5 coupled doc/script touchpoints.
Closes baseline finding F3 and retires the 5-coupled-places carry-over surface
logged in LESSONS.md 2026-05-12.

- Add `class-colors` to scripts/check-arch-imports.mjs COMPONENT_DIRS so deep
  imports past the new barrel are caught symmetric to every other component.
- Replace the architecture test "exemption WORKS" fixture with the stronger
  "deep import into class-colors NOW FAILS" assertion (Risk 4 mitigation).
- module-layout.md: Layout Rules + Per-Component Mapping (11_class-colors,
  06_annotations, 03_shared-ui) + Verification Needed #1 + shared/class-colors
  block all updated to reflect the new home.
- 11_class-colors/description.md: Caveats §7 + Module Inventory updated.
- architecture_compliance_baseline.md: F3 marked CLOSED with full pre-resolution
  context preserved (mirrors AZ-485/F4 + AZ-486/F7 pattern); F4 carry-forward
  exemption note retired.
- 04_verification_log.md: open questions #1 + #8 marked RESOLVED.
- Build passes with no circular-import warnings (AC-4); fast suite 231/13
  skipped green (AC-5); static profile green (AC-3 — zero exemptions remain).

Batch report: _docs/03_implementation/batch_14_cycle3_report.md

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-13 03:08:36 +03:00

177 lines
23 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.
# Architecture Compliance Baseline
**Scope**: full existing codebase (`src/` deployed tree + `mission-planner/` port-source).
**Date**: 2026-05-10
**Mode**: code-review baseline (Phase 1 + Phase 7 only).
**Verdict**: **FAIL** — 1 Critical, 4 High, 2 Medium, 2 Low. Verdict drives Step 2 → Step 4 / Step 8 routing per `flows/existing-code.md`; it does NOT block any pipeline.
Inputs read in Phase 1:
- `_docs/02_document/architecture.md` (incl. Architecture Vision P1P12, ADR-001..010, Mission-planner convergence plan).
- `_docs/02_document/module-layout.md` (per-component file ownership, Layer table, Verification Needed #1#8).
- `_docs/00_problem/restrictions.md`, `_docs/01_solution/solution.md` (project context).
Detection approach: TypeScript `import ... from '...'` parsing across all `.ts` / `.tsx` files; layer resolved via `module-layout.md` "Per-Component Mapping"; layer comparison against the "Allowed Dependencies (layering)" table.
---
## Findings
| # | Severity | Category | Location | Title |
|----|----------|-------------|--------------------------------------------------------------------------|---------------------------------------------------------------------|
| F1 | Critical | Architecture | `mission-planner/**` vs `src/features/flights/**` | Mission-planner duplicates 13+ modules of the deployed flights tree |
| F2 | High | Architecture | `src/features/dataset/DatasetPage.tsx:9``../annotations/CanvasEditor` | Cross-feature same-layer edge — `07_dataset` reaches into `06_annotations` |
| F3 | High | Architecture | `src/features/annotations/classColors.ts` | Physical / logical owner split — `11_class-colors` file lives inside `06_annotations` |
| F4 | High | Architecture | every component | No Public API barrels — every internal file is de-facto public — **CLOSED 2026-05-11 by AZ-485 (`23746ec`)** |
| F5 | High | Architecture | `mission-planner/src/flightPlanning/MapView.tsx ↔ MiniMap.tsx` | Pre-existing import cycle inside port-source |
| F6 | Medium | Architecture | (codebase-wide) | No `src/shared/` infrastructure for cross-cutting concerns |
| F7 | Medium | Architecture | every `api.*` / `createSSE` call site | Hardcoded `/api/<service>/...` paths instead of env-driven endpoints — **CLOSED 2026-05-11 by AZ-486 (`8a461a2`)** |
| F8 | Low | Architecture | `_docs/02_document/module-layout.md` | Layering-table inconsistency — Header → useAuth is unannotated |
| F9 | Low | Architecture | `mission-planner/src/{main,App,setupTests,vite-env}.tsx` | Inert second Vite entry tree at port-source root |
---
## Finding Details
### F1: Mission-planner duplicates 13+ modules of the deployed flights tree (Critical / Architecture)
- **Location**: `mission-planner/src/**` vs `src/features/flights/**`.
- **Description**: Component `05_flights` has two physical trees (per ADR-009). The port-source duplicates almost every module the deployed tree exposes today AND carries behaviors the deployed tree does not yet have (camera-config side panel, mission JSON I/O, satellite tile provider, richer waypoint-altitude UX). Duplication enumerated below; until convergence completes, the same logic lives in two places and silently drifts. This is the **work-list** the Architecture Vision (Mission-planner convergence plan) asked Step 2 to surface.
**Duplicated by name** (deployed → port-source):
| Deployed (`src/features/flights/`) | Port-source (`mission-planner/src/.../`) | Notes |
|--------------------------------------------|----------------------------------------------------------------------------|-------|
| `AltitudeChart.tsx` | `flightPlanning/AltitudeChart.tsx` | Both consumed by their respective panels. |
| `AltitudeDialog.tsx` | `flightPlanning/AltitudeDialog.tsx` | Port-source version uses `useLanguage` (custom i18n); deployed uses Tailwind. |
| `DrawControl.tsx` | `flightPlanning/DrawControl.tsx` | Deployed uses `newGuid` from `flightPlanUtils`; port-source from `utils.ts`. |
| `JsonEditorDialog.tsx` | `flightPlanning/JsonEditorDialog.tsx` | Mission JSON I/O — deployed version is a stub vs. port-source's full editor. |
| `MapPoint.tsx` | `flightPlanning/MapPoint.tsx` | Both import the local `pointIcon*` set. |
| `MiniMap.tsx` | `flightPlanning/MiniMap.tsx` | Port-source version is half of the F5 cycle. |
| `WindEffect.tsx` | `flightPlanning/WindEffect.tsx` | Port-source pulls `useLanguage`; deployed uses `i18next`. |
| `WaypointList.tsx` | `flightPlanning/PointsList.tsx` (renamed) | DnD reorder; same intent, different libs. |
| `FlightMap.tsx` | `flightPlanning/MapView.tsx` (renamed) | The cycle partner. |
| `FlightParamsPanel.tsx` | `flightPlanning/LeftBoard.tsx` (renamed) | Side-panel composition root. |
| `FlightsPage.tsx` | `flightPlanning/flightPlan.tsx` (renamed) | Route component. |
| `mapIcons.ts` | `icons/PointIcons.tsx`, `icons/MapIcons.tsx`, `icons/SidebarIcons.tsx` | Port-source uses inline SVG components; deployed uses `L.icon`. |
| `flightPlanUtils.ts` (`newGuid`, `calculateDistance`, `calculateAllPoints`, `parseCoordinates`, `getMockAircraftParams`) | `utils.ts` (`newGuid`); `services/calculateDistance.ts`; `services/calculateBatteryUsage.ts`; `services/AircraftService.ts` (`mockGetAirplaneParams`) | Same five symbols, scattered across four files in the port-source. |
| `types.ts` (`FlightPoint`, `CalculatedPointInfo`, `MapRectangle`, `ActionMode`, `WindParams`, `AircraftParams`, `PURPOSES`, `COORDINATE_PRECISION`, `TILE_URLS`) | `types/index.ts` + `constants/{actionModes,purposes,tileUrls,maptypes,languages,translations}.ts` | Port-source has richer constants tables (mapTypes, languages, translations) the deployed tree lacks. |
**Port-source-only** (no deployed counterpart yet — these are the behaviors that must port across Phase B):
- `flightPlanning/LanguageContext.tsx` + `LanguageSwitcher.tsx` — mission-planner's standalone i18n (deployed uses `i18next`). DO NOT port; consolidate on `i18next`.
- `flightPlanning/TotalDistance.tsx` — distance + battery readout in the left panel. Behavior the deployed tree does not yet show in the same form.
- `icons/PhoneIcon.tsx` (rotate-phone hint), `icons/SidebarIcons.tsx` (DashedArea / HideSidebar / ShowSidebar).
- `services/WeatherService.ts` — wind compute (already partially in deployed `flightPlanUtils.ts:60` with the hardcoded key — P10 violation).
- `flightPlanning/Aircraft.ts` — aircraft entity helpers.
- `config.ts``COORDINATE_PRECISION`, `GOOGLE_GEOCODE_KEY` (second hardcoded-key risk — verify).
- **Suggestion**: This is the convergence work-list. Per Architecture Vision (Mission-planner convergence plan):
- **NOT a Step 4 testability item** — these are behavior ports, not testability surgery.
- **NOT a Step 8 refactor item alone** — too large for one mechanical refactor; behavior changes break tests.
- **Primary home**: Phase B feature cycles (one cycle per port group: i18n consolidation; flight-planning UI parity; waypoint-altitude UX; mission JSON I/O; satellite tile provider; weather/battery service; camera-config side panel). Each cycle: New Task → Implement → Run Tests → Test-Spec Sync → Update Docs → Deploy → Retrospective.
- **Final Phase B cycle**: delete `mission-planner/` (its only consumer becomes zero).
- **Task / Epic**: feeds `05_flights` Step 3 (Test Spec) — every port target must have an AC for the converged behavior before its Phase B cycle starts.
### F2: Cross-feature same-layer edge — `07_dataset` reaches into `06_annotations` (High / Architecture)
- **Location**: `src/features/dataset/DatasetPage.tsx:9``import CanvasEditor from '../annotations/CanvasEditor'`.
- **Description**: `07_dataset` and `06_annotations` are both Layer 3 features. Same-layer imports are forbidden except for explicit grandfathered edges. This edge IS grandfathered (`module-layout.md` Allowed-Dependencies table footnote †, Verification Needed #2). It is the only Layer-3 ↔ Layer-3 import in the codebase. Until a proper home is chosen, the implement skill must keep treating `CanvasEditor.tsx` as READ-ONLY for `07_dataset` tasks.
- **Suggestion**: Lift `CanvasEditor.tsx` to `src/components/canvas/CanvasEditor.tsx` (under `03_shared-ui`) OR to a new `06b_canvas` component. Both options drop the same-layer edge. Decision should ride a Phase B cycle that already touches `CanvasEditor` — folding the move into a behavior change is cheaper than a standalone refactor.
- **Task / Epic**: defer to Phase B (when a `CanvasEditor`-touching feature lands) or Step 8 refactor (optional).
### F3: Physical / logical owner split for `classColors.ts` (High / Architecture) — **CLOSED 2026-05-13 by AZ-511 (cycle 3 batch 14)**
- **Resolution**: File moved from `src/features/annotations/classColors.ts` to its own component directory `src/class-colors/classColors.ts` with a proper barrel `src/class-colors/index.ts` re-exporting `getClassColor`, `getClassNameFallback`, `getPhotoModeSuffix`, `FALLBACK_CLASS_NAMES`. All 4 consumer imports updated to use the barrel (`'../class-colors'` / `'../../class-colors'`). The STC-ARCH-01 `EXEMPT_RE` for `features/annotations/classColors` was removed from `scripts/check-arch-imports.mjs`; `class-colors` was added to `COMPONENT_DIRS` so future deep imports into the new component are caught. The architecture test fixture in `tests/architecture_imports.test.ts` was reshaped from "exemption WORKS" to "synthetic deep import into class-colors NOW FAILS" (Risk 4 mitigation). The 5-coupled-places carry-over surface logged in `_docs/LESSONS.md` 2026-05-12 is fully retired. Module-layout Per-Component Mapping for `11_class-colors` and `06_annotations` updated; Verification Needed #1 marked RESOLVED. Build passes with no circular-import warnings (AC-4); fast suite 231 / 13 skipped green (AC-5).
- **Pre-resolution context (preserved for trace)**:
- **Location**: `src/features/annotations/classColors.ts`.
- **Description**: The file was under `06_annotations`'s owns-glob (`src/features/annotations/**`) but the component spec assigned it to `11_class-colors` (Layer 0 shared kernel) — four external consumers depended on it (`03_shared-ui/DetectionClasses`, `06_annotations/{CanvasEditor,AnnotationsPage,AnnotationsSidebar}`, future `07_dataset` class-distribution chart). Module-layout Verification #1 recorded the workaround: `READ-ONLY` for `06_annotations` tasks. The workaround scaled poorly — a new `06_annotations` contributor reading only the directory glob would not know the file is off-limits.
- **Suggestion (executed)**: Move physical file to its own component directory `src/class-colors/` and add a barrel.
- **Task / Epic**: AZ-511 (Epic AZ-509) — cycle 3 batch 14, 3 points.
### F4: No Public API barrels — every internal file is de-facto public (High / Architecture) — **CLOSED 2026-05-11 by AZ-485 (commit `23746ec`)**
- **Resolution**: 11 component barrels (`src/<component>/index.ts`) added — one per component except `10_app-shell` (top-level file collection, never imported as a unit). Every cross-component import in `src/`, `tests/`, and `e2e/` now goes through the barrel. The `STC-ARCH-01` static gate (`scripts/check-arch-imports.mjs --mode=arch-imports`, wired into `scripts/run-tests.sh --static`) fails the build on any deep-import regression. The architecture test `tests/architecture_imports.test.ts` exercises the gate with synthetic fixtures (AC-4 fail-on-synthetic, AC-5 pass-on-migrated). Module-layout Layout Rule #3 records the convention.
- **Carried-forward exemption**: ~~`src/features/annotations/classColors`~~**CLOSED by AZ-511 (cycle 3 batch 14)**. The file moved to `src/class-colors/` with its own barrel; the `EXEMPT_RE` was removed from `scripts/check-arch-imports.mjs`. STC-ARCH-01 has zero exemptions today.
- **Pre-resolution context (preserved for trace)**:
- **Location**: every component root (no `src/<component>/index.ts` existed before AZ-485; only `src/types/index.ts` and `mission-planner/src/types/index.ts` were barrels and those are re-export hubs, not component facades).
- **Description**: Cross-component imports used file-name granularity (`import { api } from '../api/client'`, `import { useFlight } from '../../components/FlightContext'`, etc.). Consequence: there was **no enforceable Public API surface**. Any internal refactor inside a component (split a file, rename an export) was a breaking change to every importer. Phase 7 Check #2 ("Public API respect") could not meaningfully fail in this codebase because everything was public.
- **Suggestion (executed)**: add `src/<component>/index.ts` for every component, re-exporting only the symbols listed in module-layout's "Public API" line.
- **Task / Epic**: Step 4 testability — moved to Phase B cycle 1 batch 9 / AZ-485 / Epic AZ-447.
### F5: Pre-existing import cycle inside port-source (High / Architecture)
- **Location**: `mission-planner/src/flightPlanning/MapView.tsx` (imports `MiniMap` on line 16) ↔ `mission-planner/src/flightPlanning/MiniMap.tsx` (imports `UpdateMapCenter` from `./MapView` on line 2).
- **Description**: A named-handle cycle internal to `mission-planner/`. Module-layout Verification #5 + `00_discovery.md §7` already record it. Not introduced by this scan. The cycle does NOT cross component boundaries (both files belong to `05_flights` port-source). The cycle disappears at the end of the convergence (when `mission-planner/` is deleted).
- **Suggestion**: No action. Track-only finding. Reason: fixing the cycle inside port-source means moving `UpdateMapCenter` to a third file — wasted work given the eventual delete. If the deployed tree gains the same cycle when porting, fix it there.
- **Task / Epic**: (none — closed by mission-planner deletion in the final Phase B cycle).
### F6: No `src/shared/` infrastructure for cross-cutting concerns (Medium / Architecture)
- **Location**: codebase-wide. No `src/shared/` directory exists today (module-layout Layout Rules #4).
- **Description**: There is no shared logger, no central error envelope, no config loader, no telemetry hook. Each feature page does ad-hoc `console.error` + silent catches (multiple module findings — annotations sidebar AI-detect silent catches, dataset silent catches, settings save without `try/finally`). Onboarding observability or a global error boundary today touches every feature.
- **Suggestion**: Introduce `src/shared/` (or `src/components/util/`) at Phase B kickoff with:
- `shared/logger.ts` — wraps `console.*`, adds revision + user context; replace ad-hoc `console.error`.
- `shared/config.ts` — typed `import.meta.env.*` accessor (resolves P10 OpenWeatherMap key + the future env-base-URL refactor).
- `shared/errorBoundary.tsx` — application-level boundary (today the SPA has no `ErrorBoundary` — recorded in `src__App-and-main.md` findings).
- `shared/endpoints.ts` — typed endpoint constants (closes F7).
- **Task / Epic**: Phase B candidate (one cycle for shared infrastructure) OR fold into Step 8 refactor if user picks A on the Step 8 gate.
### F7: Hardcoded `/api/<service>/...` paths instead of env-driven endpoints (Medium / Architecture) — **CLOSED 2026-05-11 by AZ-486 (commit `8a461a2`)**
- **Resolution**: `src/api/endpoints.ts` introduced as the single source of truth — 25 typed builders covering every `/api/<service>/<path>` URL the UI talks to today. Re-exported through the F4 barrel `src/api/index.ts`; consumers import `{ endpoints } from '../api'` (or `../../api`). Every production callsite of `api.*` and `createSSE()` migrated to `endpoints.*` — 13 source files (admin, annotations × 5, flights, settings, dataset, auth, client, FlightContext, DetectionClasses). The `STC-ARCH-02` static gate (`scripts/check-arch-imports.mjs --mode=api-literals`, wired into `scripts/run-tests.sh --static`) fails the build on any new `/api/<service>/` literal in `src/` outside the contract owner (`endpoints.ts`) and `*.test.tsx?` files. The colocated `src/api/endpoints.test.ts` (36 assertions, character-identical to pre-refactor URL strings) serves as the wire-contract documentation per `module-layout.md`'s "code-derived documentation" pattern. Module-layout Verification Needed item #3a records the convention.
- **F6 interaction**: `endpoints.ts` lives under `01_api-transport` (not `src/shared/`) — F6 is explicitly deferred. When/if F6 lands and moves the file, only `src/api/index.ts` flips the re-export source; consumers do not change. This is exactly the protection F4 was built to provide.
- **Pre-resolution context (preserved for trace)**:
- **Location**: every call site of `api.*()` and `createSSE()` across `src/features/**` and `src/auth/`, `src/components/FlightContext.tsx`, `src/components/DetectionClasses.tsx`. Approximately 30 call sites.
- **Description**: Consequence of ADR-006 (nginx prefix-strip). Each call site repeated `/api/<service>/<path>` as a string literal. Testability suffered — every test fixture had to duplicate paths; any nginx-route change touched every feature. Architecture intent (ADR-006 Consequences) explicitly flagged this: *"The SPA hardcodes /api/<service>/... paths in source instead of an env-driven base URL — testability is poor (finding tracked)."*
- **Suggestion (executed)**: introduce a typed endpoints module exposing builders like `endpoints.auth.login()`, `endpoints.flights.byId(id)`, `endpoints.annotations.media(query)`, etc.
- **Task / Epic**: Step 4 testability — moved to Phase B cycle 1 batch 10 / AZ-486 / Epic AZ-447.
### F8: Layering-table inconsistency — Header → useAuth is unannotated (Low / Architecture)
- **Location**: `_docs/02_document/module-layout.md` — "Allowed Dependencies (layering)" table vs "Per-Component Mapping" `03_shared-ui` row.
- **Description**: The layering table says Layer 2 may import only from layers 0+1, no same-layer edges. The per-component table explicitly lists `03_shared-ui Imports from: ..., 02_auth`. The actual import `src/components/Header.tsx:3 → ../auth/AuthContext` is by design — every header needs the current user. This is not a code violation (the per-component table is authoritative for specific edges); it's a doc bug.
- **Suggestion**: Add a layering-table footnote for `03_shared-ui` similar to the existing `07_dataset` footnote: *"03_shared-ui imports `useAuth` from `02_auth`'s `AuthContext` — same-layer cross-component edge, permitted as it is the only cross-cutting state-context dependency."* Update during Step 4 doc-touchup or fold into Step 13 docs cycle.
- **Task / Epic**: documentation only — no code change.
### F9: Inert second Vite entry tree at port-source root (Low / Architecture)
- **Location**: `mission-planner/src/{main.tsx,App.tsx,setupTests.ts,vite-env.d.ts}`.
- **Description**: `mission-planner/src/` carries its own Vite entrypoint (`main.tsx` + `App.tsx`), test setup file, and env shim. ADR-009 says the deployed Vite build does NOT compile this tree, but there is nothing in the codebase that **prevents** a future contributor from adding `mission-planner/` to a Vite `build.rollupOptions.input` and shipping two SPAs. Architecture intent is "vendored port-source, NOT a deployed component".
- **Suggestion**: Step 4 testability or Step 8 refactor — verify `vite.config.ts` has no `mission-planner/` entry; if Vite ever adds workspace-aware builds, add explicit exclusion. Defer until the convergence retires the directory entirely.
- **Task / Epic**: closed by F1's final delete cycle.
---
## Routing — what feeds where
Per `flows/existing-code.md` Step 2 → Step 4 / Step 8 rule, High and Critical Architecture findings must either (a) be appended to the testability `list-of-changes.md` input for Step 4, or (b) be deferred to Step 8 (optional Refactor) via Choose. Recommended routing:
| Finding | Recommended target step | Reason |
|---------|----------------------------------------------------|------------------------------------------------------------------------|
| F1 | **Phase B cycles** (NOT Step 4, NOT Step 8) | Behavior ports — need test safety net + per-feature deploy. |
| F2 | **Phase B** (when a `CanvasEditor`-touching task lands) | Fold the lift into a behavior change cycle. |
| F3 | **Step 4 testability** | Pure file move + import-path update — fits the "smallest fix" rule. |
| F4 | **Step 4 testability** | 11 new `index.ts` files + cohort of import-path edits — mechanical. |
| F5 | **No action** (closed by F1 final delete) | Fixing inside port-source is wasted work. |
| F6 | **Phase B** (one infra cycle) OR **Step 8 refactor** | Shared logger / config / endpoints / error boundary — design choice. |
| F7 | **Step 4 testability** | Endpoint extraction enables tests; depends on F6 if `src/shared/` is path. |
| F8 | **Documentation touch-up** (no step) | Doc-only. |
| F9 | **Defer** | Closed by F1 final delete. |
The 4 High and 1 Critical findings that should drive a Choose decision now:
- **F1** — Phase B work-list (not Step 4, not Step 8).
- **F2** — Phase B side-quest (not Step 4, not Step 8).
- **F3** — Step 4 candidate.
- **F4** — Step 4 candidate.
- **F5** — track-only (no decision needed).
---
## Baseline Delta
(N/A — this is the first baseline; no prior report to delta against. Future `code-review` invocations in `cumulative` or `full` mode will emit a Baseline Delta section against this file.)