跳转到内容

Erstes Code-Review nach dem Durchstich

此内容尚不支持你的语言。

Der Players-Durchstich lief durch.

Frontend, API Gateway, Service und Datenbank arbeiteten zusammen. Spieler konnten angelegt, angezeigt und deaktiviert werden. Die Auth-Absicherung griff. Die UI war poliert.

Aus Produktsicht: ein Erfolg.

Aus Architektursicht: ein Problem.

Die Umsetzung hatte echte Stärken.

Die vertikale Schnittlinie war eingehalten. Players-Code lag bei Players. Backend-Validierung, Tenant-Kontext, Soft Delete, Indexe — alles korrekt gesetzt. PrimeNG DataView statt Tabelle. Transloco vollständig. Keine hartcodierten Labels.

Das war nicht nichts.

Aber dann kam das Frontend-Domain-Layer.

Wer das Feature mit dem bestehenden Domain-Layer der App vergleicht — etwa mit todos oder members-directory — sieht sofort, dass zwei grundlegend unterschiedliche Muster gebaut wurden.

Das erwartete Muster:

Component dispatcht Intent via Facade
→ Facade ruft injectDispatch auf (void, kein Promise)
→ eventGroup: deactivateRequested
→ CommandStore withEventHandlers
→ exhaustMap → infrastructure/players.command.ts
→ mapResponse
→ deactivateSucceeded | deactivateFailed
→ ReadStore: reload via reloadOnWriteSuccess$
→ CommandStore: Notification via notificationEvents

Was tatsächlich gebaut wurde:

// Component
private async deactivateConfirmedPlayer(id: string): Promise<void> {
this.deleteFeedback.set(null);
this.deleteErrorDetail.set(null);
try {
await this.facade.deactivatePlayer(id);
this.deleteFeedback.set('success');
} catch (error) {
this.deleteFeedback.set('error');
this.deleteErrorDetail.set(this.toErrorMessage(error));
}
}

Fünf .set()-Aufrufe. async/await durch alle Schichten. try/catch als Fehlerkanal. Die Component orchestriert den Use Case selbst.

Das ist kein Stilproblem. Das ist ein Architekturbruch.

Warum das trotzdem gefährlich ist, obwohl es funktioniert

Abschnitt betitelt „Warum das trotzdem gefährlich ist, obwohl es funktioniert“

Genau hier liegt die Tücke.

Funktionierender Code ist nicht dasselbe wie tragfähiger Code. Die Unterschiede werden oft erst sichtbar, wenn das nächste Feature kommt, wenn zwei Requests gleichzeitig eintreffen oder wenn der erste Entwickler nach sechs Monaten den Code wieder aufmacht.

Race Conditions. Das deactivatePlayer-Pattern nutzt kein exhaustMap. Das bedeutet: Doppelklick auf Löschen schickt zwei parallele PATCH-Requests. Kein Schutz, keine Backpressure. Das reactive Muster mit exhaustMap schließt genau das strukturell aus.

Kein Event-System, kein Layering. Die Domain-Lib hatte keine +state/events/-, application/- oder infrastructure/-Ordner. Es gab keinen eventGroup, kein withEventHandlers, kein injectDispatch. HttpClient lag direkt im Signal Store. Die URL-Normalisierung wurde dreimal wiederholt. Ein eigenes toErrorMessage wurde sowohl im Store als auch in der Component definiert.

Component als Mini-Controller. Die Architekturvorgabe lautet: Component ist UI-Adapter, nicht Orchestrator. Hier hat die Component fünf lokale Signals verwaltet, Use-Case-Ablauf selbst gesteuert und Fehlerbehandlung durch alle Schichten gereicht. Das ist genau das, was “Presentation must not contain business orchestration” meint.

Feedback über lokalen State statt Notification-Events. Das Monorepo hat eine notification-events-Lib, die genau für diesen Zweck existiert: Feedback aus dem CommandStore dispatchen, zentralisiert und entkoppelt. Stattdessen wurden deleteFeedback und deleteErrorDetail als Signals in der Component gehalten — parallele Datenquellen für dasselbe fachliche Ereignis.

Das Einzelne ist lösbar. Das Muster ist das Problem.

Das ist keine Frage der Agent-Fähigkeit, sondern der Führungsqualität.

CLAUDE.md und frontend.md beschrieben das event-getriebene Architekturmuster in einer Zeile:

Event-driven UI flows: user intent → facade → command store
→ infrastructure → success/failure event → read store → VM.

Das ist eine Absichtsbeschreibung, keine Implementierungsanweisung.

Ein LLM hat einen starken Prior für das, was es aus Trainingsbeispielen kennt: async/await, try/catch, firstValueFrom, imperative Signal-Mutationen. Das ist das einfachste, bekannteste Angular-Muster. Ohne harte Gegensteuerung wird genau das verwendet — besonders wenn das zu implementierende Feature klein und überschaubar wirkt.

Das Referenzprojekt (libs/todos/domain) lag im Repo. Aber ein passiv vorhandenes Muster ist kein aktiver Constraint. Es gibt keine Anweisung, es vor der Implementierung zu lesen. Keine Ordnerstruktur als Vorgabe. Keine Liste expliziter Verbote.

Das Modell hat also aus dem Kontext gelernt, was es konnte — und dann das naheliegendste Muster verwendet.

Was geändert wurde, damit das nicht nochmal passiert

Abschnitt betitelt „Was geändert wurde, damit das nicht nochmal passiert“

Zwei Stellen wurden nachgezogen.

frontend.md wurde um drei neue Abschnitte erweitert:

  • Pflichtstruktur einer Domain-Lib (+state/events/, +state/, application/, infrastructure/) mit Dateibenennung und Zweck-Kommentaren
  • Den vollständigen Command-Flow als Codebeispiel für eventGroup, withEventHandlers, exhaustMap, mapResponse, injectDispatch und notificationEvents — jeweils mit Verweis auf die konkrete Referenzdatei in libs/todos/domain
  • Eine explizite Verbotsliste: kein firstValueFrom in Stores, kein async/await für HTTP, kein HttpClient in withProps, keine Promise-rückgebenden Facades, kein Feedback-State in Components, kein resource.reload() aus Components

AGENTS.md bekam eine Pflicht-Leseliste für jede Frontend-Domain-Implementierung:

libs/todos/domain/src/lib/+state/todos-command.store.ts
libs/todos/domain/src/lib/+state/todos-read.store.ts
libs/todos/domain/src/lib/application/todos-list.facade.ts
libs/todos/domain/src/lib/infrastructure/todos.command.ts
libs/todos/domain/src/lib/infrastructure/todos.resource.ts
libs/todos/domain/src/lib/infrastructure/todos.mapper.ts

Das Referenzprojekt ist jetzt kein passives Wissen mehr. Es ist ein aktiver Pflichtschritt.

Die Domain-Lib wurde vollständig gelöscht und nach dem richtigen Muster neu gebaut.

Vorher: drei flache Dateien, kein Layering, kein Event-System.

Nachher:

libs/tournament/players/domain/src/lib/
├── +state/
│ ├── events/
│ │ ├── players-create.events.ts
│ │ ├── players-deactivate.events.ts
│ │ ├── players-read.events.ts
│ │ └── players-ui.events.ts
│ ├── players-command.store.ts
│ ├── players-read.store.ts
│ └── players-read.mapper.ts
├── application/
│ └── players-list.facade.ts
└── infrastructure/
├── players.command.ts
├── players.endpoints.ts
├── players.mapper.ts
└── players.resource.ts

PlayersCommandStore behandelt HTTP jetzt über exhaustMap und mapResponse. PlayersListFacade nutzt injectDispatch und gibt void zurück. Die Infrastructure-Funktionen sind reine Observables ohne async/await. httpResource hat ein parse:-Callback mit zod-Schema. Notifications kommen aus dem Store via notificationEvents, nicht aus der Component.

Die Component hat sich entsprechend vereinfacht. Kein try/catch, keine Feedback-Signals, kein async. Die Dialog-Sichtbarkeit liegt im ReadStore und reagiert reaktiv auf createSucceeded.

Backend, API Gateway und Datenbank waren korrekt und blieben unverändert.

Das eigentlich Interessante an diesem Drift ist nicht das Ergebnis — es ist der Entstehungsweg.

Der Code war nicht nachlässig gebaut. Er hat eine funktionierende Oberfläche geliefert, mit korrekter Auth, korrekter Datenbankstruktur, vollständiger i18n, sauberer UI. Wer nur schaut, ob das Feature funktioniert, sieht nichts Problematisches.

Aber wer das Muster mit dem Rest des Projekts vergleicht, sieht sofort: hier wurden zwei Architekturen gleichzeitig gebaut.

Genau das ist die eigentliche Drift-Gefahr.

Nicht der eine schlechte Commit. Sondern das erste Feature, das ein abweichendes Muster etabliert — das dann alle nachfolgenden Features als scheinbar validen Standard übernehmen.

Schedule, Turniere, Gruppenphase, Spieltag — sie alle werden kommen. Und sie werden sich am ersten Muster orientieren, das sie im Codebase vorfinden.

Deshalb ist dieses Review nicht optional. Und deshalb war der Reset die richtige Entscheidung.

Ein Agent orientiert sich an vorhandenen Mustern. Wenn das erste fachliche Feature sauber ist, ist die Wahrscheinlichkeit höher, dass alle weiteren Features ebenfalls sauber werden. Wenn das erste Feature driftet, ist der nächste Drift bereits vorprogrammiert.