# Review Report: Material Alias Completeness with Blocking Dialog
Date: 2026-03-14

## Result: ✅ Approved

## Checklist Results

### Backend / Python
- [x] New endpoints have role checks: `check_materials` uses `get_current_user`, `batch_create_aliases` uses `require_admin_or_pm`
- [x] No SQL injections — all ORM usage
- [x] Pydantic input validation: `BatchAliasCreate` with `BatchAliasMapping` for POST body
- [x] Invalid IDs return 404: order not found → 404, material not found → 404
- [x] No new routers — endpoints added to existing `orders` and `materials` routers
- [x] No new models — uses existing `Material`, `MaterialAlias`
- [x] Async consistency: all handlers are `async def`
- [x] No `print()` in production code
- [x] No hardcoded paths
- [x] `storage_key` not touched

### Database
- [x] No migration needed — uses existing tables

### Frontend / TypeScript
- [x] New API interfaces in `frontend/src/api/materials.ts`: `MaterialSuggestion`, `UnmappedMaterial`, `UnmappedMaterialCheck`
- [x] No `as any` for API responses — correct types throughout
- [x] CSS vars use inline style where needed (`style={{ backgroundColor: 'var(--color-bg-surface)' }}`)
- [x] Loading states: `checkingMaterials` for button, `saving` in dialog, `quickMapMut.isPending`
- [x] Error feedback: error state in dialog, toast on quick-map success/failure
- [x] No new role-dependent UI elements (dispatch button already gated by `canDispatch`)
- [x] TypeScript compiles clean (`tsc --noEmit` passes)

### Render Pipeline
- [x] Material alias lookup order preserved: aliases FIRST, then exact name
- [x] `find_unmapped_materials()` follows same resolution logic as `resolve_material_map()`

### Security
- [x] No credentials in code
- [x] No hardcoded tokens
- [x] English variable names and comments

## Minor Notes (non-blocking)

### `UnmappedMaterialsDialog.tsx:56` — `bg-amber-500/10` opacity syntax
Uses Tailwind opacity on `bg-amber-500/10` and `border-amber-500/30`. This is fine because `amber-500` is a static Tailwind color (not a CSS variable), so the `/opacity` syntax works correctly here. No issue.

### `UnmappedMaterialsDialog.tsx:99` — `.sort()` mutates in render
The `libraryMaterials.sort(...)` inside JSX mutates the filtered array on every render. Functionally harmless since `libraryMaterials` is a fresh array from `.filter()`, but a `[...libraryMaterials].sort(...)` or `useMemo` would be cleaner. Non-blocking.

### `dispatch_single_line_render` endpoint in orders.py
This endpoint (and its frontend `dispatchLineRender` function + `dispatchLineMut` usage in `OrderLineRow`) appeared in the diff but is not part of the material alias plan. It's a separate per-line dispatch feature. It looks correct: proper auth (`require_admin_or_pm`), 404 checks, status validation, same dispatch pattern as the bulk dispatch endpoint.

## Positives
- Clean separation: service function (`find_unmapped_materials`) is reusable and follows the same resolution logic as `resolve_material_map()`
- Deduplication in material name collection (`seen` set, case-insensitive)
- Graceful fallback: if `checkOrderMaterials` fails, dispatch proceeds anyway (no blocking on network errors)
- Suggestions via `SequenceMatcher` give useful context without external dependencies
- Quick-map on Materials page provides a second entry point for the same workflow
- Batch alias endpoint reuses existing `MaterialAlias` model — no schema changes needed

## Recommendation
Approved — ready to commit.