Skip to content

feat(review): review fields beside their source, correct, approve or reject - #35

Merged
Fluory merged 81 commits into
mainfrom
claude/feat-review-ui-8
Sep 23, 2026
Merged

Fluory merged 81 commits into
mainfrom
claude/feat-review-ui-8

Conversation

@Fluory

@Fluory Fluory commented Sep 23, 2026 •

Copy link
Copy Markdown
Owner

Warum

Fixes #8 · Teil von Epic #2 · gestapelt auf #34 (Basis claude/feat-jobs-worker-7).

Arbeitsstand

Was ist passiert (Klartext)

Die Prüfansicht ist fertig. Jede erkannte Angabe (Firma, Ansprechpartner:in, Liefertermin) steht mit ihrem Status in einer Tabelle; „unsicher" und „nicht bestätigt" sind farbig und mit Warnzeichen markiert. Ein Klick auf „Quelle anzeigen" zeigt die Mail-Zeile bzw. PDF-Seite mit der markierten Fundstelle. Korrekturen speichern alten und neuen Wert, Person und Zeit – im selben Schritt wie der Audit-Eintrag; ein korrigierter Wert heißt danach „korrigiert", nie mehr „belegt". „Freigeben" setzt die Anfrage auf Freigegeben und plant den Export im selben Schritt ein; „Ablehnen" verlangt einen Grund. Andere Firmen sehen und ändern nichts (geprüft). Neu ist ein Browser-Test, der den ganzen Weg Hochladen → Verarbeitung → Prüfen → Korrigieren → Freigeben durchspielt (KI-Dienst dabei als Attrappe). CI-Änderung: Der Schritt verify:full (nur mit Label verify-full) installiert jetzt vorher Chromium – rein additiv.

Plan-Pflicht (SYSTEM.md §4)

  • Kein Auslöser – keine Modulgrenze, öffentliche API, Migration, Auth/Rechte, kein Zahlungs-/Daten-/Infrapfad, höchstens zwei Module, keine Architekturvarianten, umkehrbar
  • Auslöser zutreffend – Impact Manifest ausgefüllt (Plan vor Code)

Impact Manifest

  • Betroffene Module: review (Ansicht, Korrektur, Freigabe, Ablehnung), requests (Übergänge REVIEW → APPROVED/REJECTED, Ablehnungsgrund), audit, jobs (Export-Queue-Definition + transaktionales Einstellen; Handler in feat(export): export approved requests exactly once to the ERP mock #9), db (Migrationen 0009/0010), app (Seite /requests/:id mit Server Actions), E2E (Playwright-Smoke in verify:full), CI (Chromium-Installation vor verify:full).
  • Schnittstellen / Datenänderungen: Tabelle app.field_corrections (append-only, FORCE RLS); app.requests.rejection_reason; Queue request-export; Server Actions (Rollenprüfung serverseitig, feste Meldungscodes in der URL).
  • Akzeptanzkriterien: alle aus feat(review): review fields beside their source, correct, approve or reject #8 – siehe Nachweis.
  • Testplan: Unit (Quellenansicht); Integration (Korrektur + Audit in einer Transaktion, Status „korrigiert", Freigabe + Export-Job in einer Transaktion inkl. Rollback, Ablehnung mit Grund, falscher Status, fremde Firma inkl. RLS auf field_corrections, append-only); Playwright-Smoke.
  • Verifizierte Fakten: @playwright/test 1.63.0 (Apache-2.0); vorinstalliertes Chromium der Session ist Revision 1194 → lokal per PW_CHROMIUM_PATH, in CI per playwright install --with-deps chromium.
  • Offene Annahmen: „PDF-Seite mit Hervorhebung" ist im Pilot eine Textansicht der Seite (Segmente in Lesereihenfolge, Zitat markiert) plus Link aufs Original, kein gerendertes PDF → decision-needed in feat(review): review fields beside their source, correct, approve or reject #8.
  • Nicht-Ziele: siehe oben.
  • Risiken und Rollback: Rollenprüfung in jeder Server Action und im Modul; kein Dokumentinhalt in Logs; Migrationen additiv; Rollback per Revert.

Geändert

  • src/db/schema/app.ts, Migrationen 0009_review.sql, 0010_review_force_rls.sql; src/features/jobs/{queues,boss}.ts (Export-Queue, enqueueRequestExport)
  • src/features/review/{review,source-view,index}.ts (+ Unit-Test)
  • src/app/requests/[id]/{page,actions,messages}.tsx/ts, src/app/requests/status-labels.ts, src/app/requests/page.tsx (Statusbeschriftung), src/app/globals.css, layout.tsx
  • tests/integration/review.test.ts; playwright.config.ts, tests/e2e/{ai-stub.mjs,global-setup.ts,review-smoke.spec.ts}; package.json (e2e, verify:full, @playwright/test)
  • .github/workflows/ci.yml (Chromium vor verify:full, gleiche Bedingung)
  • Doku: docs/technical/data-model.md, docs/technical/architecture.md, CHANGELOG.md, AGENTS.md (verify:full-Zeile)

Nachweis (SYSTEM.md §11)

  • verify:changed: typecheck + lint + vitest review (Unit 5/5, Integration 12/12) grün
  • verify: lokal grün (Exit 0) gegen echtes PostgreSQL 17 + SeaweedFS – Unit 105/105, Integration 68/68, depcruise ohne Verstöße, Build, Audit (nur 1 moderate, Schwelle high); CI check: siehe Checks dieses PRs
  • verify:full / E2E-Spec: tests/e2e/review-smoke.spec.ts lokal grün (1 passed, 14 s) mit Worker, gebauter App und KI-Stub; in CI per Label verify-full
  • Akzeptanzkriterien feat(review): review fields beside their source, correct, approve or reject #8: Felder mit Status + Quelle → Integration „shows every field…" + Unit Quellenansicht + E2E; Korrektur mit Protokoll → Integration „stores a correction…"; Freigabe → Export → Integration „approve…" (inkl. Rollback bei Enqueue-Fehler); Ablehnung mit Grund → Integration „reject…"; nur Rolle mit Recht / eigene Firma → Integration „another company…", „keeps corrections company-private…"
  • Manueller Prüfnachweis: Tastatur – alle Aktionen sind native Formulare/Links; Status nie nur über Farbe (Text + ⚠)
  • Frischer Review (P1 vor Ready-for-review; Architektur/API/DB immer): unabhängiger Subagent nach review-pr inkl. Security-Regel – 0 Blocker, 3 should-fix + 5 nits; alle should-fix behoben, Ergebnis als PR-Kommentar

Doku-Entscheidung (genau eine)

  • Keine langlebige Doku betroffen – Begründung: –
  • Doku betroffen und im selben PR aktualisiert:
    • Produktdoku (P0: README): –
    • Technische Doku: docs/technical/data-model.md
    • Architekturkarte: docs/technical/architecture.md
    • ADR: –
    • CHANGELOG [Unreleased] (sichtbares Feature oder Verhalten – im selben PR, nie „später")

Entferntes oder Umbenanntes: nichts entfernt

Dateigrößen und neue Bausteine (SYSTEM.md §7)

Dateien über 500 Zeilen im Diff (Ausnahmen: generierter Code, Lockfiles, Fixtures, Migrationen, Schemas, Ressourcen, Doku, Konfiguration):

  • keine
  • bewusst belassen – Begründung: –
  • im selben PR nach fachlicher Verantwortung geteilt
  • Folge-Issue –

Über 800 Zeilen mit neuer Fachlogik oder über 1000 Zeilen (P1/P2): nicht betroffen

Neue Shared-Komponente, Utility-Datei, Adapter oder fachlicher Service:

Subagent-Einsätze

  • Frischer Review (read-only, review-pr + Security-Regel): Ergebnis und Auflösung als PR-Kommentar.

Risiken / offene Punkte


🤖 Generated with Claude Code

https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1

…ion access control

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…LS with integration proofs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…boss queues

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…nd download routes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…-only audit, composite FK

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
- sign-up requires the invitation id (link) plus the invited e-mail – no takeover by address alone
- one company per user (unique index), deterministic membership lookup, actor from membership
- organization plugin accepts only admin/clerk roles
- configurable client-IP source for the auth rate limit; local secret refused in production
- invite page: zod input, 404 for clerks, shows the invitation link; signup needs the link
- tests: wrong/missing invitation id, foreign set-active/list-members, last admin, roles
- docs: operations (rate limit/proxy, recovery), data model, exceptions register (admin plugin)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…e rejection

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
… docs for #5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…secret

The image sets NODE_ENV=production, so the previous check blocked the local compose stack.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…re script

Python 3.13 package requestflow_ai (src layout), docling/fastapi/google-genai
pinned, CPU-only torch via the PyTorch CPU index, dev tools ruff/pyright/pytest.
The synthetic PDF fixture is generated by scripts/make_fixtures.py (reportlab).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Test-first per ADR-0001 D8: quote normalisation (whitespace, case, NFKC,
hyphenation), German number and date formats, and the verifier rules that
turn unsupported model claims into unverified. Also adds the segment and
model-output types the tests build on; the verifier does not exist yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
normalize_text folds NFKC, soft hyphens, line-break hyphenation, typographic
dashes/quotes, whitespace and case. values parses German/ISO numbers and
DD.MM.YYYY, D.M.YY and ISO dates. verify_field only keeps or downgrades the
model's status: a quote not in the cited segment, an unknown segment, a value
inconsistent with the quote, found without evidence or value, and missing
with a value all become unverified with a reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…ction (red)

EML: body lines with 1-based line locators, From/Subject header segments,
RFC 2047 and quoted-printable decoding, HTML-only bodies, header newline
collapse, no attachment payloads. PDF: textline segments with page + top-left
bbox via docling-parse (model-free); the layout-pipeline test only runs with
AI_TEST_DOCLING_MODELS=1. Detection by magic bytes; .msg rejected for now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
EML via the standard library email package (policy=default): From/Subject
header segments and one segment per non-empty body line, HTML-only bodies
reduced to text lines. docling's EMAIL backend was checked but emits
paragraphs without provenance, so it cannot give line locators.

PDF via docling: the default textlines pipeline reads docling-parse text
lines (page + top-left bbox, no ML models, no network); the opt-in layout
pipeline uses DocumentConverter (OCR and tables off) and the heron layout
model. docling is imported lazily.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…ex model client (red)

The model client is exercised through the real google-genai SDK with an
httpx MockTransport replaying recorded generateContent bodies: eu multi-region
URL, bearer auth, structured-output config, token usage, fail-closed init
(no project, no credentials, dev flag without key), no silent switch to API
key mode, and schema-invalid output. Adds the env-based Settings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…lient

Prompt extract_header_v1.md (system instruction) plus render_document, which
puts segment-id-prefixed lines between <document> delimiters and neutralises
delimiter-like tags inside the document. GeminiModelClient calls Vertex via
google-genai with response_schema = ModelExtraction, JSON mime type,
temperature 0 and no tools; schema-invalid output raises ModelOutputError.
build_model_client needs VERTEX_PROJECT and credentials, loads ADC eagerly,
refuses API-key mode for Vertex, and allows the Gemini API only with
AI_ALLOW_GEMINI_API_DEV=true plus GEMINI_API_KEY.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Pipeline: PDF and EML end to end with recorded model responses, a model date
contradicting its own quote, the prompt-injection mail (the recorded model
obeys and invents a quote -> unverified), and the no-text PDF that skips the
model. API: bearer auth (401 + WWW-Authenticate), response shape, request id
handling, 400/413/415/422/429/502 mapping without echoing input, JSON logs
with IDs only. Contract: the committed OpenAPI file equals the app's schema.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…p and native stderr

Fixtures are described as hand-written in the Vertex REST format (not
recorded). New env var AI_MAX_PDF_PAGES, 411/422 codes, dev-flag
ambiguity, layout startup check. docling-parse/qpdf stderr bypasses JSON
logging; no output observed, whether it can carry document text is unknown.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
New error codes length_required and document_too_long, 411 response,
reason ambiguous_quote.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Behind uvicorn --root-path the raw scope path carries the prefix, and the
guard would silently skip /v1/extract.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
… root path

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
- AI response invariants (found ⇒ evidence, cited segment exists, unique ids, same document) → permanent
- 400/404/405 are service errors, not document errors
- processing time budget checked against the job expiry (1 h) at worker start
- failure state recorded only while PROCESSING; constraint violations on persist are permanent
- dead-letter jobs handled every round, dead-letter queue retries its own handler
- schema: fields pinned to request (same company) and to their evidence segment

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…t concurrency test; ops notes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…ervice + tests)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…, approve/reject actions

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1
…es; refuse overlong reasons; cross-tenant correction tests

Fluory commented Sep 23, 2026

Copy link
Copy Markdown
Owner Author

Frischer Review (unabhängiger Subagent, read-only, review-pr + Security-Regel)

Ergebnis: 0 Blocker · 3 should-fix · 5 nits. Bestätigt: Tenant-Isolation (jeder Zugriff in withTenant, Firma aus Session), serverseitige Rollenprüfung in Actions und Modul, Freigabe + Export-Job + Audit in einer Transaktion unter Zeilensperre, Quellenansicht rendert nur Textknoten, CSRF über Next-Origin-Check, CI-Änderung rein additiv.

# Schwere Befund Auflösung
1 should-fix Korrigierter Wert behielt den Status found („belegt") samt altem Zitat – widerspricht „found needs proof" (D8) reviewStatus = corrected („korrigiert"), Extraktionsstatus separat als „erkannt: …"; Quellenpanel sagt, dass es die Fundstelle des erkannten Werts ist. Integrationstest „shows a corrected value as corrected – never as found"
2 should-fix ?done=/?error= wurden als Freitext in role=status/alert angezeigt (Phishing-Text per Link) Actions leiten nur feste Codes weiter; messages.ts bildet sie auf Texte ab, unbekannte Codes werden ignoriert. ReviewRefused trägt jetzt einen code
3 should-fix Kein Nachweis der RLS auf field_corrections Test „keeps corrections company-private": fremde Firma sieht keine Historie, Insert mit fremder company_id scheitert an row-level security; zusätzlich rejectRequest/correctField aus fremder Firma abgelehnt
4 nit Ungültige requestId → ZodError → 500 safeParse → notFound()
5 nit Ablehnungsgrund still auf 1000 Zeichen gekürzt wird jetzt abgelehnt (reason_too_long), Test ergänzt; DB-Check-Constraint bewusst nicht ergänzt (Modul ist der einzige Schreibpfad)
6 nit Zitat-Markierung per toLowerCase() kann bei Zeichen wie „İ" verrutschen exakter Treffer zuerst, case-insensitiver Fallback nur bei gleicher Länge, sonst ganzes Segment markiert; Unit-Test ergänzt
7 nit corrected_by ohne FK bewusst (wie audit_events.actor_user_id, Historie überlebt Nutzerentfernung) – in data-model.md dokumentiert
8 nit Personenbezogene Werte im append-only Audit nicht pro Anfrage löschbar in data-model.md und PR-Risiken vermerkt, gehört zur offenen Aufbewahrungsfrage (ADR-0001)

Nach den Fixes: Review-Tests Unit 5/5, Integration 12/12, E2E-Smoke grün; pnpm verify läuft erneut.


Generated by Claude Code

@Fluory
Fluory marked this pull request as ready for review September 23, 2026 07:08
@Fluory
Fluory changed the base branch from claude/feat-jobs-worker-7 to main September 23, 2026 10:00
@Fluory
Fluory merged commit 02b3b4e into main Sep 23, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(review): review fields beside their source, correct, approve or reject

2 participants