0 Fork 0

Files

T

david e341c5938a Track HiveGrid Cloud planning reference

2026-06-26 20:48:01 +02:00

17 KiB

Raw Blame History

Phase 07 — Chatbot — VERIFICATION

Verifier: general-purpose (read-only) Date: 2026-05-16 Files reviewed: README.md, chatbot-mvp.md, chatbot-v1.md, chatbot-v2.md Overall verdict: PASS mit Auflagen — Plan ist im Kern stimmig, vollständig, sequenziell sauber begründet. Einige nicht-blockierende Lücken bei Chat-History, Reset-Funktion, Multilingual sowie eine kleinere Inkonsistenz zwischen Approach-Text und Implementation-Step bei der V1-Resolver-Override-Semantik.

1) Template-Konformität (DETAIL-MD-TEMPLATE.md, 15 Sections)

Section	mvp.md	v1.md	v2.md
Frontmatter (Status/Task-ID/Phase/Effort/Owner)	OK	OK	OK
Problem	OK	OK	OK
Depends on	OK	OK	OK
Blocks	OK	OK	OK
Approach	OK	OK	OK
Implementation Steps	OK (11)	OK (12)	OK (13)
Test Requirements	OK	OK	OK
Acceptance Criteria	OK	OK	OK
Rollback Strategy	OK	OK	OK
Security Notes	OK	OK	OK
Subagent Recommendations	OK	OK	OK
Staging-Deployment-Steps	OK	OK	OK
Deviations from Plan	OK	OK	OK
Progress Log	OK	OK	OK
Lessons Learned	OK	OK	OK

Reihenfolge: Stimmt in allen 3 Files exakt mit Template überein. Frontmatter-Detail v1.md: Leerzeile zwischen Estimated Effort und Owner (kosmetisch, nicht template-konform aber irrelevant).

README.md folgt — wie für Phase-README üblich — nicht dem Detail-Template (richtig so).

2) Dependencies konsistent

MVP:

Depends on 05-business-features/feature-flags.md (#53) — OK. Begründung im Text sauber (Multi-Tenant-Persistence, SharedStore-Cache).
Blocks: V1 + V2 — OK.

V1:

Depends on: MVP + ai-cost-phase-2.md (#55) + 10-compliance-polish/audit-compliance-export.md (#67) — OK, vollständig.
README-Dep-Graph zeigt #55 nicht als V1-Input — Inkonsistenz: Der README-Graph führt nur „Phase 10 audit-compliance-export.md" als externe V1-Dep, vergisst ai-cost-phase-2.md (#55, Rate-Limits). Empfehlung: im README-Graph eine Linie Phase 05 ai-cost-phase-2.md (#55) → #14 ergänzen, damit Graph zur Detail-MD passt. Minor.
Blocks: V2 — OK.

V2:

Depends on: V1 + meilisearch.md (#48) + vector-store.md (#49) + embedding-pipeline.md (#50) + ai-cost-phase-1/2/3.md (#54/55/56) — OK, alle 4 Phase-04-Files (technisch: 3 Phase-04-Files; Meilisearch ist Hybrid-Pre-Filter, vector-store + embedding-pipeline sind Pflicht) plus alle 3 AI-Cost-Phasen. Begründung pro Dep sauber.
Blocks: „nichts kritisches" — OK.

Auflage (Minor): README-Graph für V1 um #55 ergänzen.

3) Implementation-Steps-Qualität

MVP

Feature-Flag-Check ersetzt hardcoded 503 (chatbot-stream.server.ts:41) — Step 3 macht das explizit: „Zeile 41 entfernen; vor Step 3 resolveCapabilitiesForUser({tenantId, userId}) aufrufen". Korrekt. Zusätzlich saubere Trennung von „flag off → 403" vs. „flag on + no key → 503 mit code:NO_AI_PROVIDER_KEY". Lobenswert: unterschiedliche Status-Codes für unterschiedliche User-Wahrnehmungen.
3 Basis-Capabilities als Tools exposed — Steps 5/6/7 definieren qa_help (statische Markdown-Index + Keyword-Matching, kein Embedding), kb_search (Postgres-FTS, tenant-scoped Pflicht), account_info (Plan/Credits/Limits/MembersCount, Sensitive-Data-Whitelist). Tool-Factory-Pattern mit injizierter tenantId via Closure (kein tenantId im LLM-sichtbaren Schema) — Best Practice, sauber begründet im Approach.
Capability-gated Tool-Registry (Step 8): Tools werden NICHT registriert wenn Capability off — der Approach erklärt explizit warum „im Tool selbst prüfen" verworfen wurde (Token-Burn, User-Verwirrung). Saubere Architektur-Entscheidung.
Settings-UI + Onboarding-Warning (Steps 9/10): Vollständig.

V1

TenantChatbotAccess Prisma-Model + Per-User-Toggles (Step 1): Prisma-Schema komplett gezeigt, @@unique([tenantId, userId]) und @@index([tenantId]) korrekt, Cascade-Deletes auf Tenant+User. Sauber.
Capability-gated Tool-Registrierung (Step 6 + Step 9): Erweiterung des Switch-Statements aus MVP auf 8 Cases. High-Risk-Wrapping via wrapHighRiskTool(...) mit Pre-Flight-Rate-Limit + Audit. Technisch sauber: Wrapper-Pattern verhindert Code-Duplikation, jeder High-Risk-Tool-Pfad geht garantiert durch Rate-Limit + Audit.
Audit-Event pro High-Risk-Tool-Call (Step 7): chatbot-audit.server.ts schreibt AuditEvent mit category='chatbot.tool_call', action=capability+'.'+toolName, sanitize via Allow-List. Integration mit Task #67 explizit benannt (Hash-Chain wird genutzt, direkter DB-Write verboten). Korrekt verdrahtet.
Per-Capability Rate-Limits via Quota-Engine (Step 8): enforceQuota aus Task #55 wird explizit referenziert, Scope chatbot.${capability}, konkrete Default-Limits angegeben. Sauber verdrahtet.

Inkonsistenz im V1-Resolver (Approach vs. Step 4):

Approach Z.39 sagt: „Default-Modus ist konfigurierbar: entweder all_members oder allow_list".
Step 4 (Resolver-Logic) sagt korrekterweise: bei all_members und Row mit capabilityScopes → diese Liste; sonst Tenant-Default. OK.
Step 4 sagt aber NICHT, wie capabilityScopes per Row im UI gesetzt werden, wenn der Tenant im all_members-Modus ist — Step 10 (UI) zeigt nur „pro User Toggle enabled + Override-Capabilities" was OK ist. Konsistent, aber V2 ändert die Semantik dieses Feldes dann radikal — siehe V2.

V2 (Step 9/10) konvertiert dann capabilityScopes: String[] zu Json (Map mit inherit|grant|deny). Das ist eine breaking schema change mit Data-Migration. Plan ist explizit, Migration-Step ist drin (Step 10), Backfill-Skript referenziert. OK, aber komplex — siehe V2.

V2

RAG-Retrieval-Layer im System-Prompt-Builder (Steps 4-6): retrieveRelevantContext({tenantId, query, topK=5}) ruft vectorStore.semanticSearch aus Phase 04-Adapter mit filter: { tenantId }. System-Prompt-Builder bekommt ragSnippets? Parameter und rendert ## Relevant context-Section mit [ref:N]-Markern. Stream-Endpoint ruft Retrieval vor jedem Stream-Call auf. Korrekt, alles aus Phase 04 wiederverwendet, keine Duplikation.
Per-User-Capability-Overrides mit 3-State (Step 9): capabilityScopes neue Repräsentation als JSON-Map {[capability]: 'inherit'|'grant'|'deny'}. Resolver: start mit Tenant-Default, dann pro Capability User-Override anwenden. Migration-Strategie ausdrücklich (Step 10 mit data.ts-Backfill). Sauber, Migration explizit.
MCP-Tools über external_tools-Capability gegated (Step 11): Stub aus V1 wird ersetzt durch echte MCP-Registry-Integration; jedes MCP-Tool als Sub-Tool exposed unter external_tools-Gate. AuditEvent + Rate-Limit aus V1 wiederverwendet. Korrekt verdrahtet.
AICostEvent.capability-Feld (Step 7): trackChatbotCost(...) schreibt via ai-cost-phase-1 Producer-Helper Event mit Pflicht-Capability-Feld. Pseudo-Capability chat_general für Tool-freie Generations definiert. Sauber.

4) Test-Requirements

Pro Capability ein E2E-Test (Tool-not-exposed wenn Capability off):
- MVP: 3 Cases in capability-exposure.integration.test.ts — OK.
- V1: explizit „nicht nur 3 wie in MVP, sondern alle 8 Capabilities × 2 Runs (enabled/disabled)" — OK, Pattern wird sauber erweitert.
- V2: erweitert auf 8 × 3 (inherit/grant/deny vs. Tenant-Default) — OK, sehr gründlich.
RAG-Retrieval-Tests mit Mock-Embedding: V2 hat retrieve-relevant-context.server.test.ts (Integration mit Test-pgvector via testcontainers) + v2-rag-flow.integration.test.ts (full-flow mit Embedding-Pipeline) + Cross-Tenant-Negativ-Test als CI-Pflicht-Blocker. OK, aber kein expliziter „Mock-Embedding"-Vermerk — der Integration-Test nutzt testcontainers mit echtem pgvector. Für Unit-Tests fehlt eine mockEmbeddingProvider-Notiz. Empfehlung: Unit-Test-Variante mit deterministic Mock-Embedding ergänzen, damit Tests ohne testcontainers in CI laufen können. Minor.
Cross-Tenant-Negativ-Tests: explizit als Pflicht in allen 3 Stufen (MVP: kb_search; V1: alle 8; V2: RAG-Embeddings). Lobenswert.

5) Security-Notes — Indirect Prompt Injection

Der vom Security-Track identifizierte Indirect-Prompt-Injection-Finding ist adressiert, mit eskalierender Mitigation pro Stufe:

MVP (Read-only): Daten-Instruktions-Trennung via ---KB ARTICLE START/END----Marker, System-Prompt-Regel, Blast-Radius minimal (alle Capabilities read-only).
V1 (Schreibend): zusätzlich (1) ---TOOL_RESULT START (untrusted content)----Marker, (2) Confirm-Step für High-Risk-Tools im Chat-Widget (Tool returnt {status: 'awaiting_confirmation', confirmationId}, UI rendert Confirm-Step, separater POST endgültig), (3) Audit-Log immer aktiv, (4) Rolle-Check für admin_actions zusätzlich zur Capability.
V2 (RAG): zusätzlich (1) Strikte Marker in RAG-Section, (2) Sub-Prompt-Sandbox-Pattern (Snippets gehen in user-Channel statt system-Channel), (3) Heuristic-Filter detectInjectionMarkers pre-injection, (4) Citation-Spoofing-Validation im UI-Renderer (unbekannte refs → [broken reference]).

Alle 4 vom User explizit verlangten Mitigations (Daten-Instruktions-Trennung mit Markern, Sub-Prompt-Sandbox, Heuristic-Filter, Confirm-Step für High-Risk-Actions) sind vorhanden, korrekt verteilt über MVP/V1/V2. PASS.

Zusätzliche Security-Sorgen:

Embedding-Leak zwischen Tenants explizit als CI-Pflicht.
MCP-Tool-Trust-Boundary erwähnt Phase-06-WASM-Sandbox als künftige Mitigation.
Quota-Bypass via Stream-Abbruch → onAbort-Hook ergänzt.
Cost-Event-PII (keine Prompts/Completions) → DSGVO-konform.

6) Konsistenz mit User-Wünschen

„Modularer SaaS-bewusster Chatbot mit gestaffelten Toggles, alle 4 Layer (Bewusstsein/Master/User/Capability) durchgehend":
- Bewusstsein = System-Prompt-Builder mit Tenant/User-Context — MVP Step 4. OK.
- Master-Switch = Feature-Flag chatbot — MVP Step 3. OK.
- User-Layer = TenantChatbotAccess pro (tenantId, userId) — V1 Step 1, V2 Step 9 (3-State). OK.
- Capability-Layer = 8 Capabilities + Tool-Registry-Gating — MVP Steps 5-8, V1 Steps 5-6, V2 Step 11. OK.
- Durchgängig in allen 3 Stufen. PASS.
„Per-Tenant + Per-User-Toggles": V1 deckt Per-User ab, V2 erweitert auf 3-State-Capability-Overrides. PASS.
„RAG mit pgvector/Qdrant": V2 nutzt explizit vector-store-Adapter aus Phase 04 (vector-store.md #49), pgvector als Default genannt, Adapter erlaubt Provider-Wechsel zu Qdrant. PASS.
„AI-Cost-Tracking pro Capability": V2 schreibt AICostEvent mit Pflicht-capability-Feld, nutzt Producer aus ai-cost-phase-1.md (#54), Per-Capability-Quotas via ai-cost-phase-2.md (#55), Aggregation/Reporting via ai-cost-phase-3.md (#56). PASS.

7) Staging-Deployment-Steps

MVP: Smoke-Test mit 3 konkreten Fragen für 3 Capabilities + Cross-Tenant-Negativ-Test + Onboarding-Test. OK.
V1: Smoke-Test mit OWNER + Power-User + Audit-Check + Rate-Limit-Check (51 Calls Test-Script) + Prompt-Injection-Test mit präparierten KB-Article. OK, sehr konkret.
V2: Setup-Steps (Embedding-Provider-Key, Bulk-Embed-Backfill via rag-backfill.ts-Skript), RAG-Smoke-Test, Cost-Smoke-Test, Quota-Smoke-Test, Override-Smoke-Test, Prompt-Injection-Test. OK, alle Sub-Tracks abgedeckt.

Mocked LLM-Provider für E2E: Pattern existiert in Tests (Mock-LLM-Inspector für streamText-Tools-Map). Für Playwright-E2E wird auf Staging mit echtem Provider gefahren — das ist konsistent mit der Cost-Smoke-Test-Annahme. Nicht explizit dokumentiert wie E2E in CI-Pipeline ohne Provider-Key läuft. Empfehlung: Notiz in Test-Section ob Playwright-E2E in CI gegen Mock-Provider läuft oder nur gegen Staging — fehlt aktuell. Minor.

8) Fehlende Aspekte (vom Verifier identifiziert)

Die Aufgabe nennt explizit drei mögliche Lücken — Status:

a) Chatbot-History-Persistence

NICHT explizit adressiert. Weder MVP noch V1 noch V2 erwähnen Persistenz der Chat-Messages (User+Assistant). Im Modul-Pfad gibt es apps/web/app/modules/chatbot/db/ (in der Code-Karte erwähnt), aber Detail-MDs sagen nicht ob Chat/ChatMessage-Modelle bereits existieren oder ob neu zu bauen.
Lücke: Falls History bereits existiert → kurze Erwähnung in MVP-Approach „nutzt bestehendes Chat/ChatMessage-Schema" wäre sauber. Falls nicht → MVP Step 0 fehlt komplett (History-Tabelle + Tenant-Scope + Retention).
Empfehlung: Vor MVP-Implementation Explore-Subagent prüfen lassen, Detail-MD ergänzen. Sonst riskiert MVP einen In-Memory-Chat ohne Persistenz, was gegen „produktionsreif" verstößt.

b) Chatbot-Reset-Function (User-can-clear-history)

NICHT erwähnt. Weder in Settings-UI noch im Chat-Widget-Verhalten beschrieben. DSGVO-relevant (User soll seine eigenen Daten löschen können).
Empfehlung: V1 oder spätestens V2 Settings-UI um „Clear my chat history" + Endpoint ergänzen. Compliance-Export (Task #67) sollte ohnehin Per-User-Daten-Löschung können — Chatbot-History als ein Datentyp dort eintragen.

c) Multilingual-Chatbot-Support

NICHT erwähnt. System-Prompt ist Englisch-only (siehe Section-Namen wie ## You are, ## Tenant context, etc.). Keine Sprach-Detektion oder lokalisierte Antworten.
Empfehlung: Wenn Boilerplate Multi-Locale unterstützt (zu prüfen), sollte V1 oder V2 Locale aus User-Settings in System-Prompt aufnehmen („Respond in {locale}" oder explizite Übersetzung der RAG-Snippets). Sonst fragt deutscher User auf Deutsch und Bot antwortet auf Englisch.

Diese 3 Lücken sind nicht blocker für PASS-Verdict, weil sie eher Produkt-Features als Architektur-Lücken sind. Sie sollten aber explizit als Out-of-Scope-Notiz aufgenommen werden (z.B. in Phase-07-README oder als Backlog-Items für Phase 11 / Polish).

9) Weitere Befunde

Stärken

Sequenzielle Begründung sehr stark: README erklärt klar warum nicht „alles auf einmal" und welche Phase-Abhängigkeiten warum existieren.
Atomic-Commit-Disziplin: Jeder Step endet mit eindeutigem Commit-Message — gut für Rollback und Code-Review.
Wrapper-Pattern für High-Risk-Tools (V1 Step 9) ist die richtige Architektur — verhindert dass irgendein neuer High-Risk-Tool den Rate-Limit/Audit-Pfad versehentlich überspringt.
Capability-Exposure-Tests als Pattern wird konsequent durch alle 3 Stufen durchgezogen und sogar erweitert (3 → 8 → 24 Cases).
3-State-Override-Semantik (V2) ist eleganter als zusätzliches Model und Migration ist explizit.
Pseudo-Capability chat_general für Tool-freie Cost-Tracking ist saubere Modellierung.

Schwächen / Empfehlungen

README-Dep-Graph fehlt #55 als V1-Input (siehe Section 2). Minor.
Chat-History/Reset/Multilingual nicht adressiert (siehe Section 8). Medium. Sollte mindestens als Out-of-Scope-Notiz dokumentiert werden.
Unit-Test für retrieveRelevantContext ohne testcontainers fehlt — alle Tests dort sind Integration. Minor.
CI-Strategie für E2E-Playwright-Tests gegen LLM-Provider nicht dokumentiert. Minor.
V1 Step 8 erwähnt Defaults für Limits (data_read=200/h, etc.) aber V2 Step 12 spricht von „Per-Capability-Token-Quota-Inputs" im Settings-UI. Inkonsistenz: V1 = Call-Frequency-Limit, V2 = Token-Quota. Beide existieren parallel — explizit machen dass das ZWEI verschiedene Limits sind (Call-Rate-Limit vs. Token-Quota). Minor — Doku-Klärung empfohlen.
isProviderConfigured-Helper wird in MVP Step 3 erwähnt, aber nicht als eigener Implementation-Step. Minor: Falls Helper noch nicht existiert, sollte er als Sub-Step in Step 3 oder als Step 0 stehen.

Trennung von „chatbot.rag" vs. „chatbot"-Master-Flag

README erwähnt Subschalter chatbot, chatbot.user_allowlist, chatbot.rag — V2 Step 6 nutzt chatbot.rag. Konsistent.
chatbot.user_allowlist als separater Flag nicht in V1-Detail erwähnt — V1 nutzt stattdessen Tenant.chatbotAccessMode='allow_list'. Inkonsistenz README vs. V1-Detail: entweder Flag streichen oder als zusätzlicher Override-Layer dokumentieren. Minor.

10) Verdict

PASS mit folgenden Auflagen vor Implementation-Start:

MEDIUM: Klären ob Chat-History/Reset/Multilingual in Phase 07 Scope sind oder explizit Out-of-Scope (Backlog-Verweis). README oder Phase-Done-Definition entsprechend ergänzen.
MINOR: README-Dep-Graph um ai-cost-phase-2.md (#55) → V1 ergänzen.
MINOR: README-Subschalter-Liste vs. V1-Detail abgleichen (chatbot.user_allowlist vs. chatbotAccessMode-Spalte).
MINOR: V1/V2 explizit machen dass es ZWEI verschiedene Limit-Achsen gibt: Call-Rate-Limit (V1) und Token-Quota (V2).
MINOR: isProviderConfigured-Helper als Sub-Step in MVP Step 3 oder Step 0 dokumentieren.
MINOR: Test-Strategie für E2E gegen LLM-Provider (Staging vs. CI) kurz erwähnen.

Keine dieser Auflagen ist ein Blocker — der Plan kann implementiert werden, sobald Punkt 1 entschieden ist (Out-of-Scope-Notiz reicht).

Architektur-Korrektheit: Sehr hoch. Capability-Gating-Pattern, Wrapper-Pattern, Tenant-Scoping-Disziplin, Migration-Pfade, Security-Mitigations gegen Prompt-Injection sind alle vorbildlich.

Vollständigkeit: 95% — die fehlenden 5% sind die in Section 8 genannten Produkt-Features (History/Reset/Multilingual), nicht Architektur-Lücken.

Konsistenz: Hoch — wenige Minor-Inkonsistenzen zwischen README und Detail-MDs, alle leicht zu beheben.

17 KiB Raw Blame History Unescape Escape