Files
HartOMat/review-report.md
T
Hartmut bfd58e3419 fix: media thumbnails, product dimensions, inline 3D viewer, GLB export
Bug A: Media Library thumbnails were gray because <img src> cannot send
JWT auth headers. Added useAuthBlob() hook (fetch + createObjectURL) in
MediaBrowser.tsx. Also fixed publish_asset Celery task to populate
product_id + cad_file_id on MediaAsset for thumbnail fallback resolution.

Bug B: Product dimensions now shown in Product Details card with Ruler
icon and "from CAD" label when cad_mesh_attributes.dimensions_mm exists.

Bug C: Replaced 128×128 CAD thumbnail with InlineCadViewer component.
Queries gltf_geometry MediaAssets, fetches GLB via auth fetch → blob URL
→ Three.js Canvas with OrbitControls. Falls back to thumbnail + "Load 3D
Model" button. Polling when GLB generation is in progress.

Bug D: trimesh was in [cad] optional extra but Dockerfile only installed
[dev]. Changed to pip install -e ".[dev,cad]" — trimesh now available in
backend container, GLB + Colors export works.

Also added bbox extraction (STL-first numpy parsing) in render_step_thumbnail
and admin "Re-extract CAD Metadata" bulk endpoint.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-07 13:27:46 +01:00

56 lines
4.1 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Review Report: Phase V2-Cleanup + Phase V3
Datum: 2026-03-07
## Ergebnis: ⚠️ Kleinigkeiten
---
## Gefundene Probleme
### [backend/app/domains/media/router.py] Auth fehlt auf GET /{asset_id} und DELETE-Endpunkten
**Schwere**: Mittel
**Empfehlung**: `get_asset`, `archive_asset`, `delete_asset_permanent` haben kein `get_current_user` Dependency. War nicht im Plan-Scope, sollte aber in einem Folge-Task ergänzt werden. Aktuell könnte jede Person mit einer Asset-UUID das Asset abrufen oder löschen — allerdings sind UUIDs nicht ratbar (V2-C2 ist damit in der Praxis weitgehend erfüllt, aber formal unvollständig).
### [backend/app/domains/rendering/tasks.py] asyncio.get_event_loop() in Celery-Kontext
**Schwere**: Gering
**Empfehlung**: `asyncio.get_event_loop().run_until_complete()` in `_update_workflow_run_status()` ist ein Anti-Pattern in neueren Python-Versionen (3.10+). Deprecation-Warning möglich wenn kein laufender Loop existiert. Besser: `asyncio.run()` oder sync SQLAlchemy-Session wie in `dispatch_service.py`. Kein Blocker.
### [render-worker/scripts/export_gltf.py] O(N×M) Proximity-Loop
**Schwere**: Gering
**Empfehlung**: `mark_sharp_edges_by_proximity()` iteriert alle Blender-Mesh-Edges gegen alle OCC-Kantenmittelpunkte. Bei großen STEP-Dateien (10k+ Edges, 500+ OCC-Hinweispunkte) kann das spürbar langsam sein. Nicht kritisch für aktuelle Produktgrößen. Notiz für spätere Optimierung (z.B. KD-Tree mit `scipy.spatial`).
### [backend/app/services/step_processor.py] bbox-Extraktion ohne Shape-Guard (OCC-Pfad)
**Schwere**: Gering
**Empfehlung**: `brepbndlib.Add(shape, bbox)` kann bei degenerierten STEP-Geometrien eine leere BBox zurückgeben. Ein Guard `if not bbox.IsVoid():` vor dem `bbox.Get()` wäre robuster. Dieser OCC-Pfad ist in der aktuellen Container-Konfiguration nicht aktiv (kein OCC installiert), aber beim nächsten Container-Upgrade relevant.
---
## Positiv aufgefallen
- **V2-C1 (asset_type-Klassifizierung)**: Korrektur in beiden Stellen (`admin.py` + `step_tasks.py`) konsistent auf Extension-Basis umgestellt.
- **V2-C2 (Tenant Isolation)**: `get_current_user` Dependency korrekt auf `list_assets`, `download_asset`, `zip_download` ergänzt. Pattern konsistent mit anderen Routers.
- **V2-C3 (storage_key Normalisierung)**: `_normalize_key()` Helper in `admin.py` sauber definiert. In `step_tasks.py` inline normalisiert.
- **V2-C4 (Cache-Control)**: Header auf beiden Endpoints korrekt gesetzt.
- **V3-A1 (OCC Bounding Box in step_processor.py)**: Code korrekt, aber nur wirksam wenn OCC installiert ist — in der Produktionskonfiguration nicht aktiv.
- **V3-A2 (Frontend Dimensionen)**: `cad_mesh_attributes` im `ProductOut`-Schema sauber ergänzt. `selectinload(Product.cad_file)` war bereits in allen Queries vorhanden — kein N+1-Problem.
- **V3-B (Mark Sharp Edges)**: Proximity-basiertes Marking mit konfigurierbarem Threshold (1mm default) ist ein pragmatischer Ansatz.
- **V3-C1/C2/C3 (Workflow-Integration)**: `still_with_exports` korrekt ergänzt, Turntable-Params werden zur Laufzeit aufgelöst, WorkflowRun-Status wird nach Task-Abschluss aktualisiert.
- **bbox via STL (nachträglicher Fix)**: `_bbox_from_stl()` mit numpy min/max ist die effizienteste Methode — nutzt bereits gecachte STL-Dateien, kein STEP-Re-Parse nötig. Cadquery-Fallback für Dateien ohne STL-Cache ist korrekt implementiert.
- **`render_step_thumbnail` Patch**: Nur ausgeführt wenn `dimensions_mm` noch nicht gesetzt — vermeidet redundante Berechnungen bei Re-Renders.
- **TypeScript**: `tsc --noEmit` läuft ohne Fehler. Neue `cad_mesh_attributes`-Interface-Felder korrekt typisiert.
- **LEARNINGS.md**: 5 neue Learnings mit korrektem Format eingetragen.
---
## Empfehlung
Freigabe mit folgenden Nacharbeiten im nächsten Cleanup-Cycle:
1. Auth auf `get_asset` + `archive_asset` + `delete_asset_permanent` in `media/router.py` ergänzen
2. `_update_workflow_run_status()` auf `asyncio.run()` oder sync-SQLAlchemy umstellen
3. `if not bbox.IsVoid():` Guard in `step_processor.py` vor `bbox.Get()` einfügen
Keiner dieser Punkte blockiert den aktuellen Stand — alle Core-Features sind korrekt implementiert.
Review abgeschlossen. Ergebnis: ⚠️