ui/_docs/03_implementation/reviews/batch_08_review.md

Code Review Report

Batch: 8 — AZ-474, AZ-480 (final batch of Phase A) Date: 2026-05-11 Verdict: PASS Mode: Full (per-batch invocation by /implement)

Inputs

• Task specs:
  • _docs/02_tasks/todo/AZ-474_test_tile_split_zoom.md (6 ACs, 3 pts)
  • _docs/02_tasks/todo/AZ-480_test_prod_image_nginx_ram.md (5 ACs, 3 pts)
• Changed files (4 total, all under Blackbox Tests OWNED scope):
  • tests/tile_split_zoom.test.tsx
  • e2e/tests/tile_split_zoom.e2e.ts
  • e2e/tests/prod_image_nginx_ram.e2e.ts
  • scripts/run-tests.sh (4 new functions: static_check_nginx_body_cap, static_check_dockerfile_nginx_alpine, static_check_nginx_route_count, static_check_nginx_prefix_strip + 4 new run_static rows: STC-RES02, STC-RES03, STC-RES09, STC-RES10)

Findings

#	Severity	Category	File:Line	Title
—	—	—	—	None

No Critical, High, Medium, or Low findings.

Phase Walkthrough

Phase 1 — Context Loading

Both task specs read; ACs catalogued; module-layout.md consulted for OWNED / READ-ONLY / FORBIDDEN envelopes. Every changed file lives under tests/**, e2e/**, or scripts/run-tests.sh — the OWNED scope of the Blackbox Tests cross-cutting component (epic AZ-455). No production-source file under src/**, no src/** configuration, no nginx.conf, and no Dockerfile were touched. nginx.conf and Dockerfile are READ-ONLY for this batch (their contents are the system under test for AZ-480).

Phase 2 — Spec Compliance

Task	AC	Test	Today	Drift documented
AZ-474	AC-1 (FT-P-51 [Q] tile-split endpoint contract)	tests/tile_split_zoom.test.tsx + e2e/tests/tile_split_zoom.e2e.ts	it.fails() (fast) + test.fail (e2e) + control PASS	drift — split surface is QUARANTINED today (no Split tile button, no POST callsite to /api/annotations/dataset/<id>/split); per _docs/04_refactoring/01-testability-refactoring/deferred_to_refactor.md D11
AZ-474	AC-2 (FT-P-52 YOLO parser happy path)	tests/tile_split_zoom.test.tsx	it.fails() + control PASS	drift — no parser module exists; splitTile is fetched but not consumed
AZ-474	AC-3 (FT-P-53 isSplit honored on dataset list)	tests/tile_split_zoom.test.tsx + e2e/tests/tile_split_zoom.e2e.ts	it.fails() (fast) + test.fail (e2e) + control PASS	drift — DatasetItem.isSplit is read from the network shape but never consumed by the renderer (only isSeed drives the red-ring affordance today)
AZ-474	AC-4 (FT-P-54 auto-zoom viewport)	tests/tile_split_zoom.test.tsx	it.fails() + control PASS	drift — no <TileViewer> component; no data-viewport-rect testid mounted
AZ-474	AC-5 (FT-P-55 indicator visibility)	tests/tile_split_zoom.test.tsx	it.fails() + control PASS	drift — no role="status" indicator with a `tile
AZ-474	AC-6 (FT-N-10 malformed YOLO label → user-visible error)	tests/tile_split_zoom.test.tsx	it.fails() (drift) + 2 control PASSes (page does not crash; alert() is never called)	drift — malformed splitTile is silently ignored today; once parser + alert wire up, the in-DOM role="alert" lights up
AZ-480	AC-1 (NFT-RES-LIM-02 nginx 500M cap)	scripts/run-tests.sh static_check_nginx_body_cap (STC-RES02)	PASS — exactly 1 client_max_body_size 500M directive in nginx.conf	—
AZ-480	AC-2 (NFT-RES-LIM-03 nginx:alpine, no Node)	scripts/run-tests.sh static_check_dockerfile_nginx_alpine (STC-RES03) + e2e/tests/prod_image_nginx_ram.e2e.ts	PASS (static — final stage FROM nginx:alpine); e2e gated by docker availability + image existence	—
AZ-480	AC-3 (NFT-RES-LIM-08 steady-state RAM ≤ 200 MB)	e2e/tests/prod_image_nginx_ram.e2e.ts	gated — RUN_LONG_RUNNING=1 + docker availability; samples docker stats every 10 s for 5 min and asserts peak ≤ 200 MB	—
AZ-480	AC-4 (NFT-RES-LIM-09 9 nginx routes)	scripts/run-tests.sh static_check_nginx_route_count (STC-RES09)	PASS — exactly 9 ^\s*location\s+/api/ matches	—
AZ-480	AC-5 (NFT-RES-LIM-10 prefix-strip)	scripts/run-tests.sh static_check_nginx_prefix_strip (STC-RES10) + e2e/tests/prod_image_nginx_ram.e2e.ts	PASS (static — every /api/* location has a proxy_pass http://...:<port>/ with the trailing slash, which is nginx's canonical prefix-strip idiom); e2e probes the running nginx via /api/annotations/health	—

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

• 1 fast file / 2 e2e files / 1 static-runner edit / 0 production-source files modified.
• Total fast tests added: 13 (AZ-474). Five it.fails() (one per AC-1..5) + one it.fails() for AC-6 + 8 control PASSes (one per AC + a no-alert() defence-in-depth control).
• Total e2e tests added: 5 across 2 files.
  • e2e/tests/tile_split_zoom.e2e.ts — 2 test.fail companions for FT-P-51 and FT-P-53 (the only fast + e2e rows in AZ-474).
  • e2e/tests/prod_image_nginx_ram.e2e.ts — 3 tests: AC-2 docker probe (no Node), AC-5 prefix-strip runtime, AC-3 long-running RAM soak (gated).
• 4 new static checks added (STC-RES02, STC-RES03, STC-RES09, STC-RES10); the existing STC-S5 mission-planner exclusion and STC-PERF01 bundle-size gate are unaffected.
• 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.
• 0 files added to _docs/ — no new lessons surfaced from this batch (the URL-stub lesson from AZ-476 remains the only entry; this batch did not hit a similar trap).
• The tests/setup.ts MSW boundary (onUnhandledRequest: 'error') is preserved. tests/tile_split_zoom.test.tsx adds two narrowly-scoped beforeEach handlers (/api/admin/auth/refresh → 401 and /api/annotations/settings/user → 404) so the AuthProvider + FlightProvider mounts complete without leaking unhandled-request errors. The FlightProvider user-settings 404 is the right shape for an unauthenticated/missing settings response — the page renders defensively against it.
• The new static checks delegate to node (via node -e) for the AC-5 prefix-strip parser. The node runtime is already a hard dep of the static profile (used by check-banned-deps.mjs, check-i18n-coverage.mjs, check-ci-image-labels.mjs), so the new check inherits the same posture — no new toolchain.
• The e2e prod-image companion uses the host docker socket for which node and docker stats. The test skips with a clear reason if docker is unreachable or the ${IMAGE} (default azaion/ui:test) is not built; it never silently passes on a runner that cannot probe the contract.

Phase 5 — Static + Lint

• bun run test:fast — 26 files / 163 passed / 13 skipped / 16.38 s wall.
• ./scripts/run-tests.sh --static-only — 29 / 29 static checks PASS / 12.95 s wall (added STC-RES02 / STC-RES03 / STC-RES09 / STC-RES10; no other regressions).
• ReadLints clean on all 4 changed files.
• tsc --noEmit -p tsconfig.test.json succeeded as part of STC-T1.
• Standalone bunx tsc --noEmit against the 2 new e2e files (out-of-tree of tsconfig.test.json) — 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 (vi.spyOn(window, 'alert'), seedBearer/clearBearer, MSW handler resets in afterEach).
• The AC-6 malformed-label test installs a focused vi.spyOn(window, 'alert') to enforce NFT-SEC-07 (alert() is never called in the dataset double-click path) AND a separate control test that asserts the same defence-in-depth fact directly. Both pass today; both stay PASS after the in-DOM role="alert" lands.
• The DatasetPage tests do NOT depend on the editor tab actually rendering CanvasEditor for the malformed annotation — the assertion is on the dataset list shape (no role="alert") + the no-alert() spy. JSDOM's missing getContext shows up as a stderr noise from CanvasEditor's draw effect when the editor tab mounts; it does not affect the AC-6 assertions because they target the dataset card surface, not the canvas itself.
• The new static checks are deliberate single-responsibility shell functions. static_check_nginx_prefix_strip uses node -e rather than awk/sed because the conditional "proxy_pass with trailing slash OR rewrite" logic is much clearer in JS; the threshold (every /api/* block has at least one of the two patterns within its block-scope) is explicit in the script.
• The e2e prod-image test uses docker run -d --rm -p 0:80 ${IMAGE} so the container picks an ephemeral port — the test does not require port 80 to be free on the runner. The 0:80 form was chosen explicitly (not --network host) so the test composes cleanly inside CI runners that may already have other services bound to common ports.

Phase 7 — Architecture Compliance

• No layer-direction violations. Tests are leaves of the import graph; the new static checks are shell + node and live entirely in scripts/run-tests.sh.
• No new cyclic dependencies (verified via tsc --noEmit and bun run build in the static profile).
• src/features/dataset/DatasetPage.tsx, src/types/index.ts, nginx.conf, and Dockerfile are all exercised but not modified.
• New static checks (STC-RES02, STC-RES03, STC-RES09, STC-RES10) run at the same point in the runner as the other config-static checks; ordering is: type-check (STC-T1) → vite build (STC-B1) → dist scans (STC-S5, STC-PERF01) → nginx/image scans (new) → no-OWM-key-in-dist (STC-SEC1B). The nginx/image scans do not require dist/; they could run earlier, but grouping them after the build keeps the static profile's "first half: source / config; second half: artefact" structure intact.
• 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 the final two blackbox-test tasks (11 ACs total) with zero production-code edits, every drift paired with a runnable control test, full static + fast suite green, and four new commit-time static gates (STC-RES02, STC-RES03, STC-RES09, STC-RES10) covering the production image / nginx routing surface.