diff --git a/_docs/02_document/architecture_compliance_baseline.md b/_docs/02_document/architecture_compliance_baseline.md index 67d3bf1..53be988 100644 --- a/_docs/02_document/architecture_compliance_baseline.md +++ b/_docs/02_document/architecture_compliance_baseline.md @@ -21,10 +21,10 @@ Detection approach: TypeScript `import ... from '...'` parsing across all `.ts` | 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 | +| 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//...` paths instead of env-driven endpoints | +| F7 | Medium | Architecture | every `api.*` / `createSSE` call site | Hardcoded `/api//...` 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 | @@ -86,12 +86,16 @@ Detection approach: TypeScript `import ... from '...'` parsing across all `.ts` - **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) +### F4: No Public API barrels — every internal file is de-facto public (High / Architecture) — **CLOSED 2026-05-11 by AZ-485 (commit `23746ec`)** -- **Location**: every component root (no `src//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//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). +- **Resolution**: 11 component barrels (`src//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` — the file is logically owned by `11_class-colors` but physically lives under `06_annotations` (F3). Re-exporting it through the `06_annotations` barrel creates a circular import (`AnnotationsPage → DetectionClasses → 06_annotations barrel → AnnotationsPage`). Consumers import the path directly under an `EXEMPT_RE` in the static check. The exemption disappears when F3 moves the file. + +- **Pre-resolution context (preserved for trace)**: + - **Location**: every component root (no `src//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//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) @@ -111,12 +115,16 @@ Detection approach: TypeScript `import ... from '...'` parsing across all `.ts` - `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//...` paths instead of env-driven endpoints (Medium / Architecture) +### F7: Hardcoded `/api//...` paths instead of env-driven endpoints (Medium / Architecture) — **CLOSED 2026-05-11 by AZ-486 (commit `8a461a2`)** -- **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//` 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//... 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). +- **Resolution**: `src/api/endpoints.ts` introduced as the single source of truth — 25 typed builders covering every `/api//` 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//` 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//` 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) diff --git a/_docs/02_document/components/01_api-transport/description.md b/_docs/02_document/components/01_api-transport/description.md index b9d54e7..47b28f6 100644 --- a/_docs/02_document/components/01_api-transport/description.md +++ b/_docs/02_document/components/01_api-transport/description.md @@ -28,6 +28,16 @@ |--------|-----------|-------| | `subscribe(url, onMessage, onError?): { close }` | factory | Creates `EventSource` with the **bearer token in the query string** (browser `EventSource` can't set headers). Returns a `close()` handle. | +### `src/api/endpoints.ts` (since AZ-486 / F7) + +| Export | Signature | Notes | +|--------|-----------|-------| +| `endpoints` | `Readonly<{ admin, annotations, flights, detect }>` of typed builder functions | Single source of truth for every `/api//...` URL the UI talks to. Each leaf is a function — `() => string` for constant paths, `(id, ...) => string` for parameterised ones. Wire-contract pinned by `src/api/endpoints.test.ts` (36 assertions). | + +### `src/api/index.ts` (Public API barrel, since AZ-485 / F4) + +Re-exports the component's public surface: `api`, `createSSE`, `setToken`, `getToken`, `getApiBase`, `setNavigateToLogin`, `endpoints`. Consumers OUTSIDE this component MUST import from the barrel; direct imports of `src/api/{client,sse,endpoints}` from other components are blocked by `STC-ARCH-01`. + ## 3. External API Specification This component does not *expose* an API; it consumes the suite's. The set of consumed endpoints (collected from feature module docs): @@ -40,7 +50,7 @@ This component does not *expose* an API; it consumes the suite's. The set of con | `detect/` | `/api/detect/...` | `06_annotations` | | `loader/`, `resource/`, `gps-denied-*`, `autopilot/` | `/api/{loader,resource,gps-denied-desktop,gps-denied-onboard,autopilot}/...` | various features | -**No service-specific client modules exist**. URL strings are inlined at every call site (testability finding from autodev Step 4). +**No service-specific client modules exist**. URL strings are produced by typed builders in `src/api/endpoints.ts` (added by AZ-486 / F7, commit `8a461a2`) — the previous "URL strings inlined at every call site" testability finding (F7) is **CLOSED**. The `STC-ARCH-02` static gate (`scripts/check-arch-imports.mjs --mode=api-literals`, wired into `scripts/run-tests.sh`) forbids re-introducing `/api//...` literals under `src/`. ## 5. Implementation Details @@ -60,7 +70,7 @@ This component does not *expose* an API; it consumes the suite's. The set of con - **No timeout / cancellation**. (Step 4.) - **Bearer in SSE query string**. Accepted trade-off; document in `security_approach.md` (Step 6). - **No reconnect-on-token-rotate** for SSE consumers — every feature that uses SSE will silently stop receiving events after the first refresh (Step 8 hardening). -- **No service-specific clients** → URL strings duplicated across features. Risk of typos surfacing as 404s only at runtime (Step 4). +- ~~No service-specific clients~~ → **CLOSED by AZ-486 / F7**: URL strings centralised in `src/api/endpoints.ts`; STC-ARCH-02 enforces it. Typos now surface at build time (TS strict on the builder names) or in `endpoints.test.ts`, never at runtime. ## 8. Dependency Graph @@ -76,3 +86,5 @@ This component does not *expose* an API; it consumes the suite's. The set of con |------|------------| | `src/api/client.ts` | `_docs/02_document/modules/src__api__client.md` | | `src/api/sse.ts` | `_docs/02_document/modules/src__api__sse.md` | +| `src/api/endpoints.ts` | `_docs/02_document/modules/src__api__endpoints.md` | +| `src/api/index.ts` (barrel) | (no separate doc — re-exports surface listed in §2 above) | diff --git a/_docs/02_document/modules/src__api__client.md b/_docs/02_document/modules/src__api__client.md index 9a5ba90..07b6cd8 100644 --- a/_docs/02_document/modules/src__api__client.md +++ b/_docs/02_document/modules/src__api__client.md @@ -42,11 +42,11 @@ export const api = { - `204` → `undefined as T`. - `!res.ok` → throw `new Error(\`${status}: ${text || statusText}\`)`. Body text is read defensively (`.catch(() => '')`). - Otherwise → `res.json()` (no schema validation — caller types the response). -- `refreshToken()` — `POST /api/admin/auth/refresh` with `credentials: 'include'`. On 200, expects `{ token: string }` and stores it. Returns boolean. +- `refreshToken()` — `POST endpoints.admin.authRefresh()` (i.e. `/api/admin/auth/refresh`) with `credentials: 'include'`. On 200, expects `{ token: string }` and stores it. Returns boolean. (Path produced by the `endpoints` builder; closes F7.) ## Dependencies -- **Internal**: none. +- **Internal**: `./endpoints` — `endpoints.admin.authRefresh()` used by the internal `refreshToken()` helper (since AZ-486 / F7). - **External**: `fetch`, `Headers`, `FormData`, `Response` (browser globals). No npm runtime dependency. ## Consumers (intra-repo) @@ -71,9 +71,9 @@ None defined here. The generic `T` parameter is supplied by call sites. ## Configuration -URLs are **string literals** at every call site (`/api/admin/...`, `/api/flights?...`, etc.). There is no base-URL constant. The `vite.config.ts` dev proxy and `nginx.conf` production rules forward `/api/*` to per-service backends. +URLs are produced by typed builders in `src/api/endpoints.ts` (see `src__api__endpoints.md`) — the F7 finding from the architecture baseline is now CLOSED. Every consumer (this module included) imports `endpoints` and calls `endpoints..(...)`; the `STC-ARCH-02` static gate forbids re-introducing literal `/api//...` strings under `src/`. -A `VITE_API_BASE_URL` env-var fix is the canonical Step 4 testability candidate (workspace `README.md` calls this out). +There is no base-URL constant: the path strings are still relative. The `vite.config.ts` dev proxy and `nginx.conf` production rules forward `/api/*` to per-service backends. `getApiBase()` (exported from this module) supplies the host prefix at runtime where the consumer needs an absolute URL (e.g. the manual `fetch(getApiBase() + endpoints.admin.authRefresh(), ...)` call inside `refreshToken()`). ## External integrations diff --git a/_docs/02_document/modules/src__api__endpoints.md b/_docs/02_document/modules/src__api__endpoints.md new file mode 100644 index 0000000..13699b2 --- /dev/null +++ b/_docs/02_document/modules/src__api__endpoints.md @@ -0,0 +1,135 @@ +# Module: `src/api/endpoints.ts` + +> **Source**: `src/api/endpoints.ts` (79 lines) +> **Topo batch**: B2 (leaf — no internal imports) +> **Introduced**: AZ-486 (2026-05-11, commit `8a461a2`), closing architecture baseline finding F7. + +## Purpose + +Single source of truth for every `/api//` URL the UI talks to. Replaces the hardcoded string literals that previously lived at each `api.*` / `createSSE` call site (and at every `src={...}` URL for API-served images / videos). The `endpoints` object is the canonical wire-contract documentation: each builder produces a character-identical string to the literal it superseded, so MSW handlers + e2e stubs + the nginx routing table all keep matching. + +Together with the `STC-ARCH-02` static gate (see [Configuration](#configuration)), this module enforces "no hardcoded API path literals in `src/`" as a build-time invariant rather than a code-review aspiration. + +## Public interface + +```ts +export const endpoints = { + admin: { + authRefresh: () => string + authLogin: () => string + authLogout: () => string + users: () => string + user: (id: string) => string + classes: () => string + class: (id: string | number) => string + }, + annotations: { + classes: () => string + settingsUser: () => string + settingsSystem: () => string + settingsDirectories: () => string + annotations: () => string + annotationsByMedia: (mediaId: string, pageSize?: number) => string // pageSize default = 1000 + annotationImage: (annotationId: string) => string + annotationThumbnail: (annotationId: string) => string + annotationEvents: () => string + media: (queryString: string) => string + mediaFile: (mediaId: string) => string + mediaItem: (mediaId: string) => string + mediaBatch: () => string + dataset: (queryString: string) => string + datasetItem: (annotationId: string) => string + datasetBulkStatus: () => string + datasetClassDistribution: () => string + }, + flights: { + collection: (queryString?: string) => string // GET ?pageSize=... lists; POST (no qs) creates + aircrafts: () => string + aircraft: (id: string) => string + flight: (id: string) => string + flightWaypoints: (id: string) => string + flightWaypoint: (flightId: string, waypointId: string) => string + flightLiveGps: (id: string) => string + }, + detect: { + media: (mediaId: string) => string // POST → trigger detection for a media item + }, +} as const +``` + +The whole object is `as const`, so each leaf's return type is the narrow string literal where possible (e.g. `'/api/admin/auth/refresh'`) and the parameterised builders carry a `string` return. + +## Internal logic + +- **Pure data + template strings.** No side effects, no I/O, no caching. Every builder is a one-line `() => '...'` or arrow returning a template literal. +- **Function form (not constants)**, per direction at task-creation time: + - Parameterised paths (e.g. `flight(id)`) need a function anyway. Keeping every entry as a function — even the constant ones — gives a single uniform call shape at every site (`endpoints.x.y()`) so reviewers don't have to remember which entries take parens and which don't. + - Per-builder tree-shaking under Vite's production rollup remains intact. +- **Query strings owned by the caller for variable-shape paths.** Where the query is dynamic (`flights.collection`, `annotations.media`, `annotations.dataset`), the caller builds a `URLSearchParams.toString()` and the builder owns only the path + `?`. This keeps the wire contract identical to pre-refactor literals at every callsite. + +## Public API (barrel re-export) + +`src/api/index.ts` re-exports `endpoints` alongside `api`, `createSSE`, `setToken`, `getToken`, `getApiBase`, `setNavigateToLogin`. Consumers OUTSIDE the `01_api-transport` component MUST import from the barrel (`import { endpoints } from '@/api'` or `from '../api'`) — direct imports of `src/api/endpoints` from other components are blocked by `STC-ARCH-01` (F4 closure, see `src__api__client.md`). + +## Dependencies + +- **Internal**: none. +- **External**: none. + +## Consumers (intra-repo) + +After the AZ-486 migration, `endpoints` is imported by: + +- `src/api/client.ts` — internal `refreshToken()` helper uses `endpoints.admin.authRefresh()`. +- `src/auth/AuthContext.tsx` — `authRefresh`, `authLogin`, `authLogout`. +- `src/components/FlightContext.tsx` — `flights.collection`, `flights.flight`, `annotations.settingsUser`. +- `src/components/DetectionClasses.tsx` — `admin.classes`, `admin.class`. +- `src/features/admin/AdminPage.tsx` — `admin.users`, `admin.user`. +- `src/features/annotations/AnnotationsPage.tsx` — annotation CRUD endpoints, `detect.media`. +- `src/features/annotations/AnnotationsSidebar.tsx` — `annotations.annotationEvents` (SSE), bulk-status, dataset endpoints. +- `src/features/annotations/CanvasEditor.tsx` — `annotations.annotationImage`, `annotations.annotationThumbnail`. +- `src/features/annotations/MediaList.tsx` — `annotations.media`, `annotations.mediaFile`, `annotations.mediaItem`, `annotations.mediaBatch`. +- `src/features/annotations/VideoPlayer.tsx` — `annotations.mediaFile`. +- `src/features/dataset/DatasetPage.tsx` — `annotations.dataset*` family, `annotations.classes`, `annotations.annotationImage`. +- `src/features/flights/FlightsPage.tsx` — full `flights.*` surface + `annotations.settingsUser`. +- `src/features/settings/SettingsPage.tsx` — `annotations.settings*`, `flights.aircrafts`. + +This is the full intra-repo consumer list — `STC-ARCH-02` guarantees no production-source caller falls outside it. + +## Data models + +None defined here. Path-string output only. + +## Configuration + +The module IS the API-path configuration. The only "config" is the nginx routing table that maps each `/api//...` prefix to a concrete backend service — see `src__api__client.md` → External integrations for the live table. + +**Static enforcement (`STC-ARCH-02`)**: + +- **Script**: `scripts/check-arch-imports.mjs --mode=api-literals`. +- **Wired into**: `scripts/run-tests.sh` (functional profile, static group) — runs before any unit test. +- **What it forbids**: any `/api//...` literal in `[`'"]` quoting under `src/`. +- **Exempt files**: this file (`src/api/endpoints.ts`) and `src/**/*.test.ts(x)` only. +- **Bypass policy**: none. Adding a new exempt path requires updating the exempt regex in the script AND a `module-layout.md` rule revision in the same commit. + +## External integrations + +This module integrates nothing directly. It documents — as TypeScript values — the wire contract for every external integration the SPA has, as routed by `nginx.conf`. See the routing table in `src__api__client.md` → External integrations for the per-prefix backend mapping. + +## Security + +- **No bearer plumbing here.** Token injection still happens in `client.ts` (`Authorization` header) and `sse.ts` (`access_token` query parameter). Builders return URLs **without** the token. +- **No URL-encoding** of interpolated `id` / `mediaId` / `queryString` parameters. All current callsites pass already-safe values (UUIDs, ints, pre-built `URLSearchParams.toString()` output). If any future caller passes user-controlled text, the builder must add `encodeURIComponent` (see open question below). +- **No CSRF surface change** — same posture as the pre-refactor literals. + +## Tests + +- **`src/api/endpoints.test.ts`** (36 Vitest assertions): pins every builder's output to its exact pre-refactor URL string. This is the contract documentation — any wire-contract change MUST update this test in the same commit as the backend / MSW / e2e stub change. Includes one barrel-re-export assertion (`endpoints` is reachable via `import { endpoints } from '../api'`). +- **`tests/architecture_imports.test.ts`** (AZ-486 / STC-ARCH-02 suite, 6 cases): verifies the static gate passes on the migrated codebase AND fails when a synthetic single-quoted / double-quoted / template-literal `/api//...` literal is introduced in `src/`. Also verifies the `*.test.ts` and `//` comment exemptions. + +## Notes / open questions + +- **`detect.media` only exposes the single-segment path** that the UI uses today (`POST /api/detect/`). The full `detect/` service has more endpoints (per the nginx table) but no UI callsite consumes them. Add new builders only when a real callsite needs them — don't pre-populate. +- **`flights.collection` overloads its return** on whether `queryString` is provided. Acceptable while the contract is "GET with `?pageSize`, POST without" — but if a third flights-collection verb (DELETE? PUT?) is ever added with its own query shape, split into named builders rather than threading more conditional logic through one. +- **No URL-encoding of interpolated params** (see Security). Add `encodeURIComponent` at the first callsite that needs it, plus a contract-test case in `endpoints.test.ts`. Currently safe across all 36 pinned URLs. +- **Wire-contract test coverage is exact-string, not shape.** This is deliberate: a "looks like a path" matcher would silently accept a hyphen-to-underscore change that breaks the backend. Updating these strings IS a wire-contract change — treat the test as a release-gate. diff --git a/_docs/02_document/modules/src__api__sse.md b/_docs/02_document/modules/src__api__sse.md index 15d3c1b..8d7fd69 100644 --- a/_docs/02_document/modules/src__api__sse.md +++ b/_docs/02_document/modules/src__api__sse.md @@ -49,7 +49,7 @@ None defined here. The generic `T` is supplied by the caller. ## Configuration -URLs are passed in by callers (string-literal at call sites). The same testability remark as `api/client.ts` applies: a `VITE_API_BASE_URL` is the natural Step 4 fix. +URLs are passed in by callers. Since AZ-486 / F7 (commit `8a461a2`), callers obtain those URLs from `endpoints.*` builders in `src/api/endpoints.ts` rather than from inline string literals. The `STC-ARCH-02` static gate enforces this at every callsite under `src/`. `createSSE` itself is path-agnostic and takes any `url` — the `endpoints` discipline is upheld at the call site, not here. ## External integrations diff --git a/_docs/02_document/modules/src__auth__AuthContext.md b/_docs/02_document/modules/src__auth__AuthContext.md index 80b6a61..d9a0e95 100644 --- a/_docs/02_document/modules/src__auth__AuthContext.md +++ b/_docs/02_document/modules/src__auth__AuthContext.md @@ -34,18 +34,18 @@ State: **Bootstrap effect (mount-only)**: ```ts -api.get<{ user: AuthUser; token: string }>('/api/admin/auth/refresh') +api.get<{ user: AuthUser; token: string }>(endpoints.admin.authRefresh()) .then(data => { setToken(data.token); setUser(data.user) }) .catch(() => {}) .finally(() => setLoading(false)) ``` -The refresh endpoint is invoked with `credentials: 'include'` only inside `client.ts`'s **internal** `refreshToken()` helper — but here we go through the public `api.get()` path, which does NOT include credentials. **This is a real divergence**: `client.ts`'s internal `refreshToken()` (used in the 401 retry) sends the cookie; the bootstrap call in `AuthContext` does not. The endpoint must therefore accept the refresh either via cookie (then bootstrap fails silently for non-cookie clients — which is everyone after a hard reload) **or** via some other mechanism (a refresh token in `localStorage`, etc.). **Flag for Step 4 verification** against the `admin/` service contract; this is likely a real bug masking by silent `.catch`. +The refresh endpoint is invoked with `credentials: 'include'` only inside `client.ts`'s **internal** `refreshToken()` helper — but here we go through the public `api.get()` path, which does NOT include credentials. **This is a real divergence**: `client.ts`'s internal `refreshToken()` (used in the 401 retry) sends the cookie; the bootstrap call in `AuthContext` does not. The endpoint must therefore accept the refresh either via cookie (then bootstrap fails silently for non-cookie clients — which is everyone after a hard reload) **or** via some other mechanism (a refresh token in `localStorage`, etc.). **Flag for Step 4 verification** against the `admin/` service contract; this is likely a real bug masking by silent `.catch`. The path string itself is unaffected by AZ-486 — `endpoints.admin.authRefresh()` produces `'/api/admin/auth/refresh'` character-identically to the pre-refactor literal, so the divergence is structural, not URL-based. **`login(email, password)`**: ```ts -const data = await api.post<{ token; user }>('/api/admin/auth/login', { email, password }) +const data = await api.post<{ token; user }>(endpoints.admin.authLogin(), { email, password }) setToken(data.token); setUser(data.user) ``` @@ -54,7 +54,7 @@ Throws to caller (LoginPage) on bad credentials. **`logout()`**: ```ts -try { await api.post('/api/admin/auth/logout') } catch {} +try { await api.post(endpoints.admin.authLogout()) } catch {} setToken(null); setUser(null) ``` @@ -65,7 +65,7 @@ Network failure on logout is silently swallowed because we want to clear local a ## Dependencies - **Internal**: - - `../api/client` — `api`, `setToken`. + - `../api` (barrel) — `api`, `endpoints`, `setToken`. (Since AZ-485 / F4 + AZ-486 / F7: barrel import + endpoint builders.) - `../types` — `AuthUser` type. - **External**: `react` (`createContext`, `useContext`, `useState`, `useCallback`, `useEffect`, `ReactNode`). @@ -86,7 +86,7 @@ From the §7a dependency graph: ## Configuration -Endpoints (string-literal): `/api/admin/auth/refresh`, `/api/admin/auth/login`, `/api/admin/auth/logout`. Routed by `nginx.conf` to the `admin/` service. +Endpoints (typed builders from `../api/endpoints`, since AZ-486 / F7): `endpoints.admin.authRefresh()`, `endpoints.admin.authLogin()`, `endpoints.admin.authLogout()` — producing `/api/admin/auth/refresh`, `.../login`, `.../logout` respectively. Routed by `nginx.conf` to the `admin/` service. No env vars consumed directly — token storage policy is defined in `client.ts` (in-memory; not persisted to `localStorage`). diff --git a/_docs/02_document/modules/src__components__DetectionClasses.md b/_docs/02_document/modules/src__components__DetectionClasses.md index 4df741b..325a2d7 100644 --- a/_docs/02_document/modules/src__components__DetectionClasses.md +++ b/_docs/02_document/modules/src__components__DetectionClasses.md @@ -24,7 +24,7 @@ Fully controlled — the parent owns `selectedClassNum` and `photoMode`. The cur ## Internal logic - **Class catalogue load** (mount-only `useEffect`): - - `api.get('/api/annotations/classes')`. + - `api.get(endpoints.annotations.classes())` (= `/api/annotations/classes`, since AZ-486 / F7). - On a non-empty array → `setClasses(list)`. - On an empty array OR a thrown error → `setClasses(FALLBACK_CLASSES)`. - **`FALLBACK_CLASSES`** is a module-private 3 × |`FALLBACK_CLASS_NAMES`| matrix: @@ -45,8 +45,8 @@ Fully controlled — the parent owns `selectedClassNum` and `photoMode`. The cur ## Dependencies - **Internal**: - - `../api/client` — `api.get()`. - - `../features/annotations/classColors` — `getClassColor(i)`, `FALLBACK_CLASS_NAMES`. + - `../api` (barrel) — `api`, `endpoints`. (Since AZ-485 / F4 + AZ-486 / F7.) + - `../features/annotations/classColors` — `getClassColor(i)`, `FALLBACK_CLASS_NAMES`. (Cross-component import preserved; flagged in Consumers below.) - `../types` — `DetectionClass` type. - **External**: `react`, `react-i18next`, `react-icons/md`, `react-icons/fa`. @@ -70,7 +70,7 @@ This is the **canonical example** of the cross-layer import flagged in `_docs/02 ## Configuration -Endpoint: `/api/annotations/classes` — string-literal URL (testability fix scheduled for Step 4). +Endpoint: `endpoints.annotations.classes()` → `/api/annotations/classes` (typed builder from `../api/endpoints`, since AZ-486 / F7). Photo-mode value set is `{0, 20, 40}` — hardcoded, mirrored by `FALLBACK_CLASSES`. If the backend grows a fourth mode (e.g. thermal, IR), every consumer of `photoMode` will need a coordinated change. @@ -78,7 +78,7 @@ Tailwind tokens: `bg-az-orange` (Regular), `bg-az-blue` (Winter), `bg-purple-600 ## External integrations -- HTTP `GET /api/annotations/classes` → `DetectionClass[]`. Backed by the `annotations/` service (`.NET`) per `nginx.conf`. +- HTTP `GET endpoints.annotations.classes()` (= `/api/annotations/classes`) → `DetectionClass[]`. Backed by the `annotations/` service (`.NET`) per `nginx.conf`. ## Security diff --git a/_docs/02_document/modules/src__components__FlightContext.md b/_docs/02_document/modules/src__components__FlightContext.md index 1dc95f0..ad56edf 100644 --- a/_docs/02_document/modules/src__components__FlightContext.md +++ b/_docs/02_document/modules/src__components__FlightContext.md @@ -27,13 +27,13 @@ export function FlightProvider({ children }: { children: ReactNode }): JSX.Eleme State: -- `flights: Flight[]` — most recent list returned by `GET /api/flights?pageSize=1000`. +- `flights: Flight[]` — most recent list returned by `GET endpoints.flights.collection('pageSize=1000')` (= `/api/flights?pageSize=1000`). - `selectedFlight: Flight | null` — the active flight, or `null` if none. Survives across pages because the provider is mounted above the route tree. **`refreshFlights()`** (`useCallback`, no deps): ```ts -const data = await api.get<{ items: Flight[] }>('/api/flights?pageSize=1000') +const data = await api.get<{ items: Flight[] }>(endpoints.flights.collection('pageSize=1000')) setFlights(data.items ?? []) ``` @@ -42,8 +42,8 @@ Errors are silently swallowed (`try { ... } catch {}`). `pageSize=1000` is a har **Bootstrap effect** (`useEffect` keyed on `[refreshFlights]`, runs once because `refreshFlights` is `useCallback([])`): 1. `refreshFlights()` (no `await` — runs in parallel with #2). -2. `api.get('/api/annotations/settings/user')` → - - if `settings?.selectedFlightId` is truthy: `api.get('/api/flights/${settings.selectedFlightId}')` → `setSelectedFlight(f)`. +2. `api.get(endpoints.annotations.settingsUser())` → + - if `settings?.selectedFlightId` is truthy: `api.get(endpoints.flights.flight(settings.selectedFlightId))` → `setSelectedFlight(f)`. - errors at every step silently swallowed. The selected flight is therefore looked up by **its own GET**, not by indexing into the cached `flights` list. This is intentional — the user might have a `selectedFlightId` that fell off the first 1000 flights. @@ -52,7 +52,7 @@ The selected flight is therefore looked up by **its own GET**, not by indexing i ```ts setSelectedFlight(f) -api.put('/api/annotations/settings/user', { selectedFlightId: f?.id ?? null }).catch(() => {}) +api.put(endpoints.annotations.settingsUser(), { selectedFlightId: f?.id ?? null }).catch(() => {}) ``` Optimistic — local state updates immediately; the persisted setting is fire-and-forget. If the PUT fails, the next reload will fall back to the previously-stored ID and the user's selection silently reverts. Flag for Step 4. @@ -60,7 +60,7 @@ Optimistic — local state updates immediately; the persisted setting is fire-an ## Dependencies - **Internal**: - - `../api/client` — `api`. + - `../api` (barrel) — `api`, `endpoints`. (Since AZ-485 / F4 + AZ-486 / F7: barrel import + endpoint builders.) - `../types` — `Flight`, `UserSettings` types. - **External**: `react` (`createContext`, `useContext`, `useState`, `useEffect`, `useCallback`, `ReactNode`). @@ -80,9 +80,9 @@ From the §7a dependency graph: ## Configuration -Endpoints (string-literal): `/api/flights?pageSize=1000`, `/api/flights/{id}`, `/api/annotations/settings/user`. Routed by `nginx.conf` to the `flights/` and `annotations/` services. +Endpoints (typed builders from `../api/endpoints`, since AZ-486 / F7): `endpoints.flights.collection('pageSize=1000')`, `endpoints.flights.flight(id)`, `endpoints.annotations.settingsUser()` — producing `/api/flights?pageSize=1000`, `/api/flights/{id}`, `/api/annotations/settings/user` respectively. Routed by `nginx.conf` to the `flights/` and `annotations/` services. -`pageSize=1000` is a **hardcoded ceiling** — if the system ever has >1000 flights, the cache is silently truncated. The `flights/` service contract probably caps at 1000; verify and document in `data_parameters.md` (Step 6). Flag. +`pageSize=1000` is a **hardcoded ceiling** — if the system ever has >1000 flights, the cache is silently truncated. The `flights/` service contract probably caps at 1000; verify and document in `data_parameters.md` (Step 6). Flag. (Note: the literal lives in the caller, not in the `endpoints.flights.collection` builder — moving the ceiling into the builder is a future change.) ## External integrations diff --git a/_docs/02_document/modules/src__features__admin__AdminPage.md b/_docs/02_document/modules/src__features__admin__AdminPage.md index f29c865..01a343d 100644 --- a/_docs/02_document/modules/src__features__admin__AdminPage.md +++ b/_docs/02_document/modules/src__features__admin__AdminPage.md @@ -40,9 +40,9 @@ No props. Reads everything via `api/client` and local state. - **Bootstrap effect** (`useEffect([])` — runs once at mount): ```ts - api.get('/api/annotations/classes').then(setClasses).catch(() => {}) - api.get('/api/flights/aircrafts').then(setAircrafts).catch(() => {}) - api.get('/api/admin/users').then(setUsers).catch(() => {}) + api.get(endpoints.annotations.classes()).then(setClasses).catch(() => {}) + api.get(endpoints.flights.aircrafts()).then(setAircrafts).catch(() => {}) + api.get(endpoints.admin.users()).then(setUsers).catch(() => {}) ``` Three independent calls, all silently swallowed on error. No retry, @@ -51,10 +51,11 @@ No props. Reads everything via `api/client` and local state. `_docs/ui_design/README.md`. - **`handleAddClass()`**: 1. Guard: `if (!newClass.name) return`. - 2. `await api.post('/api/admin/classes', newClass)`. - 3. Refetch via `api.get('/api/annotations/classes')` — note the - **read** path is the public `annotations/` endpoint, while the - **write** path is the `admin/` endpoint. Architectural caveat: + 2. `await api.post(endpoints.admin.classes(), newClass)` (= `/api/admin/classes`). + 3. Refetch via `api.get(endpoints.annotations.classes())` — note the + **read** path is the public `annotations/` endpoint + (`/api/annotations/classes`), while the **write** path is the + `admin/` endpoint (`/api/admin/classes`). Architectural caveat: two different services own the same logical entity. Document in `architecture.md` §integration-points (Step 3a). 4. Reset `newClass` to its initial values. @@ -62,25 +63,26 @@ No props. Reads everything via `api/client` and local state. non-2xx); the throw is uncaught and reaches React's error boundary (none configured). Flag. - **`handleDeleteClass(id)`**: optimistic local update — - `await api.delete('/api/admin/classes/${id}')` then - `setClasses(prev => prev.filter(c => c.id !== id))`. **No + `await api.delete(endpoints.admin.class(id))` (= `/api/admin/classes/${id}`) + then `setClasses(prev => prev.filter(c => c.id !== id))`. **No ConfirmDialog** despite this being destructive. Inconsistent with the user-deactivation flow which uses ConfirmDialog. Flag for Step 4 against `_docs/ui_design/README.md` confirmation-dialog spec. - **`handleAddUser()`** — analogous to `handleAddClass` against - `POST /api/admin/users` and `GET /api/admin/users`. Guards on - `email && password`. + `POST endpoints.admin.users()` and `GET endpoints.admin.users()` + (both → `/api/admin/users`). Guards on `email && password`. - **`handleDeactivate()`** — fired from the ConfirmDialog confirm: - 1. `PATCH /api/admin/users/${deactivateId}` with `{ isActive: false }`. + 1. `PATCH endpoints.admin.user(deactivateId)` (= `/api/admin/users/${deactivateId}`) with `{ isActive: false }`. 2. Optimistic local update: marks the row inactive. 3. Closes the dialog (`setDeactivateId(null)`). No "reactivate" path — once `isActive: false`, the row only renders the badge and no Deactivate button. Verify with `admin/` service: is reactivation an admin task or out of scope? -- **`handleToggleDefault(a)`** — `PATCH /api/flights/aircrafts/${a.id}` - with `{ isDefault: !a.isDefault }`, then optimistic local flip. Note - this allows multiple `isDefault: true` aircraft to coexist (the - backend should enforce exclusivity; the UI does not). +- **`handleToggleDefault(a)`** — `PATCH endpoints.flights.aircraft(a.id)` + (= `/api/flights/aircrafts/${a.id}`) with `{ isDefault: !a.isDefault }`, + then optimistic local flip. Note this allows multiple `isDefault: + true` aircraft to coexist (the backend should enforce exclusivity; + the UI does not). - **Layout** (left → center → right, all in one horizontal flex): - **Left column** (`w-[340px]`): detection-classes table + add row. - **Center column** (`flex-1 max-w-md`): AI settings form, GPS @@ -93,7 +95,7 @@ No props. Reads everything via `api/client` and local state. ## Dependencies - **Internal**: - - `../../api/client` — `api`. + - `../../api` (barrel) — `api`, `endpoints`. (Since AZ-485 / F4 + AZ-486 / F7.) - `../../components/ConfirmDialog` — for user deactivation. - `../../types` — `DetectionClass`, `Aircraft`, `User`. - **External**: `react` (`useState`, `useEffect`), @@ -137,19 +139,18 @@ backend assigns `id` and other server-managed fields. ## External integrations -| Method | Path | Purpose | +| Method | Builder → Path | Purpose | |---|---|---| -| `GET` | `/api/annotations/classes` | List detection classes (read path uses annotations service) | -| `POST` | `/api/admin/classes` | Create detection class (write path uses admin service) | -| `DELETE` | `/api/admin/classes/{id}` | Delete detection class | -| `GET` | `/api/flights/aircrafts` | List aircraft | -| `PATCH` | `/api/flights/aircrafts/{id}` | Toggle `isDefault` | -| `GET` | `/api/admin/users` | List users | -| `POST` | `/api/admin/users` | Create user | -| `PATCH` | `/api/admin/users/{id}` | Set `isActive: false` (deactivate) | +| `GET` | `endpoints.annotations.classes()` → `/api/annotations/classes` | List detection classes (read path uses annotations service) | +| `POST` | `endpoints.admin.classes()` → `/api/admin/classes` | Create detection class (write path uses admin service) | +| `DELETE` | `endpoints.admin.class(id)` → `/api/admin/classes/{id}` | Delete detection class | +| `GET` | `endpoints.flights.aircrafts()` → `/api/flights/aircrafts` | List aircraft | +| `PATCH` | `endpoints.flights.aircraft(id)` → `/api/flights/aircrafts/{id}` | Toggle `isDefault` | +| `GET` | `endpoints.admin.users()` → `/api/admin/users` | List users | +| `POST` | `endpoints.admin.users()` → `/api/admin/users` | Create user | +| `PATCH` | `endpoints.admin.user(id)` → `/api/admin/users/{id}` | Set `isActive: false` (deactivate) | -Routed by `nginx.conf` to `admin/`, `annotations/`, `flights/` -backends. +Path builders live in `src/api/endpoints.ts` (since AZ-486 / F7). Routed by `nginx.conf` to `admin/`, `annotations/`, `flights/` backends. ## Security diff --git a/_docs/02_document/modules/src__features__annotations.md b/_docs/02_document/modules/src__features__annotations.md index 77a0eda..f22b998 100644 --- a/_docs/02_document/modules/src__features__annotations.md +++ b/_docs/02_document/modules/src__features__annotations.md @@ -9,19 +9,21 @@ Owns the `/annotations` route. Lets the user: 2. Play / pause / step a video, scrub the timeline, mute, with frame stepping at 1 / 5 / 10 / 30 / 60 frames in both directions (assumed 30 FPS — see Findings). 3. Draw / move / resize / multi-select bounding boxes on a custom canvas overlay, click-and-drag with Ctrl-modifier behaviours, snap to image-normalized coordinates 0–1. 4. Pick the active detection class (1–9 hotkeys) and PhotoMode (Regular / Winter / Night) via the shared `DetectionClasses` component. -5. Save the per-frame detection set back to `POST /api/annotations/annotations`, with **fall-back to in-memory storage** when the file is a local blob: URL or the backend is unreachable. -6. Stream the annotations sidebar from the `GET /api/annotations/annotations/events` SSE feed; show each row with the gradient defined in `_docs/ui_design/README.md`. -7. Trigger AI detection via `POST /api/detect/{mediaId}` (modal log overlay). +5. Save the per-frame detection set back to `POST endpoints.annotations.annotations()` (= `/api/annotations/annotations`), with **fall-back to in-memory storage** when the file is a local blob: URL or the backend is unreachable. +6. Stream the annotations sidebar from the `GET endpoints.annotations.annotationEvents()` (= `/api/annotations/annotations/events`) SSE feed; show each row with the gradient defined in `_docs/ui_design/README.md`. +7. Trigger AI detection via `POST endpoints.detect.media(mediaId)` (= `/api/detect/{mediaId}`) — modal log overlay. 8. Download an annotation as YOLO `.txt` + a PNG of the frame with rectangles burned in. +> All path strings produced by `endpoints.*` builders from `src/api/endpoints.ts` (since AZ-486 / F7). + ## Module map | Module | Layer | Responsibility | |---|---|---| | `classColors.ts` | leaf | (already documented separately) Class-number → colour + photoMode-suffix lookup. | -| `MediaList.tsx` | sub-component | Left panel media browser. Owns `media[]` state, debounced filter, dropzone upload, blob: local-mode fallback when backend POST fails. Calls `GET /api/annotations/media`, `DELETE /api/annotations/media/{id}`, `POST /api/annotations/media/batch`. | -| `VideoPlayer.tsx` | sub-component | Native `