azaion/ui
1
0
Fork 0
mirror of https://github.com/azaion/ui.git synced 2026-06-21 15:51:11 +00:00
Code Issues Packages Projects Releases Wiki Activity
Files
f2451944fd9c1427514bedc3bd88e63b83487f28
ui/_docs/02_document/architecture_compliance_baseline.md
T
Oleksandr Bezdieniezhnykh 510df68bcf [AZ-447] autodev Steps 1-4 baseline: docs, tests, refactor specs 
Captures the full output of autodev existing-code Phase A through
Step 4 (Code Testability Revision) for the Azaion UI workspace:

- Step 1 Document: _docs/02_document/ (FINAL_report, architecture,
  glossary, components/, modules/, diagrams/, system-flows,
  module-layout) plus _docs/00_problem/ + _docs/01_solution/ +
  _docs/legacy/ + _docs/how_to_test + README.
- Step 2 Architecture Baseline: architecture_compliance_baseline.md.
- Step 3 Test Spec: _docs/02_document/tests/ (environment,
  test-data, blackbox/performance/resilience/security/
  resource-limit tests, traceability-matrix), enum_spec_snapshot,
  expected_results/results_report.md (98 rows), plus the
  run-tests.sh + run-performance-tests.sh runners.
- Step 4 Code Testability Revision: 01-testability-refactoring/
  run dir (list-of-changes C01-C07, deferred_to_refactor,
  analysis/research_findings + refactoring_roadmap) and the 7
  child task specs AZ-448..AZ-454 under _docs/02_tasks/todo/
  plus _dependencies_table.md.
- _docs/_autodev_state.md pins the cursor at Step 4 / refactor
  Phase 4 entry so /autodev resumes cleanly.

Epic AZ-447 (UI testability gates) tracks the 7 child tasks that
will land in subsequent commits.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 00:38:49 +03:00

166 lines
20 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 P1–P12, 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 |
| 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 |
| 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)
- **Location**: `src/features/annotations/classColors.ts`.
- **Description**: The file is under `06_annotations`'s owns-glob (`src/features/annotations/**`) but the component spec assigns it to `11_class-colors` (Layer 0 shared kernel) — three external consumers depend on it (`03_shared-ui/DetectionClasses`, `06_annotations/{CanvasEditor,AnnotationsPage,AnnotationsSidebar}`, future `07_dataset` class-distribution chart). Module-layout Verification #1 records the workaround: `READ-ONLY` for `06_annotations` tasks. The workaround scales poorly — a new `06_annotations` contributor reading only the directory glob will not know the file is off-limits.
- **Suggestion**: Move physical file to `src/shared/classColors.ts` (introducing a `src/shared/` layer for true Layer-0 utilities) or to `src/components/detection/classColors.ts` (under `03_shared-ui`). Either move drops the workaround and aligns physical/logical ownership.
- **Task / Epic**: Step 4 testability — minimal, surgical move (rename + import-path update across 4 consumers).
### F4: No Public API barrels — every internal file is de-facto public (High / Architecture)
- **Location**: every component root (no `src/<component>/index.ts` exists today; only `src/types/index.ts` and `mission-planner/src/types/index.ts` are barrels and they're re-export hubs, not component facades).
- **Description**: Cross-component imports use file-name granularity (`import { api } from '../api/client'`, `import { useFlight } from '../../components/FlightContext'`, etc.). Consequence: there is **no enforceable Public API surface**. Any internal refactor inside a component (split a file, rename an export) is a breaking change to every importer. Phase 7 Check #2 ("Public API respect") cannot meaningfully fail in this codebase because everything is public. Module-layout Verification #3 records the same observation.
- **Suggestion**: Step 4 testability candidate — add `src/<component>/index.ts` for every component, re-exporting only the symbols listed in module-layout's "Public API (de-facto)" line for that component. Then a future Phase 7 invocation can flag deep imports as Architecture findings instead of folding into background noise.
- **Task / Epic**: Step 4 testability (single mechanical change per component; ~11 new files + ~30 import-path edits).
### 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)
- **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 repeats `/api/<service>/<path>` as a string literal. Testability suffers — every test fixture must duplicate paths; any nginx-route change touches every feature. Architecture intent (ADR-006 Consequences) explicitly flags this: *"The SPA hardcodes /api/<service>/... paths in source instead of an env-driven base URL — testability is poor (finding tracked)."*
- **Suggestion**: Step 4 testability — introduce `src/shared/endpoints.ts` (or per-component `endpoints.ts` if shared/ is deferred) that exposes typed builders: `endpoints.auth.login()`, `endpoints.flights.byId(id)`, `endpoints.annotations.media(query)`, etc. Replace every string-literal path. Allows tests to mock at the endpoints layer rather than at every `fetch` call. Compounds well with F6 if `src/shared/` lands first.
- **Task / Epic**: Step 4 testability (mechanical extract; per-component cohort).
### 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.)
Reference in New Issue View Git Blame Copy Permalink