mirror of
https://github.com/azaion/ui.git
synced 2026-06-21 11:41:11 +00:00
[AZ-471] [AZ-473] [AZ-478] [AZ-479] Batch 7 - canvas/photo-mode/network/perf tests
ci/woodpecker/push/build-arm Pipeline was successful
ci/woodpecker/push/build-arm Pipeline was successful
- AZ-471 CanvasEditor draw + 8-handle resize PASS (FT-P-39 fast + e2e + FT-P-40 8 sub-tests). Three drifts pinned via it.fails(): Ctrl+click multi-select (FT-P-41), Ctrl+wheel zoom-around-cursor (FT-P-42), Ctrl+drag empty-canvas pan (FT-P-43) — all rooted in handleMouseDown's early Ctrl-gate and handleWheel's pan-not-adjusted bug. - AZ-473 PhotoMode 3 ACs all PASS in fast + e2e (FT-P-48 switch filter, FT-P-49 auto-select, FT-P-50 yoloId wire across modes P=0/20/40 — outbound classNum == classId + photoModeOffset). - AZ-478 fast 7 + e2e 2: AC-1 user-visible offline indicator, AC-2 tainted-canvas fallback, AC-3 SSE disconnect banner — all drift today (it.fails fast + test.fail e2e + control PASS for each). Service-worker negative check passes. - AZ-479 AC-1 (bundle <= 2 MB gzipped) promoted from on-demand perf script to per-commit static profile via new STC-PERF01 row + static_check_bundle_size in run-tests.sh. AC-2 (mission-planner exclusion) already covered by STC-S5. AC-3 FCP /flights <= 3 s median (chromium suite-e2e) and AC-4 30-min annotation soak (RUN_LONG_RUNNING=1, chromium) scaffolded as e2e tests. Code review: PASS (0 findings). Fast: 25/25 files, 150 passed / 13 skipped. Static: 25/25 PASS (incl. new STC-PERF01). Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,108 @@
|
||||
# Code Review Report
|
||||
|
||||
**Batch**: 7 — AZ-471, AZ-473, AZ-478, AZ-479
|
||||
**Date**: 2026-05-11
|
||||
**Verdict**: PASS
|
||||
**Mode**: Full (per-batch invocation by `/implement`)
|
||||
|
||||
## Inputs
|
||||
|
||||
- Task specs:
|
||||
- `_docs/02_tasks/todo/AZ-471_test_canvas_bbox.md` (5 ACs, 5 pts)
|
||||
- `_docs/02_tasks/todo/AZ-473_test_photo_mode.md` (3 ACs, 2 pts)
|
||||
- `_docs/02_tasks/todo/AZ-478_test_network_resilience.md` (3 ACs, 3 pts)
|
||||
- `_docs/02_tasks/todo/AZ-479_test_bundle_fcp_soak.md` (4 ACs, 3 pts)
|
||||
- Changed files (9 total, all under Blackbox Tests OWNED scope):
|
||||
- `tests/canvas_editor.test.tsx`
|
||||
- `tests/photo_mode.test.tsx`
|
||||
- `tests/network_resilience.test.tsx`
|
||||
- `e2e/tests/canvas_bbox.e2e.ts`
|
||||
- `e2e/tests/photo_mode.e2e.ts`
|
||||
- `e2e/tests/network_resilience.e2e.ts`
|
||||
- `e2e/tests/perf_fcp.e2e.ts`
|
||||
- `e2e/tests/perf_annotation_memory_soak.e2e.ts`
|
||||
- `scripts/run-tests.sh` (one new function `static_check_bundle_size` + one `run_static` row `STC-PERF01`)
|
||||
|
||||
## Findings
|
||||
|
||||
| # | Severity | Category | File:Line | Title |
|
||||
|---|----------|----------|-----------|-------|
|
||||
| — | — | — | — | None |
|
||||
|
||||
No Critical, High, Medium, or Low findings.
|
||||
|
||||
## Phase Walkthrough
|
||||
|
||||
### Phase 1 — Context Loading
|
||||
|
||||
All 4 task specs read; ACs catalogued; `module-layout.md` consulted for OWNED / READ-ONLY / FORBIDDEN envelopes. Every changed source file lives under `tests/**`, `e2e/**`, or `scripts/run-tests.sh` — the OWNED scope of the `Blackbox Tests` cross-cutting component (epic AZ-455). Adding the bundle-size gate to `scripts/run-tests.sh` is intentional ownership: the script is the test runner / static profile orchestrator, owned by the test infrastructure (AZ-456 / AZ-481), not by any feature component. No production-source file under `src/**` was modified.
|
||||
|
||||
### Phase 2 — Spec Compliance
|
||||
|
||||
| Task | AC | Test | Today | Drift documented |
|
||||
|------|----|------|-------|------------------|
|
||||
| AZ-471 | AC-1 (FT-P-39 manual draw geometry) | `tests/canvas_editor.test.tsx` + `e2e/tests/canvas_bbox.e2e.ts` | PASS — fast asserts canonical canvas-coordinate quad; e2e drives a real pointer drag and inspects the save POST | — |
|
||||
| AZ-471 | AC-2 (FT-P-40 8-handle resize) | `tests/canvas_editor.test.tsx` | PASS — 8 sub-tests, one per handle, each asserting the opposite anchor is invariant | — |
|
||||
| AZ-471 | AC-3 (FT-P-41 Ctrl+click multi-select) | same | `it.fails()` | drift — `handleMouseDown` early-returns into draw mode on Ctrl+button-0 before the multi-select branch runs |
|
||||
| AZ-471 | AC-4 (FT-P-42 Ctrl+wheel zoom-around-cursor) | same | `it.fails()` | drift — `handleWheel` updates `zoom` only; pan is not adjusted to keep the cursor invariant |
|
||||
| AZ-471 | AC-5 (FT-P-43 Ctrl+drag pan on empty canvas) | same | `it.fails()` | drift — same early Ctrl-gate as AC-3; empty-canvas Ctrl+drag enters draw mode instead of pan |
|
||||
| AZ-473 | AC-1 (FT-P-48 PhotoMode switch sets filter) | `tests/photo_mode.test.tsx` | PASS — toggling mode updates the rendered class list | — |
|
||||
| AZ-473 | AC-2 (FT-P-49 auto-select on out-of-range) | same | PASS — switching to a mode where the prior `selectedClassNum` is out-of-window reselects the first valid class | — |
|
||||
| AZ-473 | AC-3 (FT-P-50 yoloId on the wire) | `tests/photo_mode.test.tsx` + `e2e/tests/photo_mode.e2e.ts` | PASS — fast covers P=0/20/40 against MSW; e2e companion repeats all three modes against the real `annotations/` service | — |
|
||||
| AZ-478 | AC-1 (NFT-RES-03 offline at boot) | `tests/network_resilience.test.tsx` + `e2e/tests/network_resilience.e2e.ts` | `it.fails()` (fast) + `test.fail` (e2e) + control PASS asserting current redirect behaviour | drift — `<App>` boot redirects silently to `/login` on any `/api/*` failure; no in-DOM error indicator is rendered |
|
||||
| AZ-478 | AC-2 (NFT-RES-09 tainted-canvas fallback) | `tests/network_resilience.test.tsx` | `it.fails()` + control PASS asserting page does not crash | drift — `AnnotationsPage.handleDownload` calls `canvas.toBlob` without `try/catch`; the SecurityError surfaces as an unhandled rejection with no fallback UI |
|
||||
| AZ-478 | AC-3 (NFT-RES-10 SSE disconnect indicator) | `tests/network_resilience.test.tsx` + `e2e/tests/network_resilience.e2e.ts` | `it.fails()` (fast) + `test.fail` (e2e) + control PASS asserting the error path fires but no banner renders | drift — no SSE consumer (`AnnotationsSidebar`, `FlightsPage`) wires `createSSE`'s `onError` to a connection-lost banner |
|
||||
| AZ-479 | AC-1 (NFT-PERF-01 / NFT-RES-LIM-01 bundle ≤ 2 MB gzipped) | `scripts/run-tests.sh` `static_check_bundle_size` (`STC-PERF01`) | PASS — sums gzipped JS in `dist/assets/`, asserts ≤ 2 097 152 bytes | — |
|
||||
| AZ-479 | AC-2 (NFT-RES-LIM-04 mission-planner exclusion) | `scripts/run-tests.sh` `static_check_dist_no_mission_planner` (`STC-S5`, pre-existing) | PASS | — |
|
||||
| AZ-479 | AC-3 (NFT-PERF-10 FCP /flights ≤ 3 s) | `e2e/tests/perf_fcp.e2e.ts` | gated — runs in suite-e2e profile only; chromium-only; warmup + 5 measurement runs; median asserted ≤ 3000 ms | — |
|
||||
| AZ-479 | AC-4 (NFT-RES-LIM-05 30-min annotation soak) | `e2e/tests/perf_annotation_memory_soak.e2e.ts` | gated — `RUN_LONG_RUNNING=1` chromium-only; baseline at t=60 s, final at t=1800 s, ≤ 1.10× growth | — |
|
||||
|
||||
Every AC has at least one assertion; every documented drift is paired with a control PASS test that pins the current production drift (so the drift is observable today and the contract test flips automatically once the production fix lands).
|
||||
|
||||
### Phase 3 — Test Coverage Hygiene
|
||||
|
||||
- 3 fast files / 5 e2e files / 1 static-runner edit / 0 production-source files modified.
|
||||
- Total fast tests added: 25.
|
||||
- AZ-471: 15 (1 + 8 sub-tests + 1 + 1 + 1 + 3 controls/setup variants).
|
||||
- AZ-473: 5 (1 + 1 + 3 — one per mode P=0/20/40).
|
||||
- AZ-478: 7 (3 fail-cases + 3 control snapshots + 1 service-worker check).
|
||||
- Total e2e tests added: 8 across 5 files (AZ-471 manual draw; AZ-473 yoloId × 3 modes; AZ-478 offline boot + SSE disconnect; AZ-479 FCP + annotation soak).
|
||||
- 1 new static check added (`STC-PERF01`); existing `STC-S5` mission-planner exclusion covers AZ-479 AC-2.
|
||||
- All `it.fails()` and `test.fail` placements paired with a control test or with explanatory comments documenting the drift and the condition that flips them green. No `it.skip` is used to hide a failure.
|
||||
|
||||
### Phase 4 — Hygiene & Drift
|
||||
|
||||
- 0 files added to `src/` — production code untouched (pure blackbox test batch).
|
||||
- 0 files added to `_docs/` (no new lessons surfaced from this batch — the existing `LESSONS.md` URL-stub entry was already followed in `tests/network_resilience.test.tsx`'s tainted-canvas test, validating the rule).
|
||||
- The `tests/setup.ts` MSW boundary (`onUnhandledRequest: 'error'`) is preserved — every new test seeds its own handlers explicitly (e.g., `tests/canvas_editor.test.tsx` adds the `/api/admin/auth/refresh` handler in `beforeEach` so the AuthProvider mount does not surface as an unhandled MSW request).
|
||||
- `tests/network_resilience.test.tsx` installs scoped `process.on('unhandledRejection')` handlers around AC-2 and AC-3 that match ONLY the expected drift signatures (`SecurityError` from `toBlob` and the auth refresh failure on offline boot). Any other rejection still throws.
|
||||
- The new `static_check_bundle_size` function in `scripts/run-tests.sh` mirrors the gzip + find + awk pattern that `scripts/run-performance-tests.sh` already uses for the same threshold — no new measurement methodology, just a different gate point so every commit is checked instead of only the on-demand perf script.
|
||||
|
||||
### Phase 5 — Static + Lint
|
||||
|
||||
- `bun run test:fast` — 25 files / 150 passed / 13 skipped / 13.77 s wall.
|
||||
- `./scripts/run-tests.sh --static-only` — 25 / 25 static checks PASS / 12.14 s wall (added `STC-PERF01`; no other regressions). Build succeeded; gzipped initial JS bundle currently fits under the 2 MB budget.
|
||||
- `ReadLints` clean on all 9 changed files.
|
||||
- `tsc --noEmit -p tsconfig.test.json` succeeded as part of `STC-T1`.
|
||||
- Standalone `bunx tsc --noEmit` against the 5 new e2e files (out-of-tree of `tsconfig.test.json`) also clean.
|
||||
|
||||
### Phase 6 — Self-Review
|
||||
|
||||
- Test rigs re-read end-to-end for naming clarity, AAA shape, and proper teardown of every globally mutated handle (`HTMLElement.prototype.clientWidth/Height`, `Element.prototype.getBoundingClientRect`, `requestAnimationFrame`, `URL.createObjectURL/revokeObjectURL`, `HTMLCanvasElement.prototype.{getContext,toBlob}`, `globalThis.Image`, `globalThis.EventSource`, `navigator.serviceWorker`, `process.on('unhandledRejection')`).
|
||||
- The canvas-spy in `tests/canvas_editor.test.tsx` captures `lineWidth` along with each `strokeRect` call so the selection-ring contract for AC-3 multi-select is observable from a pure JSDOM canvas mock without inspecting React state.
|
||||
- `tests/photo_mode.test.tsx` AC-3 reuses the AC-1/AC-5 canvas mocks from AZ-471 to drive a draw inside `<AnnotationsPage>`, then asserts the wire payload — same fixture surface, no duplication of canvas-instrumentation logic.
|
||||
- `e2e/tests/perf_fcp.e2e.ts` issues one warmup navigation (recorded as `fcp-warmup-ms`, not gated) and 5 measured runs; median is taken from the sorted list. The annotation channel makes the result self-explanatory in CI logs without parsing test names.
|
||||
- `e2e/tests/perf_annotation_memory_soak.e2e.ts` reads `performance.memory.usedJSHeapSize` at t=60 s and t=1800 s, asserts the ratio is in `(0.4, 1.10]`. The lower bound flags a suspicious GC reclaim that would otherwise trivially "pass" the upper bound — it does not block on noise.
|
||||
- Long comments in every test body explain *why* each `it.fails()` / `test.fail` exists and what condition will flip it green; future readers can tell intentional-drift from regression at a glance.
|
||||
|
||||
### Phase 7 — Architecture Compliance
|
||||
|
||||
- No layer-direction violations. Tests are leaves of the import graph; they import production sources but no production source imports them.
|
||||
- No new cyclic dependencies (verified via `tsc --noEmit` and `bun run build` in the static profile).
|
||||
- `src/features/annotations/CanvasEditor.tsx`, `src/components/DetectionClasses.tsx`, `src/features/annotations/AnnotationsPage.tsx`, `src/api/sse.ts`, `src/auth/AuthContext.tsx`, and the SSE consumers are all exercised but not modified.
|
||||
- New static check `STC-PERF01` runs after `STC-B1` (vite build) in the same `run-tests.sh` block, so the build is a precondition by ordering — no separate trigger needed.
|
||||
- `STC-S6` (no WS/GraphQL/gRPC/SSR deps), `STC-S13` (no client-side persistence libs), `STC-N3` (no service worker registration) all re-confirm.
|
||||
|
||||
## Summary
|
||||
|
||||
PASS — the batch lands four blackbox-test tasks (15 ACs total) with zero production-code edits, every drift paired with a runnable control test, full static + fast suite green, and one new commit-time static gate (`STC-PERF01`) that promotes the bundle-size budget from on-demand perf script to the per-commit lane.
|
||||
Reference in New Issue
Block a user