Files
ui/_docs/02_document/architecture_compliance_baseline.md
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

23 KiB
Raw Permalink Blame History

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.tsCOORDINATE_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:9import 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/classColorsCLOSED 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//... 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.)