Files
Melo/docs/reviews/2026-08-27-sync-ausbau/review_panel_report.md
T
Hermes (Server)andClaude Sonnet 5 852c17cd48 Review-Panel: Sync-Ausbau-Spec (Verdikt 6/10, nachschärfen)
Adversariales 5-Reviewer-Panel (Opus) mit 2 Debattenrunden,
Vollständigkeits-Audit, konsolidierter Faktenprüfung (~70 Belege,
keine Halluzination) und Richterurteil.

Ergebnis: SPEC NACHSCHÄRFEN DANN FREIGEBEN. Kein P0 im Geltungsbereich.
17 Aktionspunkte, wichtigste: POST /favorites (Voll-Ersatz) streichen
statt Regel, SSE und beidseitigen Playlist-Merge herausnehmen,
irreversiblen Bestands-Löschpfad benennen (Backlog-P0, Rückfrage an
Dustin offen).

Bemerkenswert: Der vom Panel selbst empfohlene Basis-Snapshot wurde
von der Verifikation als gefährlicher entlarvt als das Problem, das
er lösen sollte — abgelehnt.

Reviewer-Rohtexte bleiben lokal (.gitignore), nur Bericht + Urteil
im Repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcDiyJdVRqh1TtJk5JiabX
2026-08-27 01:35:55 +02:00

131 lines
7.0 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-Panel-Bericht — Sync-Ausbau-Spec
**Gegenstand:** `docs/superpowers/specs/2026-08-27-sync-ausbau-design.md`
**Datum:** 2026-08-27
**Panel:** 5 Reviewer (Feasibility, Risk, Devil's Advocate, Flutter/Android,
Sync-/Verteilte-Systeme) + Vollständigkeits-Auditor + konsolidierter
Verifizierer + Oberster Richter. Alle Modell: Opus.
**Ablauf:** Phase 3 (unabhängig) → 4 (Reflexion) → 5 (Debatte, 2 Runden) → 7
(blinde Schlussurteile) → 8 (Audit) → 10+11 (Zitat- + Severity-Prüfung) → 14
(Urteil).
**Review-Modus:** Exhaustive (Design-Dokument), aber alle Bestandsbehauptungen
gegen echten Code geprüft.
**Verdikt: SPEC NACHSCHÄRFEN DANN FREIGEBEN · Score 6/10 · Konfidenz: Hoch**
Panel-Scores: 6 / 5 / 5 / 5 / 5 (Mittel 5,2). Der Richter liegt darüber, weil
die Faktenprüfung zwei tragende Säulen der Panel-Kritik entfernt hat.
---
## Kurzfassung
Die Spec ist **im Kern richtig und faktisch überwiegend korrekt** — rund zwanzig
Behauptungen über den Bestandscode haben fünf unabhängigen Prüfungen
standgehalten, und in zwei Fällen lag das Panel falsch, nicht die Spec. Es gibt
**kein P0 im Geltungsbereich**: Der Datenverlust-Bug, den die Spec beheben will,
kann heute nichts verlieren (die Live-DB hat 0 Favoriten, und
`MeloCloudService.favoriten()` hat null Aufrufer).
Drei belegte Substanzmängel verhindern die direkte Freigabe:
1. Eine **falsche Garantie** über Lösch-Propagation.
2. **Fünf zu optimistische Bestandsbehauptungen** („existiert bereits", wo es
das nicht tut).
3. **Zu großer Umfang** für einen Wurf: sechs Ziele, von denen genau eines auf
nichts wartet und einen echten Bug behebt.
**Der wichtigste Einzelbefund richtet sich gegen das Panel, nicht gegen die
Spec:** Das Panel konvergierte in Debattenrunde 2 auf einen „Basis-Snapshot" als
Merge-Mechanismus. Die separate Verifikation hat nachgerechnet — liefert
`GET /favorites` fälschlich eine leere Menge (was `parseFavoriten` heute bei
jedem Fehler im 200er-Körper tut, und sechs Handler desselben Servers antworten
so), kollabiert die Formel und **löscht den gesamten Bestand auf allen Geräten**.
Der empfohlene Ersatz wäre gefährlicher gewesen als das Problem. Er ist
abgelehnt.
---
## Umfang und Grenzen
Geprüft wurde ein Design-Dokument, kein laufender Code. Nicht bewertbar:
Laufzeitverhalten, echte Gerätetests, Server-Last unter echten Bedingungen.
Alle Code-Aussagen sind read-only verifiziert (~70 Datei:Zeile-Belege).
Epistemik-Labels: [VERIFIZIERT] [KONSENS] [EINZELQUELLE] [UNVERIFIZIERT]
Typ-Labels: [BESTANDSFEHLER] (existiert heute) [PLAN-RISIKO] (entsteht erst
durch Umsetzung)
---
## Konsens-Punkte (vom Richter bestätigt)
- Die **Lösch-Kette ist Glied für Glied bestätigt** [VERIFIZIERT]
[BESTANDSFEHLER]: Verschwindet eine Datei lokal (SD-Karte nicht eingehängt,
Dateimanager, Verschieben), markiert der Scan sie als gelöscht, der Sync meldet
das dem Server, und der Server löscht die Audiodatei aus **beiden** Orten
(Registry + Navidrome-Ordner). Für die Datei gibt es keinen Grabstein, der
Reparaturweg ist verifiziert kaputt. Die Lösch-Bremse greift bei 325 Titeln
erst ab 109 gleichzeitigen Löschungen. **Nicht von dieser Spec verursacht**
bisher nie ausgelöst (`SUM(deleted)=0`), aber ein echter Bestandsfehler.
- Die **Merge-Semantik der Playlisten ist unterspezifiziert** [KONSENS]: vier
offene Entscheidungen (Identität, Reihenfolge bei Gleichstand,
Tombstone-Verhalten, Namens-Zuordnung als Nicht-Funktion).
- **`flutter_local_notifications` erzwingt Gradle-Umbau** (Desugaring) —
der einzige harte Build-Blocker der Planung.
## Wo das Panel falsch lag (Verifikation korrigiert)
- „Auswahl-Modus existiert so nicht" — **widerlegt**, er existiert
(`sortable_song_list.dart`).
- „Pro-Song-Lade-Loop bringt keinen Fortschritt/Doppel-Lauf-Schutz mit" —
**widerlegt**, `DownloadService.lade()` bringt beides mit. **Die Spec hatte
recht.**
- „Navidrome-Playlist-Import erzeugt Merge-Explosion" — **widerlegt**, die
importierten Playlisten sind leer (ID-Räume treffen nie).
- „SSE erschöpft den Worker-Pool" — **vom Panel selbst zurückgezogen**
(ThreadingMixIn ohne Pool).
---
## Aktionsliste (17 Punkte, priorisiert)
Vollständig mit Formulierungsvorschlägen in
`state/phase_14_judge_ruling.md`, Abschnitt 6. Die wichtigsten:
| # | Sev | Änderung |
|---|---|---|
| A1 | P1 | **`POST /favorites` (Voll-Ersatz) ersatzlos streichen** — nur noch additive `toggle set:true` für `lokal \ server`. Macht den Datenverlust-Bug *strukturell* unmöglich statt per Regel. |
| A2 | P1 | **`parseFavoriten` härten** (Fehler im 200er-Körper werfen, nicht `[]` liefern) + den grünen Bestandstest mitändern, der heute das Gegenteil festschreibt. |
| A3 | P1 | **Falsche Lösch-Garantie streichen** — auch *online* entfernte Herzen propagieren nicht zuverlässig. |
| A4 | P1 | **Basis-Snapshot ausdrücklich ablehnen** und begründen (Gegen-Empfehlung zum Panel). |
| A5 | P1 | **Playlist-Sync auf einseitige Sicherung reduzieren** — löst alle vier offenen Semantik-Fragen ersatzlos auf. Beidseitiger Merge wird eigene Spec. |
| A6 | P1 (Spec) / **P0 (Backlog)** | **Irreversiblen Löschpfad benennen** — plus die eine Frage, die kein Reviewer entscheiden kann (siehe unten). |
| A7 | P1 | **SSE herausnehmen** — der Hauptnutzen hängt am `song_upload`-Event, das die Spec selbst auslagert; acht Fehlerklassen für heute null Adressaten. |
| A8 | P1 | **Notification halbieren**: Sync-Bericht sofort, persistente Notification als eigene Stufe mit Beweis-Build. |
| A9 | P1 | **Fehlerfälle von Zusagen auf zu bauende Arbeit umstellen** — fünf „bestehende Schutzmechanismen" gibt es nicht. |
| A10 | P1 | **Testliste um Bestandsänderungen erweitern** — sie nennt heute nur neue Tests. |
| A17 | P1 | **Auslieferungsreihenfolge festschreiben** — fehlt heute ganz. |
| A11A16 | P2/P3 | Bestandsbehauptungen korrigieren, offene Entscheidungen treffen, Kleinigkeiten. |
**Bedingung des Richters:** Die nachgeschärfte Fassung wird einmal kurz
gegengelesen, bevor der Implementierungsplan entsteht — A1, A5 und A7 ändern,
*was* gebaut wird, nicht nur *wie* es beschrieben ist.
---
## Meta-Beobachtung zum Verfahren
Bemerkenswert stark: ~70 geprüfte Belege, **keine einzige Halluzination**,
keine Fehlzuordnung. Mehrere Reviewer haben eigene P0-Befunde aktiv falsifiziert
— das Gegenteil von Konsens-Drift.
Bemerkenswert schwach, und lehrreich: **Das Panel prüfte die Spec adversarial,
seine eigene Empfehlung aber nur konsensual.** Sobald Runde 2 auf den
Basis-Snapshot konvergiert war, griff ihn niemand mehr mit derselben Härte an.
Erst die separate Verifikation deckte auf, dass er selbst einen Löschpfad
schafft. Zweitens war der Suchraum zu klein: zwei Runden „Outbox oder
Basis-Snapshot" behandelten den Schreibweg als gegeben — die billigste Lösung
bestand darin, einen Aufruf zu **streichen**, nicht einen Mechanismus zu bauen.
Drittens: die Testschuld fand erst der Vollständigkeits-Auditor; fünf Reviewer
sezierten `sync_service.dart` zeilengenau und betraten `test/` nur einmal.