Skip to content

feat: immutable audit log and human-in-the-loop approval - #8

Merged
div0rce merged 6 commits into
mainfrom
feat/m07-audit-hitl
May 29, 2026
Merged

feat: immutable audit log and human-in-the-loop approval#8
div0rce merged 6 commits into
mainfrom
feat/m07-audit-hitl

Conversation

@div0rce

@div0rce div0rce commented May 29, 2026

Copy link
Copy Markdown
Owner

Milestone

M7 — Immutable audit log + human-in-the-loop approval

Summary

Adds backend/app/audit.py (semantic emitters wrapping the M1 append-only
audit_events repo, plus the replay_workflow_state primitive that the M7 DoD
keystone test consumes); wires emission into the M3/M4 extraction success path
and the M6 routing path with state-change-only semantics; introduces
backend/app/routers/review.py with GET /review and POST /review/{id}/ approve|reject for the human-in-the-loop transitions; documents the layer in
docs/audit-and-review.md; and adds 13 new tests (including the keystone
state-from-replay test) running offline against FakeLLM/FakeEmbedder.

Definition of Done

  • All DoD items from MILESTONES.md met
  • make check passes (ruff + ruff-format + mypy strict + pytest)
  • Tests added/updated for new logic — 13 new M7 tests
  • PROGRESS.md updated — M7 marked complete on branch; M6 row updated to ☑ merged
    with PR feat: deterministic idempotent workflow engine #7 link; 4 new decision-log entries
  • No secrets committed; sample data is synthetic
  • Guardrails intact (citation-or-refuse, PII redaction, confidence gating, audit
    logging) — audit logging now wired; the other three already in place from
    M3/M5/M5 respectively.

M7 DoD verification (from MILESTONES.md)

  • Every model suggestion and human decision writes exactly one audit event
    (tested).
    extract.extract_document emits extraction.created after
    persistence (failures emit nothing). workflow.route_extraction captures the
    prior status and emits workflow.routed only when apply_routing actually
    changed state (idempotent re-routes emit zero events). The /review routes
    emit exactly one event per successful 200; 4xx responses emit nothing.
  • Approve/reject transitions are valid and audited. The router scope is
    narrow: POST /review/{id}/approve|reject only accept items in needs_review
    (409 otherwise; 404 for unknown ids; 422 for missing actor). On success the
    status is updated, exactly one review.approved/review.rejected event is
    emitted with the human's actor and optional note, and the response carries
    the persisted audit_event_id.
  • State-from-replay test: current workflow_items state is reconstructable
    from audit_events. replay_workflow_state(session, workflow_item_id) walks
    every event for the target oldest-first and returns the final WorkflowStatus
    from after.status. test_state_from_replay_reconstructs_current_status
    drives a five-event lifecycle (insert → route transition → reject → reopen →
    approve) and asserts the replayed status equals the persisted one.

Notes / decisions

See PROGRESS.md "Decision log" — four M7 entries:

  • Audit-action catalogue (extraction.created, workflow.routed,
    review.approved, review.rejected) is stable; adding new actions requires an
    explicit decision.
  • Emission posture: idempotent re-routes do not double-emit; failures emit
    nothing; only state-changing operations write events.
  • replay_workflow_state reproduces current state from the audit log alone —
    the M7 keystone.
  • Review router transition surface is intentionally narrow; supervisor reversals
    live behind a future audited action.

Other deliberate choices:

  • The M1 audit_events table already had the columns M7 needs (actor,
    action, target_type, target_id, before, after, request_id, ts).
    Zero migrations in this PR.
  • Strings, not an enum, for the action catalogue so the DB doesn't need an
    enum-type migration when a new action is added.
  • The HTTP actor is a request-body field for now; M8 will derive it from
    authentication.

Schema/migration concerns

None. M7 is pure code + docs + tests on top of the existing M1 schema.

Test coverage

backend/tests/test_audit_and_review.py (12 tests):

  • Emission counts (3): one event per fresh route; zero on idempotent re-route;
    two when a re-route legitimately transitions status.
  • Review router (6): GET queue returns needs_review items; approve and
    reject both transition + audit; second approve on already-decided item is 409;
    404 for unknown id; 422 for empty actor.
  • State-from-replay (2 + 1): five-event lifecycle replays to the persisted
    status; replay returns None for an item with no events; AuditAction catalogue
    is pinned.

Total project tests: 163 (was 150 in M6).

Reminder

Please squash-merge this PR (Golden Rule #2: one milestone = one squash-merge
commit on main).

div0rce added 5 commits May 28, 2026 23:22
… primitive

backend/app/audit.py adds the M7 layer on top of the M1 append-only audit_events
table:

- AuditAction string constants for the catalogue (extraction.created,
  workflow.routed, review.approved, review.rejected); strings (not enums) so future
  actions can land without an enum migration.
- emit_extraction_created(session, extraction): records every model-produced
  Extraction with the full payload + per-field confidence + citations + model_name
  in the 'after' JSONB.
- emit_workflow_routed(session, workflow_item, prior_status): records routing
  decisions that actually changed state; idempotent re-routes do NOT emit.
- emit_review_decision(session, workflow_item, prior_status, decision, actor,
  note): records human approve/reject decisions with the human's actor and
  optional note.
- replay_workflow_state(session, workflow_item_id): walks audit events oldest-first
  and reconstructs the final WorkflowStatus from 'after.status' values. M7 keystone.

Wiring:
- extract.extract_document calls emit_extraction_created after persistence.
- workflow.route_extraction captures prior status, calls apply_routing, and emits
  workflow.routed only when the workflow_item was inserted or transitioned.

backend/app/routers/review.py:
- GET /review (limit, offset; returns needs_review queue).
- POST /review/{id}/approve -> AUTO_APPROVED + review.approved audit event.
- POST /review/{id}/reject -> REJECTED + review.rejected audit event.
- 409 on transition from any status other than needs_review (the human path is
  scoped to this transition; M6's IllegalTransition guard remains for re-routes).
- main.py includes the review router.
…ystone)

Three groups mapping onto the M7 DoD:

Emission counts (3):
- route_extraction emits exactly one workflow.routed event (before=None on insert).
- An idempotent re-route emits zero additional events (DoD: exactly one event per
  state-changing decision).
- A re-route that flips status emits a second event with before set to the prior
  status.

Review router (6):
- GET /review returns needs_review items.
- POST /review/{id}/approve transitions to AUTO_APPROVED, persists, and emits
  exactly one review.approved audit event whose actor and note match the body.
- POST /review/{id}/reject transitions to REJECTED and emits review.rejected.
- A second approve on an already-decided item is 409.
- 404 for unknown ids; 422 for empty actor.

State from replay (2 + 1):
- A multi-event lifecycle (route insert -> route transition -> reject -> reopen ->
  approve) emits 5 audit events; replay_workflow_state walks them and reproduces
  the current workflow_items.status. **M7 keystone.**
- replay_workflow_state returns None for a workflow item with no events.
- AuditAction catalogue is pinned (extraction.created, workflow.routed,
  review.approved, review.rejected).

Total project tests: 163 (was 150 in M6).
…ansitions

Documents the four-action catalogue (extraction.created, workflow.routed,
review.approved, review.rejected), the before/after JSON shapes, the emission
rules (idempotent re-routes do not double-emit, failures do not persist or emit),
the HITL transition table (only needs_review can be approved/rejected via this
router; supervisor reversals are deferred), the state-from-replay contract and
limits, and the wiring diagram.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c9450c081

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/app/workflow.py Outdated
Comment thread backend/app/routers/review.py Outdated
@div0rce

div0rce commented May 29, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Re-review PR #8 after commit 957f092 fix: make audit transitions concurrency-safe.

Focus only on the two previously reported P1 concurrency/audit findings.

Previously reported findings:

  1. workflow.routed audit emission was based on stale pre-upsert status and could emit duplicate routed events when a concurrent loser returned the item created by another worker.
  2. Review approve/reject transitions used an unconditional status update after a prior status check, allowing concurrent stale decisions to overwrite each other and emit conflicting audit events.

Verify:

  • route_extraction() emits workflow.routed only when the current call actually created a workflow item or changed its status.
  • Losing idempotent reroute/race paths return the existing item without emitting duplicate audit.
  • apply_routing() compatibility is preserved if existing callers still expect a WorkflowItem.
  • apply_routing_result() / RoutingPersistenceResult accurately reports created/changed/prior status semantics.
  • Review approve/reject uses a conditional transition from needs_review.
  • Stale/concurrent review losers return 409.
  • Failed conditional review transitions emit no audit event.
  • First successful review decision remains final and cannot be overwritten.
  • Existing workflow invariants and review router behavior remain intact.
  • New tests meaningfully cover stale loser paths, not just sequential happy paths.
  • No schema/migration changes were introduced.
  • No M8+ dashboard/frontend/eval work was introduced.

Output only:

  • BLOCKING
  • NON-BLOCKING
  • CLEANUP
  • FINAL VERDICT

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@div0rce
div0rce merged commit b85bf74 into main May 29, 2026
1 check passed
@div0rce
div0rce deleted the feat/m07-audit-hitl branch May 29, 2026 03:55
div0rce added a commit that referenced this pull request May 29, 2026
* docs(progress): record M7 merged (PR #8) and mark M8 in progress

* feat(dashboard): GET /dashboard/{volume,categories,confidence,sla} for the M8 UI

Four KPI feeds, each shaped for direct consumption by Recharts:

- /dashboard/volume?days=N: extraction count per UTC day for the last N days
  (default 30, capped 365). Backfills missing days with zero so the chart
  always has N points.
- /dashboard/categories: extraction count per schema_name, descending. Empty
  corpus returns {points: []}.
- /dashboard/confidence: histogram of every per-field confidence value across
  all extractions, in ten 0.1-wide bins covering [0, 1]. 1.0 lands in the last
  bucket. total_fields reports the population size for context.
- /dashboard/sla?threshold_hours=H: aging breakdown of needs_review items in a
  fixed coarse <1h/1-4h/4-24h/>24h scheme. over_sla counts items past the
  configurable threshold (default 24h, max 720h).

main.py wires the router. 11 tests cover happy paths, query-param validation,
empty corpus, and the response key contract the frontend's typed API client
will mirror.

* feat(frontend): scaffold Vite + React + TypeScript app (M8)

package.json, tsconfig project references (app + node), vite.config.ts (proxies
/query, /extract, /review, /dashboard to localhost:8000 in dev; vitest config
inline), index.html, frontend .gitignore, React entry (main.tsx) wrapping App in
StrictMode + BrowserRouter, top-level layout (App.tsx) with NavLink-driven
Routes for /, /review, /dashboard, plus a 404 fallback. Minimal handwritten CSS
(no framework) for legible layout, badges per workflow status, and chart-card
boxes. Vitest setup file extends jest-dom matchers.

* feat(frontend): typed API client mirroring backend Pydantic shapes

frontend/src/api.ts wraps a single fetch() helper with strongly-typed functions
that mirror the backend's response shapes one-to-one:
- postQuery / QueryResponse + Citation
- getReviewQueue / approveReview / rejectReview / ReviewItem / ReviewDecisionResponse
- getVolume / getCategories / getConfidence / getSla and matching response types

ApiError carries the HTTP status. Base URL falls back to '' so paths resolve
same-origin against the Vite proxy in dev or a reverse proxy in prod;
overridable via ?api=... or VITE_API_BASE for local debugging.

* feat(frontend): Query and Review views with loading/empty/error states

Query view: question textarea, primary Ask button (disabled when empty or
in-flight), submits to POST /query, renders the answer (or refusal text + reason)
and a citations list with chunk id, document id, score, and chunk text. State
machine: idle | loading | answered | error.

Review view: loads needs_review queue from GET /review and renders one card per
item with status badge, reason, and Approve/Reject buttons. Decisions POST to
/review/{id}/{approve|reject} with a reviewer 'actor' (default user:reviewer,
editable per session; auth-derived actor arrives later). Optimistic local
removal of the row on success, with a status flash. Shows empty state when the
queue is empty and an error banner on network/API failures.

* feat(frontend): Dashboard view with the four KPI Recharts components

Dashboard view fetches /dashboard/{volume,categories,confidence,sla} in parallel
and renders four small Recharts cards in a 2x2 grid:
- VolumeChart: per-day extraction count for the last 30 days (BarChart).
- CategoriesChart: extraction count per schema_name (horizontal BarChart).
- ConfidenceChart: per-field-confidence histogram in 10 0.1-wide bins.
- SlaChart: needs_review aging breakdown <1h/1-4h/4-24h/>24h with the
  configured threshold's over_sla count surfaced in the title.

Each chart card handles its own empty case (zero rows -> 'no data yet' message)
so the Dashboard composer can stay simple. The view itself surfaces loading
('Loading metrics…') and error states.

* test(frontend): smoke component tests for Query and Review flows

Query.test.tsx (3 tests): renders the input + disabled-by-default Ask button;
on submit with a mocked answered response renders the answer text and one
citation with chunk_id; on submit with a mocked refused response renders the
'Refused' heading and the reason string.

Review.test.tsx (3 tests): loads the queue and renders one card per item;
clicking Approve POSTs to /review/{id}/approve with the reviewer actor body and
optimistically removes the row, flashing a status; renders the empty-state when
the queue is empty.

All HTTP is mocked via vi.stubGlobal('fetch', ...). No network in CI.

* ci: add frontend job (npm ci, lint, vitest, build) parallel to backend

Splits the existing single 'check' job into two parallel jobs:

- backend (renamed from 'check'): unchanged. Postgres+pgvector service container,
  EMBEDDINGS_PROVIDER=fake, ruff + mypy + alembic upgrade + pytest + make seed.
- frontend (new): actions/setup-node@v4 with Node 20 and npm cache keyed off
  frontend/package-lock.json; runs npm ci, npm run lint (tsc --noEmit),
  npm test (vitest), npm run build (vite build).

Both jobs run on pull_request and push to main; PR is mergeable when both pass.

frontend/.gitignore now excludes *.tsbuildinfo (TypeScript incremental build
artifacts that tsc -b produces locally).

* docs(progress): mark M8 complete on branch with DoD verification and decision log

* fix(frontend): preserve query parameters across navigation

* fix: include frontend verification in local checks

* fix: use explicit UTC grouping for dashboard metrics
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.

1 participant