Add independent review for changes since 814705c
This commit is contained in:
52
docs/review/review-20260224.md
Normal file
52
docs/review/review-20260224.md
Normal file
@@ -0,0 +1,52 @@
|
|||||||
|
# Code Review – Änderungen von `814705c` bis `HEAD` (`ab7aa62`)
|
||||||
|
|
||||||
|
## 1) High-risk Findings
|
||||||
|
|
||||||
|
### Keine blocker-kritischen Defekte gefunden
|
||||||
|
Ich habe im betrachteten Commit keine eindeutigen Crash-/Datenverlust-Blocker gefunden, die einen sofortigen Release-Stopp erzwingen.
|
||||||
|
|
||||||
|
> Hinweis: Es gibt aber eine **relevante funktionale Regression** (siehe Nitpicks #1), die je nach Produktanforderung als Go/No-Go-Kriterium gewertet werden kann.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 2) Nitpicks / Verbesserungen
|
||||||
|
|
||||||
|
1. **Funktionale Regression in der UI (ehemals 4 Style-Regler, jetzt nur noch 1 sichtbar)**
|
||||||
|
In `lib/main.dart` zeigt `_buildColorStep()` nur noch den Slider `fidelityStructure`.
|
||||||
|
Die bisherigen Feineinstellungen `ditheringStrength`, `edgeEmphasis`, `colorVariation` sind weiter im State/Codec vorhanden, aber nicht mehr direkt im UI editierbar.
|
||||||
|
**Impact:** Nutzer verlieren granulare Qualitätskontrolle; gespeicherte Werte bleiben zwar erhalten, sind aber kaum noch aktiv manipulierbar.
|
||||||
|
|
||||||
|
2. **Stepper-Logik ohne Validierung / Guidance**
|
||||||
|
`onStepContinue` lässt das Voranschreiten ohne notwendige Preconditions zu (z. B. ohne Bildauswahl bis Schritt 4). Das ist technisch robust (Generate guarded), aber UX-seitig irreführend.
|
||||||
|
**Vorschlag:** Continue-Button je Schritt konditional deaktivieren oder klare Inline-Hinweise anzeigen.
|
||||||
|
|
||||||
|
3. **`Fertig`-Button im letzten Step ohne echte Aktion**
|
||||||
|
In `controlsBuilder` wird im letzten Schritt der Label-Text auf `Fertig` gesetzt, aber `onStepContinue` führt dort effektiv nichts mehr aus.
|
||||||
|
**Vorschlag:** Auf letzter Stufe Button ausblenden oder mit sinnvoller Aktion belegen (z. B. Export, Speichern, Zur Projektliste).
|
||||||
|
|
||||||
|
4. **Projekt-Ladesemantik setzt immer auf Step `result`**
|
||||||
|
Beim Laden mit Bild springt der Flow direkt auf `MosaicFlowStep.result`. Das ist für „Quick resume“ okay, nimmt aber ggf. den Guided-Flow-Charakter.
|
||||||
|
**Vorschlag:** Optionales Verhalten (z. B. Restore des letzten Steps oder Konfiguration `resumeAtResult`).
|
||||||
|
|
||||||
|
5. **Testabdeckung für neue Kernpfade noch dünn**
|
||||||
|
Es gibt gute Basis-Tests (`project_codec_test`, einfacher Stepper-Smoke-Test), aber es fehlen Tests für:
|
||||||
|
- latest/autosave-Verhalten (`latest_project.json`),
|
||||||
|
- manuell vs. automatisch speichern,
|
||||||
|
- Export-JSON-Struktur inkl. Result-Payload,
|
||||||
|
- Laden aus Snapshot vs. latest.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 3) Go/No-Go Empfehlung
|
||||||
|
|
||||||
|
**Empfehlung: GO mit Auflagen (kein Hard No-Go).**
|
||||||
|
|
||||||
|
Begründung:
|
||||||
|
- Die neuen Features (4-Step-Flow, autosave/latest, JSON-Export, Bestätigungsdialog beim Löschen) sind grundsätzlich sinnvoll umgesetzt.
|
||||||
|
- Kein klarer, reproduzierbarer Blocker im Diff erkennbar.
|
||||||
|
- Vor Release sollten jedoch mindestens die UX-/Funktionsregressionspunkte (insb. fehlende 3 Style-Regler) bewusst entschieden/fixiert werden.
|
||||||
|
|
||||||
|
**Release-Auflagen (kurz):**
|
||||||
|
1. Entscheiden/fixen, ob die 3 fehlenden Style-Regler absichtliche Scope-Reduktion oder Regression sind.
|
||||||
|
2. Stepper-UX (Continue/Finish-Verhalten) konsistenter machen.
|
||||||
|
3. 2–3 gezielte Tests für Save/Load/Export-Pfade ergänzen.
|
||||||
Reference in New Issue
Block a user