Zum Inhalt

Code-Review — Block 7.4 (Audit-Reader + Bulk-Ops + Connectors-Stub)

Branch: platform/block-7-4b-audit-bulk-frontend gegen platform/v1.0.0, inkl. Backend-Merge 61689c2 (7.4a). Zwei Sub-Commits, 23 Dateien (+2270 / -13). Tests: Backend 26 neu (12 unit + 14 integration), Frontend 12 neu Vitest + 2 neu Playwright. Regression Block 5–7 grün (79/79 Backend, 100/100 Vitest, 8/8 Playwright sequential).

Summary

ID Sev Datei:Zeile Titel Entscheidung
— — — Keine Merge-Blocker —
M1 75 AuditLogPage.vue (gesamt) Filter-Shortcut aus TenantsDetailPage wirkt nicht — ?entity_id=… aus Route wird nicht gelesen Deferred (UX-Fix Block 7.5 oder fast-follow)
M2 70 module_service.py:213–268, tenant_service.py:213–223 Bulk-Loop ohne Max-Cap und ohne Batch-Hydration (N round-trips) Deferred (Limit-Hardening Phase D)
M3 65 audit_service.py:96–168 CSV-Export mappt actor_keycloak_id / ip_address / session_id nicht in den Header Deferred
M4 60 operator_tenants.py:329, tenant_modules.py:312 Bulk-Audit-Auffindbarkeit: entity_id="bulk:N" schließt sich gegenseitig aus mit entity_id=<tenant.id>-Filter Deferred (Konvention dokumentieren)
M5 55 types/audit.ts:42–52 KNOWN_ACTIONS ist kuratierte Liste, hat keine Connector/Template/License-Actions Deferred (Block 13/14 erweitern)
L1 40 playwright.config.ts:19 workers: 1 als Pragma Deferred (akzeptiert)
L2 35 operator_audit.py:131 datetime.utcnow() (Py3.12 deprecated) Deferred
L3 30 operator_audit.py Filter Keine Validierung date_from <= date_to Deferred
L4 25 TenantsListPage.vue / TenantModulesSection.vue Bulk-UI ohne Progress-Indicator über Toast-Latenz Deferred (akzeptiert für v1.0.0)

Plan-Alignment sauber: Pfad-Split 7.4a/7.4b wie in PORTING-7.4.md, JSONB-Search ohne GIN als TODO mit Trigger > 10k, kein Connectors-Backend, kein Per-Tenant-Audit-Embed, kein Templates-Bulk. Atomarität via request_scoped_session + Single-Audit-Entry konsistent zur Block-5-Cleanup-Konvention.

Critical (≥80)

Keine. Atomarität-Pfad gut: TenantNotFound und ModuleNotAssignable rollen den ganzen Block via Context-Manager zurück (verifiziert über test_bulk_*_rollback-Pfade). Race-Sicherheit über ON CONFLICT DO UPDATE aus Block-5-Cleanup wiederverwendet. Scope-Guards (_require_operator für Tenants-Bulk, _require_operator_or_vendor für Modules-Bulk + Audit-Reader) konsistent. RLS-Bypass nur, wo der Operator fremde Tenants legitim sehen muss; expliziter tenant_id-WHERE bleibt für Defense-in-Depth bestehen.

Decision-Points

  1. Bulk-Audit Single-Entry vs. N-Entries. Die Lutz-Entscheidung ist für Compliance-Lesbarkeit richtig: ein Reviewer sieht „der Operator hat 17 Tenants in einer Aktion soft-gelöscht" als ein Event mit voller ID-Liste in details.after.soft_deleted_ids. Der Tradeoff ist die Auffindbarkeit (M4): wer den Audit-Log mit entity_id=<tenant.id> filtert, findet die tenant.bulk_soft_deleted- Zeile nicht, weil entity_id="bulk:17" ist. Empfehlung: Single- Entry behalten, aber im JSON-Search-Filter einen UI-Hint ergänzen („für Bulk-Events nach Tenant-UUID in JSON-Suche fragen, nicht im Entity-ID-Feld") und in der Doku verankern. Skalierungsobergrenze: details.after-Liste wird bei sehr großen Bulks (mehrere tausend IDs) groß — bei aktuellem 50/Page-Limit der Audit-Liste irrelevant, aber CSV-Export mit include_details=true kann eine einzelne Zelle in den dreistelligen KB drücken. Akzeptabel für v1.0.0; bei Bedarf eine bulk_summary-Hilfstabelle in einem späteren Block.

  2. Audit-Tab im TenantsDetailPage als Filter-Shortcut. Vertretbar für v1.0.0. Aber M1 ist real: der Button navigiert zu /audit?entity_id=<tenant.id>, und die Audit-Page liest den Query-Parameter nirgends — useAuditEntries.ts:42 initialisiert entity_id: null, kein useRoute().query.entity_id-Hookup. Damit ist der Shortcut heute ein leerer Klick. Fix ist trivial (eine useRoute()-Auswertung in AuditLogPage.vue onMounted, plus Aufruf von setFilter("entity_id", route.query.entity_id)). Score 75 statt 80 nur, weil der User über das Filter-Feld den Wert manuell einfügen kann — die UX-Erwartung „Klick öffnet bereits gefiltert" ist aber gebrochen. Empfehlung: vor Phase D fixen, nicht nach Block 7-Abschluss vergessen.

  3. Bulk ohne Progress-Indicator. Akzeptabel. 100 Tenants × ein session.get + ein UPDATE liegen unter 500 ms gegen lokales Postgres. Bei 1000+ ändert sich die Wahrnehmung. Da es keinen Max-Cap gibt (M2), könnte ein Operator theoretisch 10000 IDs submitten und die UI hängt für Sekunden. Empfehlung: weiches Cap im Backend (z. B. max_items=500 pro Request) parallel zur N+1-Optimierung in Phase D, plus optional <FormButton :loading> reicht visuell. Kein Blocker für v1.0.0.

  4. workers: 1 in Playwright. Pragma, kein Smell. Cross-Test-Isolation per „lege Tenant pro Test, suche per Slug, delete am Ende" funktioniert sequenziell sauber. Parallel-Worker würde dieselbe Tenants-Liste lesen, gegenseitig Rows soft-deleten und die Specs flaky machen — das wäre der eigentliche Smell. Die Test-Suite ist in 8 Specs sequenziell unter ~30 s; der Lauf-Zeit- Overhead ist akzeptabel. Wenn Test-Volumen wächst, ist die richtige Antwort workers: N + per-Worker-DB-Schemas, nicht „Tenant-Slugs uniquen und hoffen". Heute korrekt.

  5. Connectors-Stub liest nichts vom Backend. Genau richtig. Block 13 wird die Operator-API designen; ein zwischenzeitliches Read-Only auf der Bestands-Tabelle würde Felder wie last_sync_at, error_state, permission_mode, die Block 13 entscheidet, in der UI festklopfen. Hint-Card mit Konnektor-Roadmap-Auszug ist die sauberste Lösung. Defensiv, ja — aber Block 13 ist ein Mehrjahres- Konnektor-Subsystem (Welle 1 v1.0.0 → MediaWiki v1.1.0 → Teams/Slack v1.2.x), und genau diese Defensive verhindert, dass eine Stub-API zur impliziten Spezifikation wird.

Deferrable (<80)

  • M1 (75) — ?entity_id-Routing-Hookup fehlt. UX-Bug. Fix < 10 LOC. In Block 7.5 oder vor Block 8.
  • M2 (70) — Bulk-N+1. module_service.bulk_assign_to_tenant macht session.get + INSERT...ON CONFLICT...RETURNING pro Item. Bei 100 Items sind das 200 Round-Trips. tenant_service.bulk_soft_delete hat dieselbe Form. Refactor: Pre-Validate per WHERE id IN (...)- Bulk-Select, dann ein einzelnes INSERT ... VALUES (...), (...) ON CONFLICT DO UPDATE bzw. ein UPDATE tenants SET deleted_at = now() WHERE id IN (...) AND deleted_at IS NULL RETURNING id. Phase D.
  • M3 (65) — CSV-Export-Felder. actor_keycloak_id, ip_address, session_id sind in DB + Pydantic + TS-Type, aber der CSV-Header in audit_service.export_csv listet sie nicht. Forensisch ärgerlich. Spalten ergänzen, Default-Header-Kompatibilität egal — Excel verkraftet zusätzliche Spalten.
  • M4 (60) — Bulk-Audit-Auffindbarkeit. Single-Entry-Konvention dokumentieren (vermutlich in docs-kora/docs/konzepte/audit.md oder Runbook), damit der Operator beim Suchen weiß, dass Bulk-Events nicht über entity_id auffindbar sind, sondern über action=*.bulk_* oder JSON-Search.
  • M5 (55) — KNOWN_ACTIONS-Drift. Bei Block 13/14 (Connectors, Lizenz-Audit) als Erweiterungspunkt mitnehmen.
  • L1 (40) — workers: 1. Akzeptiert.
  • L2 (35) — datetime.utcnow(). Auf datetime.now(UTC) umstellen, sobald irgendein anderer Bereich des Backends das Warning trifft.
  • L3 (30) — Date-Range-Validation. date_from > date_to führt heute zu leerer Result-Liste, kein 422. Pydantic-Validator in AuditListParams einbauen oder in der Route prüfen.
  • L4 (25) — Bulk-UI-Progress. Toast-Latenz ist heute nicht spürbar; bei Cap-Hardening (M2) nochmal anschauen.

Looks good

  • Pfad-Split 7.4a/7.4b sauber durchgehalten — Backend ist unabhängig mergebar, Frontend verlinkt nur auf existierende Routes.
  • Atomarität-Pattern aus Block 5 Cleanup (request_scoped_session + populate_existing=True) konsistent in den neuen Bulk-Pfaden.
  • Default-Date-Range „letzte 7 Tage" im Composable verhindert Initial-Full-Scan; defaultLast7DaysIso() ist UTC-stabil.
  • CSV-Export mit UTF-8-BOM + Hard-Limit + truncated-Footer ist Excel-tauglich und trunziert sichtbar.
  • Selection-Reset bei state.items-Watch in TenantsListPage.vue sowie bei tenantId-Watch in TenantModulesSection.vue schließt die klassische „IDs ausgewählt, die nicht mehr in der Liste sind"-Falle.
  • Filter-Toolbar mit 300-ms-Debounce auf Free-Text-Inputs hält den Server-Hit-Druck niedrig.
  • Connectors-Stub mit Roadmap-Verlinkung ist die richtige Defensive gegen Block-13-Präjudiz.
  • Pre-Flight-Discovery-Pflicht (PORTING-7.4.md mit Pfad- Entscheidung + Performance-Hinweisen) konsequent eingehalten und ist inzwischen Standard-Praxis im Platform-Branch.

Empfehlung

Merge nach platform/v1.0.0 mit Commit-Message-Footer „Code review: 9 issues found, 0 fixed, 9 deferred (<80)". M1 als Fast-Follow (10 LOC in AuditLogPage.vue onMounted) idealerweise vor dem Block-7- Abschluss-Tag, um den Audit-Tab-Filter-Shortcut funktional zu machen — sonst verspricht die UI etwas, was sie heute nicht liefert.