mirror of
https://github.com/azaion/ui.git
synced 2026-06-21 11:31:11 +00:00
[AZ-486] F7 endpoint builders + STC-ARCH-02 (cycle 1 close)
Single source of truth for every /api/<service>/... URL the UI talks to: src/api/endpoints.ts (25 typed builders) re-exported via the F4 barrel. Migrates 13 production callsites in admin / annotations / flights / settings / dataset / auth / api-client / FlightContext / DetectionClasses to endpoints.* . Adds the STC-ARCH-02 static gate (--mode=api-literals in scripts/check-arch-imports.mjs, wired into scripts/run-tests.sh) that fails any new hardcoded /api/<service>/ literal in src/ outside endpoints.ts and *.test.tsx? files. Tests: +36 contract assertions in src/api/endpoints.test.ts (every builder, character-identical), +6 STC-ARCH-02 architecture cases in tests/architecture_imports.test.ts (single / double / template literal fail paths, *.test.* exemption, line-comment skip, migrated codebase pass). Fast profile 167 -> 209 PASS / 13 SKIP / 0 FAIL, +42 new, 0 regressions. Static profile 31 / 31 PASS. Closes architecture baseline finding F7. Cycle 1 of Phase B closed. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,100 @@
|
||||
# Batch Report
|
||||
|
||||
**Batch**: 10 (Phase B cycle 1, batch 2 of 2 — cycle close)
|
||||
**Tasks**: AZ-486 (Endpoint builders + STC-ARCH-02)
|
||||
**Date**: 2026-05-11
|
||||
**Cycle**: Phase B feature cycle, Step 10 — Implement
|
||||
**Total complexity**: 5 pts
|
||||
**Epic**: AZ-447 (`01-testability-refactoring`)
|
||||
**Closes**: architecture baseline finding **F7** (`_docs/02_document/architecture_compliance_baseline.md`)
|
||||
**Depends on**: AZ-485 (F4 barrels) — committed in `23746ec`, AC-6 verified barrel re-export
|
||||
|
||||
## Task Results
|
||||
|
||||
| Task | Status | Files Modified / Added | Tests | AC Coverage | Issues |
|
||||
|------|--------|------------------------|-------|-------------|--------|
|
||||
| AZ-486_refactor_endpoint_builders | Done | **2 new files** (`src/api/endpoints.ts` — 25 builders; `src/api/endpoints.test.ts` — 36 contract assertions); **1 barrel update** (`src/api/index.ts` re-exports `endpoints`); **13 production files migrated** (`src/api/client.ts`, `src/auth/AuthContext.tsx`, `src/components/{FlightContext,DetectionClasses}.tsx`, `src/features/{admin/AdminPage,annotations/{AnnotationsPage,AnnotationsSidebar,CanvasEditor,MediaList,VideoPlayer},dataset/DatasetPage,flights/FlightsPage,settings/SettingsPage}.tsx`); **1 static-check script extended** (`scripts/check-arch-imports.mjs` — added `--mode=api-literals` for STC-ARCH-02 alongside existing `--mode=arch-imports` for STC-ARCH-01); **1 runner wired** (`scripts/run-tests.sh` — STC-ARCH-02 row added; STC-ARCH-01 invocation made explicit with `--mode=arch-imports`); **1 test file extended** (`tests/architecture_imports.test.ts` — 6 new STC-ARCH-02 cases covering single-/double-/template-literal fail paths, *.test.* exemption, line-comment skip, and migrated-codebase pass); **1 doc updated** (`_docs/02_document/module-layout.md` — `01_api-transport` Public API now lists `endpoints`; Verification Needed item #3a records F7 resolution + STC-ARCH-02 inventory + exemptions) | 36 new contract tests in `src/api/endpoints.test.ts` + 6 new architecture tests in `tests/architecture_imports.test.ts` = **+42 fast tests**; fast profile re-baselined from 167 → 209 passes (0 regressions); 31/31 static checks PASS including new `STC-ARCH-02` | 7 / 7 ACs covered | None |
|
||||
|
||||
## AC Test Coverage: All 7 ACs covered
|
||||
|
||||
| AC | Where | Profile | Status |
|
||||
|----|-------|---------|--------|
|
||||
| AC-1 — Every current path has a builder; URL strings character-identical | `src/api/endpoints.test.ts` (36 `expect(...).toBe('...')` cases — one per builder × every realistic input shape) | fast | PASS — every URL literal that lived in source before this refactor is reproduced exactly. Parameter interpolation (id strings, query strings, two-segment composites like `flightWaypoint(flightId, waypointId)`) covered explicitly. The test file IS the wire contract per `module-layout.md`'s "code-derived documentation" pattern. |
|
||||
| AC-2 — No `/api/<service>/` literals remain in production | `node scripts/check-arch-imports.mjs --mode=api-literals` (exit 0) over `src/` excluding `endpoints.ts` and `*.test.tsx?`; cross-checked with workspace-wide grep showing only `endpoints.ts` retains the literals | static (`STC-ARCH-02`) | PASS — 0 hits outside the documented exemptions |
|
||||
| AC-3 — Static gate fails on a newly-introduced literal | `tests/architecture_imports.test.ts` — 3 fail-on-synthetic-fixture cases (single-quoted, double-quoted, template-literal) all assert `status != 0` and `stderr` mentions `STC-ARCH-02` + fixture filename | fast | PASS — every quote style trips the gate. Single quote and template literal cases shown in the run log; double-quote case implicitly verified (same regex branch) |
|
||||
| AC-4 — Static gate passes on the migrated codebase | `tests/architecture_imports.test.ts` `AC-4: passes on the migrated codebase` + `STC-ARCH-02` row in the static profile | fast + static | PASS — exit code 0, stderr empty |
|
||||
| AC-5 — Fast profile remains green | `bash scripts/run-tests.sh` (static + fast); fast went 167 / 13 / 0 → 209 / 13 / 0 (+36 endpoint contract + 6 architecture STC-ARCH-02 = +42 new passes) | static + fast | PASS — 0 regressions; only adds new tests |
|
||||
| AC-6 — `endpoints` is re-exported from `src/api/index.ts` (the F4 barrel) | `src/api/endpoints.test.ts` `AC-6: barrel re-export` (`endpoints === endpointsViaBarrel`); 13 production consumers import `endpoints` via `'../api'` or `'../../api'` — verified by STC-ARCH-01 still PASS | fast + static | PASS — same object identity; no deep imports reintroduced |
|
||||
| AC-7 — MSW handlers and e2e stubs continue to match | Pre-existing MSW handlers across the fast suite still intercept correctly (no NEW "intercepted unhandled" errors introduced by the refactor); URL strings character-identical per AC-1; e2e profile not run in this batch (per project's batch-level testing strategy — handed off to Step 11 / Step 16 full run) | fast | PASS — observed MSW unhandled-warning lines under `ConfirmDialog.test.tsx` are pre-existing noise (AuthProvider boot triggers `/api/admin/auth/refresh` which that test file deliberately leaves unhandled; the auth-refresh URL is character-identical to pre-refactor); no new failure modes |
|
||||
|
||||
## Design Decisions
|
||||
|
||||
1. **Single shared static-check script with `--mode` flag, not a second `check-api-literals.mjs`.** Mirrors AZ-485's "single source of truth" decision (batch 9 / Design Decision #1). Both gates walk the same codebase, use the same `IGNORED_DIRS` / `SOURCE_EXT` / `walkSourceFiles` machinery, and skip the same single-line comments. Forking the script would have duplicated the walker and the comment-skip rule in two places, which is exactly the kind of drift STC-ARCH-* gates exist to prevent. The `--mode` flag is a one-line dispatch in `main()`.
|
||||
|
||||
2. **STC-ARCH-02 regex matches all three quote styles** (`'`, `"`, `` ` ``), not just single quotes as the task spec's illustrative `ripgrep "'/api/[a-z-]+/"` suggested. Quote style is not a meaningful difference for "no hardcoded URLs in production" — a developer could regress the gate by switching quote styles otherwise. The regex `[\`'"]/api/[a-z][a-z-]*/` requires the path to be preceded by a string-opener, which avoids false positives on comment text that mentions `/api/<service>/` as documentation. Three fail-on-synthetic test cases (one per quote style) lock this behavior in.
|
||||
|
||||
3. **`*.test.{ts,tsx}` files under `src/` are exempted from STC-ARCH-02.** Tests legitimately assert URL strings — MSW handlers, the `endpoints.test.ts` contract itself, and existing colocated tests (`src/api/client.test.ts`, `src/auth/{AuthContext,ProtectedRoute}.test.tsx`, `src/components/Header.test.tsx`) all reference `/api/...` literals. The exemption is documented in five places: the static-check script's `API_LITERAL_EXEMPT_FILES_RE` comment, the bash wrapper in `run-tests.sh`, `module-layout.md` Verification Needed item #3a, the architecture test (`AC-3: still PASSES when an offending literal lives in a *.test.ts file under src/`), and the in-code header comment at the top of `endpoints.ts`. Same exemption discipline as the AZ-485 F3-pending exemption (5 places).
|
||||
|
||||
4. **Function-form builders everywhere (not constants).** Pinned by the task spec's "Why function form" comment in `endpoints.ts`. Allows parameter interpolation without callsites re-introducing template literals (and re-introducing STC-ARCH-02 violations), keeps tree-shaking per-builder under Vite's production rollup, and lets builders that take a query string own the `?` boundary so the wire contract stays identical (e.g., `endpoints.annotations.dataset(queryString)` returns `` `/api/annotations/dataset?${queryString}` `` — caller passes `params.toString()`, not a literal `?`-prefixed string).
|
||||
|
||||
5. **`endpoints.flights.collection(queryString?)` accepts an optional query string.** Today's callsites are split: `FlightContext` reads `'/api/flights?pageSize=1000'` (GET with query), `FlightsPage` writes `'/api/flights'` (POST without query). One builder with an optional arg keeps the wire-contract surface coherent; passing `undefined` returns the bare path. Validated by two test cases (`flights.collection() without query` and `flights.collection(queryString) appends ?queryString`). Same shape used for `annotations.dataset(queryString)` and `annotations.media(queryString)`.
|
||||
|
||||
6. **`endpoints.annotations.annotationsByMedia(mediaId, pageSize?=1000)` exposes the page-size constant.** Every current callsite uses `pageSize=1000`; the optional arg lets future tuning be a single-file change (per task spec NFR / Maintainability). Two test cases pin both the default and the override path.
|
||||
|
||||
7. **`endpoints.admin.class(id: string | number)` widens the ID type.** `DetectionClass.id` is `number` in the type system today (per `AdminPage` line 51 cast), but the rest of the admin builders take `string` because user/aircraft IDs are GUIDs. Widening to `string | number` at the builder accepts today's number-typed call from `AdminPage.handleDeleteClass` without an explicit cast and stays forward-compatible if the backend later switches `DetectionClass.id` to UUID. Two test cases (`'cls-7'` and `42`) pin both arms.
|
||||
|
||||
8. **`endpoints.detect.media(mediaId)` is the only entry under a non-annotations namespace.** The `/api/detect/<mediaId>` path is a single-segment service path (no further segments today) consumed only by `AnnotationsSidebar`. Keeping it under its own `detect` namespace mirrors the URL's first segment and leaves room for future detect-service endpoints without renaming.
|
||||
|
||||
9. **`src/api/endpoints.ts` lives under `01_api-transport` — F6 explicitly out of scope.** Per the AZ-486 spec's `Excluded` section, the future `src/shared/` move (F6) is deferred. The barrel-re-export pattern means consumers import `{ endpoints } from '../api'` — when F6 lands and moves the file, only `src/api/index.ts` needs to flip the re-export source; consumers do not change. This is exactly the protection F4 / AZ-485 was built to provide.
|
||||
|
||||
## Code Review Verdict: PASS
|
||||
|
||||
Self-review (implement skill Step 9 / 10) applied to the 2 new + 13 modified + 1 script-extended + 1 runner-wired + 1 doc-updated + 1 test-extended files:
|
||||
|
||||
- **0 Critical, 0 High, 0 Medium, 0 Low findings.**
|
||||
- **Scope discipline**: every modified file is one of (contract author, deep-literal consumer, static-check author, runner wirer, contract documentor, gate test author). The spec's listed files are all migrated; two additional files outside the spec's explicit list (`CanvasEditor.tsx`, `VideoPlayer.tsx`) were also migrated because they contain `/api/<service>/` literals as `<img src>` / `<video src>` URLs — including them is required for AC-2 / STC-ARCH-02 to pass, and the spec's "every component that calls `api.*` or `createSSE`" intent reads "every UI callsite of a wire-contract URL", which these are.
|
||||
- **No silent error suppression**: `check-arch-imports.mjs` writes the full hit list to stderr (with `STC-ARCH-02` named in the header) before exiting non-zero; the bash delegate (`static_check_no_api_literals`) propagates the exit code; `run-tests.sh` records the result into the static CSV. No new `try { } catch { }` blocks in production code; no new `2>/dev/null` redirects.
|
||||
- **Single-responsibility**: `endpoints.ts` exports one shape (the typed URL-builder object) with one job (produce wire-contract URLs). `endpoints.test.ts` has one job (pin every URL string). `check-arch-imports.mjs` now has two modes but each scanner function (`scanArchImports`, `scanApiLiterals`) has one job. The `main()` dispatch is a 12-line config-and-call.
|
||||
- **No new dependencies**: `endpoints.ts` is plain TypeScript with `as const`. `endpoints.test.ts` uses Vitest + the barrel import. The script extension uses Node stdlib (`fs`, `path`, `url`) only — same as before.
|
||||
- **Architecture compliance (Phase 7)**: STC-ARCH-01 still PASS (no new cross-component deep imports); the new `endpoints` import in `client.ts` is intra-component (`./endpoints`). The 12 modified consumer files all import `endpoints` via the `01_api-transport` barrel. Layer direction unchanged.
|
||||
- **Cross-task consistency (Phase 6)**: builds on AZ-485 cleanly — uses the same barrel pattern (`module-layout.md` Rule #3) and the same static-check delivery mechanism (`scripts/check-arch-imports.mjs`). The shared script now has symmetric STC-ARCH-01 + STC-ARCH-02 modes. AZ-447 epic now has both F4 and F7 closed.
|
||||
- **Performance**: `STC-PERF01` (initial JS bundle ≤ 2 MB gzipped) still PASS after the refactor. The `endpoints` object is small and tree-shakeable per builder per the Vite production rollup; observed bundle size unchanged within measurement noise.
|
||||
|
||||
## Auto-Fix Attempts: 0
|
||||
|
||||
The session resumed an in-progress AZ-486 batch (the user-recommended "option A" path from `/autodev` re-entry). No auto-fix loop was needed — the missing pieces (DatasetPage + DetectionClasses migrations, STC-ARCH-02 wiring, architecture test extension, doc update) were straightforward additions to a coherent partial implementation. The first `bash scripts/run-tests.sh` run went green: 31/31 static + 209/13/0 fast.
|
||||
|
||||
## Stuck Agents: None
|
||||
|
||||
No multi-pass investigations. The resume was a continuation, not a debug loop.
|
||||
|
||||
## Test Run Summary
|
||||
|
||||
- `bash scripts/run-tests.sh` (static + fast) — exit 0
|
||||
- **Static profile**: 31 / 31 PASS, including `STC-ARCH-01` (166 ms) and the new `STC-ARCH-02` (179 ms). `STC-T1` (tsc) 3.8 s, `STC-B1` (vite build) 7.4 s.
|
||||
- **Fast profile**: `bun run test:fast` — 28 files / **209 passed** / 13 skipped / 22.58 s wall. +42 vs end-of-batch-9 (167 + 36 endpoint contract + 6 architecture STC-ARCH-02 = 209). 0 regressions.
|
||||
- `node scripts/check-arch-imports.mjs --mode=api-literals` (direct invocation) — exit 0, stderr empty.
|
||||
- `node scripts/check-arch-imports.mjs --mode=arch-imports` (direct invocation) — exit 0, stderr empty.
|
||||
- `ReadLints` — clean on all modified files.
|
||||
- Pre-existing MSW "intercepted unhandled" stderr lines under `ConfirmDialog.test.tsx` are NOT new and NOT caused by this batch: the failing URL `/api/admin/auth/refresh` is character-identical pre- and post-refactor (AC-1 verifies); the warning has been latent in the suite and is not a blocker.
|
||||
|
||||
## Documented Drifts (cumulative across batch)
|
||||
|
||||
None new. The F3-pending exemption (`classColors`) carried forward from batch 9 is unchanged. STC-ARCH-02 has no F3 interaction.
|
||||
|
||||
## Phase B Cycle 1 Status
|
||||
|
||||
This is **batch 2 of 2** in Phase B cycle 1 (the cycle covered baseline findings **F4** + **F7** under epic AZ-447). Both batches are now complete:
|
||||
|
||||
- Batch 9 / AZ-485 — F4 (Public API barrels + STC-ARCH-01) — committed `23746ec`
|
||||
- Batch 10 / AZ-486 — F7 (Endpoint builders + STC-ARCH-02) — this batch, uncommitted pending user approval
|
||||
|
||||
**Cycle 1 complete** once batch 10 is committed. Per the autodev existing-code flow, Step 10 (Implement) then auto-chains to Step 11 (Run Tests) → Step 12 (Test-Spec Sync) → Step 13 (Update Docs) → Step 14 (Security Audit, optional) → Step 15 (Performance Test, optional) → Step 16 (Deploy) → Step 17 (Retrospective) → loop back to Step 9 with `cycle: 2` incremented.
|
||||
|
||||
## Cumulative Code Review Trigger
|
||||
|
||||
Per the implement skill Step 14.5, cumulative code review fires every `K=3` batches. Phase B cycle 1 had only 2 batches (AZ-485, AZ-486); no cumulative review is triggered at this cycle close. The existing `cumulative_review_batches_07-08_cycle1_report.md` was the Phase A wrap. The next cumulative review will be after batches 11 + 12 + 13 of cycle 2 (or whenever the next 3-batch window closes).
|
||||
|
||||
## Next Batch
|
||||
|
||||
**All Phase B cycle 1 tasks complete.** Final implementation report for cycle 1 will be `_docs/03_implementation/implementation_report_phase_b_cycle1.md` (written at the close of Step 10 once user approves commit of batch 10). The autodev orchestrator will auto-chain to Step 11 (Run Tests, full suite, owned by `test-run/SKILL.md`) after commit.
|
||||
Reference in New Issue
Block a user