From e63fb966fde8483c91bb6e55b5e0931d2dbc09ad Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Sat, 8 Aug 2026 21:41:59 -0500 Subject: [PATCH 01/10] docs(recall): design entry review workflow Machine-extracted entries need a durable human disposition before they can safely become trusted or stay rejected. The design keeps that decision explicit in the review state so extraction generation changes cannot silently reverse it.\n\nThe existing SQLite CHECK makes review-state evolution an archive concern instead of a Go business rule. The approved design removes that constraint through one narrowly scoped, data-preserving migration and defines the API, UI, freshness, and failure contracts for the implementation. --- .../2026-08-08-recall-entry-review-design.md | 258 ++++++++++++++++++ 1 file changed, 258 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-08-recall-entry-review-design.md diff --git a/docs/superpowers/specs/2026-08-08-recall-entry-review-design.md b/docs/superpowers/specs/2026-08-08-recall-entry-review-design.md new file mode 100644 index 000000000..bc83cd01b --- /dev/null +++ b/docs/superpowers/specs/2026-08-08-recall-entry-review-design.md @@ -0,0 +1,258 @@ +# Recall Entry Review Design + +## Summary + +The Recall Corpus will let a user make a durable disposition on one accepted, +machine-extracted entry at a time. An expanded `unreviewed_auto` row offers two +actions: approve the entry as written or archive it as rejected. Approval is +available only while provenance remains valid; archive remains available when +provenance has been revoked. + +The decision is stored in the existing entry row. Approval produces an +accepted `human_reviewed` entry. Rejection produces an archived +`human_rejected` entry. Both review states are outside extraction lifecycle +management, so later generation activation, retirement, reconciliation, or +re-extraction cannot silently undo a human decision. + +## Goals + +- Review individual accepted machine-extracted entries from the Corpus table. +- Make approval and rejection durable across extraction lifecycle changes. +- Prevent approval when the cited provenance has been revoked. +- Keep the interaction local to the expanded row and preserve scroll position. +- Keep Recall query revisions and vector freshness correct after a decision. +- Replace the SQLite review-state constraint with Go-owned validation. + +## Non-goals + +- Editing entry titles, bodies, triggers, evidence, or transferability. +- Reviewer notes, identity, history, or a separate audit log. +- Bulk actions, undo, or restoring rejected entries. +- Reviewing staged entries from building or retired generations. +- A CLI or MCP review surface. +- Recall mutation support for PostgreSQL or DuckDB stores. +- General entry-status administration beyond this two-action workflow. + +## Approaches Considered + +### Dedicated review action with durable review states + +This is the selected approach. A purpose-built API accepts only `approve` or +`archive` and delegates to one storage transition. The entry's status records +whether it is served, while its review state records the human disposition. +Extraction already treats every state other than `unreviewed_auto` as +human-touched, so the decision remains stable without special lifecycle +exceptions. + +### Reuse reviewed import + +The import surface creates or supersedes entries and validates external +evidence payloads. Reusing it for an in-place decision would broaden a narrow +state transition into a second ingestion path and obscure conflict handling. +It is rejected. + +### Generic entry patch endpoint + +A generic patch could cover future editing, but it would expose fields and +transition combinations that are deliberately out of scope. The review action +keeps the public contract small and can be extended only when another workflow +is designed. + +## Review-State Model + +The allowed review states are defined and validated in Go: + +| Review state | Meaning | +| ----------------- | -------------------------------------------- | +| `human_reviewed` | A human approved the entry for serving | +| `human_rejected` | A human rejected and archived the entry | +| `unreviewed_auto` | Generated or omitted review decision | +| `calibrated_auto` | Automated output from a calibrated policy | +| `eval_raw` | Quarantined evaluation material | + +Approval changes only `review_state` from `unreviewed_auto` to +`human_reviewed`; status remains `accepted`. Archive changes status from +`accepted` to `archived` and review state from `unreviewed_auto` to +`human_rejected`. Both transitions update `updated_at` in the same transaction. +Entry content, evidence, provenance, transferability, source identity, and +extraction metadata do not change. + +Approval requires all of the following at commit time: + +- the entry exists; +- status is `accepted`; +- review state is `unreviewed_auto`; and +- `provenance_ok` is true. + +Archive requires the first three conditions but deliberately permits revoked +provenance. A repeated decision or any stale transition is a conflict rather +than an idempotent success. Trusted Recall remains defined as accepted, +`human_reviewed`, transferable, provenance-valid material; a rejected entry can +never become trusted because it is archived. + +## SQLite Schema Migration + +The `recall_entries.review_state` SQL `CHECK` constraint is removed from the +canonical schema. Business rules for allowed review states move to shared Go +validation used by every Recall insertion and mutation boundary. New databases +therefore have no review-state business rule embedded in SQLite. + +Existing archives receive one narrowly scoped, transactional migration of the +`recall_entries` table. It is implemented as a single immutable migration +guarded by schema inspection. The migration: + +1. detects the legacy constrained table and is a no-op for the new shape; +2. takes exclusive writer ownership during normal writable startup; +3. creates the unconstrained replacement with the canonical column shape; +4. copies every entry while preserving `rowid`, IDs, timestamps, source links, + supersession links, and all other values; +5. swaps the table, recreates its indexes and Recall entry triggers, and keeps + the external-content FTS index attached to the preserved rowids; +6. verifies row counts and `PRAGMA foreign_key_check` before commit; and +7. restores foreign-key enforcement on every success or failure path. + +No session, evidence, query measurement, extraction progress, or vector data is +discarded. The migration is not reused as an evolving compatibility path. +Read-only opens do not attempt it; the writable daemon or a normal writable +command must upgrade the archive first. + +## Storage API + +The shared store contract gains a typed review operation, conceptually: + +```go +ReviewRecallEntry( + ctx context.Context, + id string, + action RecallReviewAction, +) (RecallEntry, error) +``` + +`RecallReviewAction` accepts only `approve` and `archive`. The SQLite +implementation validates the action before opening a transaction, performs a +conditional transition, classifies a zero-row update by inspecting the current +entry inside the transaction, and returns the committed entry. PostgreSQL and +DuckDB implementations return their existing read-only error because those +stores do not own Recall mutations. + +Existing Recall entry and evidence triggers advance the ranked-query revision. +Archiving also advances the served-corpus revision and emits the existing vector +change journal entry because the entry leaves the accepted corpus. The server +notifies the Recall embedding scheduler only after a successful commit. A +failed or conflicting decision produces no scheduler notification. + +## HTTP API + +The writable server exposes: + +```text +POST /api/v1/recall/entries/{id}/review +Content-Type: application/json + +{"action":"approve"} +``` + +The only accepted action values are `approve` and `archive`. A successful +response contains the updated `RecallEntry`, allowing the UI to update the row +without refetching the current page. + +Errors follow the existing JSON API conventions: + +- `400 Bad Request` for malformed JSON or an unknown action; +- `404 Not Found` when the entry does not exist; +- `409 Conflict` when the entry is not accepted, has already received a human + disposition, or approval encounters revoked provenance; +- the existing read-only response for stores that cannot mutate Recall; and +- `503 Service Unavailable` with `Retry-After` for a closed writer or transient + maintenance condition. + +The handler does not make stale decisions appear successful. Error messages +identify the failed precondition without exposing transcript content. + +## Corpus UI + +Review controls live in the existing expanded table row. They appear only for +accepted `unreviewed_auto` entries, keeping the collapsed table dense and +leaving staged, reviewed, rejected, imported, and evaluation rows read-only. + +The expanded row presents: + +- **Approve** as the primary action; +- **Archive** as the secondary destructive action; and +- a short provenance warning when approval is disabled. + +Approve submits immediately. Archive opens the shared confirmation modal and +submits only after confirmation. While a request is in flight, both controls +are disabled and the row remains expanded. A mutation error is shown inline in +that row so other entries remain usable. + +On success, the returned entry replaces the local row. If the new status or +review state no longer matches the active filters, the row is removed locally. +The page is not reloaded, pagination is not reset, and scroll position is +preserved. The review-state filter includes localized labels for +`human_rejected` and every existing state. + +All new labels, confirmation copy, disabled explanations, busy text, and error +copy use the Paraglide message catalogues. Existing kit-ui buttons and modal +components provide interaction and styling; the feature adds no one-off control +chrome. + +## Concurrency and Failure Handling + +The database transaction is the source of truth. The UI's disabled state avoids +duplicate clicks from one browser, while the conditional update detects another +client or background mutation that wins the race. A conflict leaves the local +row unchanged and gives the user a refreshable explanation. + +Writer shutdown, maintenance, or commit failure must not report an uncommitted +decision as successful. Scheduler notification is best effort after commit: if +notification fails or the scheduler is absent, the durable mutation and its +change journal remain correct for startup or backstop reconciliation. + +## Documentation + +The Recall guide will describe the individual review workflow, the +`human_rejected` state, provenance gating, and the distinction between status +and review state. Internal extraction documentation will state explicitly that +both approved and rejected human dispositions are outside generation lifecycle +management. + +## Testing + +Tests exercise behavior rather than implementation text: + +- database tests cover approve, archive, revoked-provenance archive, rejected + approval, missing entries, repeated and stale decisions, timestamp changes, + query/corpus revision effects, and generation activation preserving both + human dispositions; +- migration tests open a legacy constrained archive containing entries, + evidence, supersession links, and Recall FTS data, then verify preservation, + foreign-key integrity, searchability, idempotence, and insertion of + `human_rejected` through the Go boundary; +- server tests cover response bodies, status mapping, `Retry-After`, read-only + behavior, and scheduler notification only after committed changes; and +- frontend component tests cover action visibility, revoked-provenance approval + gating, archive confirmation, busy and inline-error states, local row update + or removal under filters, and retained expansion/scroll behavior. + +Implementation follows red-first tests. Focused database, server, and frontend +tests run before repository formatting, linting, Go formatting/vetting, and the +broader relevant suites. + +## Acceptance Criteria + +- An accepted `unreviewed_auto` entry with valid provenance can be approved from + its expanded Corpus row and immediately becomes accepted `human_reviewed`. +- The same kind of entry can be archived after confirmation and immediately + becomes archived `human_rejected`. +- Revoked provenance disables approval but does not prevent archive. +- Later extraction activation, retirement, reconciliation, or re-extraction + does not move or delete either human-dispositioned entry. +- Concurrent or repeated decisions fail with a conflict and never overwrite the + first committed decision. +- Existing archives upgrade without losing Recall rows, evidence, FTS linkage, + source relationships, or revision integrity. +- Allowed review states are enforced in Go rather than by a SQLite `CHECK`. +- Successful mutations keep lexical, vector, hybrid, and paginated Recall reads + fresh through the existing revision and scheduler mechanisms. +- Read-only deployments do not expose enabled review controls. From 7622ab436fe55b82f50afd0efbf458df1e22768e Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Sat, 8 Aug 2026 21:53:18 -0500 Subject: [PATCH 02/10] docs(recall): plan entry review implementation The approved workflow spans a data-preserving SQLite migration, transactional review semantics, an HTTP boundary, and row-local frontend behavior. A file-specific red-first plan keeps those layers independently verifiable while preserving the clean scope of individual approve and archive decisions. --- .../plans/2026-08-08-recall-entry-review.md | 939 ++++++++++++++++++ 1 file changed, 939 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-08-recall-entry-review.md diff --git a/docs/superpowers/plans/2026-08-08-recall-entry-review.md b/docs/superpowers/plans/2026-08-08-recall-entry-review.md new file mode 100644 index 000000000..7a9f7a33d --- /dev/null +++ b/docs/superpowers/plans/2026-08-08-recall-entry-review.md @@ -0,0 +1,939 @@ +# Recall Entry Review Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:executing-plans to implement this plan directly in the current +> agent, task-by-task. Keep execution inline; do not dispatch subagents. Steps +> use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let users approve or durably reject one machine-extracted Recall entry +from its expanded Corpus row. + +**Approved spec/design:** +`docs/superpowers/specs/2026-08-08-recall-entry-review-design.md` + +**Architecture:** A one-time SQLite table migration removes the review-state +`CHECK`, while shared Go validation adds `human_rejected`. A transactional +store operation owns the legal transitions, a dedicated HTTP action exposes +it, and the Corpus panel updates the affected row locally. + +**Tech Stack:** Go, SQLite, `net/http`, testify, Svelte 5, TypeScript, +Paraglide, kit-ui, Vite+/Vitest. + +## Global Constraints + +- Use red/green TDD for every behavior change. +- Use `kenn:db-migration-discipline` before editing schema or migration code. + This PR contains exactly one immutable migration. +- The migration rebuilds only `recall_entries`, preserves `rowid`, and passes + `PRAGMA foreign_key_check` before commit. +- Do not bump parser `dataVersion`; this schema-only migration must not force a + session resync. +- Validate allowed review states in Go, not with a SQLite `CHECK`. +- Approval is `accepted/unreviewed_auto` to `accepted/human_reviewed` and + requires valid provenance. +- Archive is `accepted/unreviewed_auto` to `archived/human_rejected` and + remains available with revoked provenance. +- Treat stale, repeated, or otherwise illegal transitions as conflicts. +- Do not modify content, evidence, transferability, source identity, or + extraction metadata. +- Notify the Recall embedding scheduler only after a committed decision. +- Keep controls in the expanded row. Approve is immediate; Archive uses the + shared confirmation modal. +- Add no notes, audit table, bulk action, undo, CLI, MCP, PostgreSQL mutation, + DuckDB mutation, or generic patch endpoint. +- Use `localization-paraglide` before editing message catalogues and keep all + five locales synchronized. +- Use `kenn:commit` before every commit. Do not add attribution trailers. + +--- + +### Task 1: Move review-state validation to Go and migrate SQLite + +**Files:** + +- Modify: `internal/recall/types.go` +- Modify: `internal/recall/types_test.go` +- Modify: `internal/db/schema.sql` +- Create: `internal/db/recall_review_migration.go` +- Create: `internal/db/recall_review_migration_test.go` +- Modify: `internal/db/db.go` + +**Interface:** `recall.ReviewStateHumanRejected` is an allowed state. Writable +open upgrades the legacy constrained table before schema initialization; later +opens are no-ops. + +- [ ] **Step 1: Load migration and testing guidance** + +Read and follow `kenn:db-migration-discipline`, +`kenn:test-scope-discipline`, and `testing-without-tautologies`. Confirm this +branch contains no other schema migration. + +- [ ] **Step 2: Write the failing state-normalization test** + +Extend `TestNormalizeReviewState` in `internal/recall/types_test.go`: + +```go +{ + name: "human rejected", + value: ReviewStateHumanRejected, + wantState: ReviewStateHumanRejected, + wantOK: true, +}, +``` + +- [ ] **Step 3: Verify RED** + +```bash +CGO_ENABLED=1 go test -tags fts5 ./internal/recall \ + -run '^TestNormalizeReviewState$' -count=1 +``` + +Expected: compilation fails because `ReviewStateHumanRejected` is undefined. + +- [ ] **Step 4: Add the Go-owned state** + +Add the constant and accept it in `NormalizeReviewState`: + +```go +const ( + ReviewStateHumanReviewed = "human_reviewed" + ReviewStateHumanRejected = "human_rejected" + ReviewStateUnreviewedAuto = "unreviewed_auto" + ReviewStateCalibratedAuto = "calibrated_auto" + ReviewStateEvalRaw = "eval_raw" +) + +switch value { +case ReviewStateHumanReviewed, + ReviewStateHumanRejected, + ReviewStateUnreviewedAuto, + ReviewStateCalibratedAuto, + ReviewStateEvalRaw: + return value, true +default: + return "", false +} +``` + +Run the focused test again and expect PASS. + +- [ ] **Step 5: Write the failing legacy migration tests** + +Create `internal/db/recall_review_migration_test.go`. Build a temporary archive, +seed a session, two entries with explicit rowids, evidence, and a supersession +link, then replace only `recall_entries` with the legacy definition containing: + +```sql +review_state TEXT NOT NULL DEFAULT 'unreviewed_auto' + CHECK (review_state IN ( + 'human_reviewed', 'unreviewed_auto', 'calibrated_auto', 'eval_raw' + )) +``` + +Close the fixture, reopen with `Open(path)`, and assert rowids, entries, +evidence, supersession, FTS search, and foreign keys survive. Query +`sqlite_master.sql` and assert the migrated table contains no +`CHECK (review_state IN`. Then prove the new Go write boundary accepts +`human_rejected`: + +```go +_, err = reopened.InsertRecallEntry(RecallEntry{ + ID: "rejected", Type: "fact", Scope: "project", + Status: corerecall.StatusArchived, + ReviewState: corerecall.ReviewStateHumanRejected, + Title: "Rejected", Body: "Rejected after review.", + SourceSessionID: "session-1", +}) +require.NoError(t, err) +``` + +Add an idempotence test that calls the migration twice and proves row count, +rowids, and schema remain unchanged. Use only `t.TempDir()` archives. + +- [ ] **Step 6: Verify migration RED** + +```bash +CGO_ENABLED=1 go test -tags fts5 ./internal/db \ + -run 'Test(OpenMigratesLegacyRecallReviewConstraint|MigrateRecallReviewStateConstraintIsIdempotent)' \ + -count=1 +``` + +Expected: inserting `human_rejected` still fails the legacy SQL constraint. + +- [ ] **Step 7: Remove the canonical SQL constraint** + +Change only the `review_state` column in `internal/db/schema.sql`: + +```sql +review_state TEXT NOT NULL DEFAULT 'unreviewed_auto', +``` + +Keep the default. Do not replace the constraint or bump `dataVersion`. + +- [ ] **Step 8: Implement the pre-init migration** + +Create `migrateRecallReviewStateConstraintLocked`. It must inspect +`sqlite_master.sql`, return when the table is missing or unconstrained, pin one +writer connection, save and disable `PRAGMA foreign_keys`, and restore the +setting on every exit: + +```go +func migrateRecallReviewStateConstraintLocked(w *writerHandle) (retErr error) { + var tableSQL string + err := w.QueryRow(`SELECT sql FROM sqlite_master + WHERE type = 'table' AND name = 'recall_entries'`).Scan(&tableSQL) + if errors.Is(err, sql.ErrNoRows) { + return nil + } + if err != nil { + return fmt.Errorf("probing recall_entries review constraint: %w", err) + } + if !strings.Contains(tableSQL, "CHECK (review_state IN") { + return nil + } + + ctx := context.Background() + conn, err := w.Conn(ctx) + if err != nil { + return fmt.Errorf("acquiring recall review migration connection: %w", err) + } + defer func() { retErr = errors.Join(retErr, conn.Close()) }() + + var foreignKeys int + if err := conn.QueryRowContext(ctx, `PRAGMA foreign_keys`).Scan(&foreignKeys); err != nil { + return fmt.Errorf("reading foreign-key mode: %w", err) + } + if _, err := conn.ExecContext(ctx, `PRAGMA foreign_keys = OFF`); err != nil { + return fmt.Errorf("disabling foreign keys: %w", err) + } + defer func() { + if foreignKeys != 0 { + _, restoreErr := conn.ExecContext(ctx, `PRAGMA foreign_keys = ON`) + retErr = errors.Join(retErr, restoreErr) + } + }() + + tx, err := conn.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("beginning recall review migration: %w", err) + } + defer func() { _ = tx.Rollback() }() + if _, err := tx.ExecContext(ctx, recallReviewStateMigrationSQL); err != nil { + return fmt.Errorf("migrating recall review state: %w", err) + } + if err := verifyRecallReviewMigrationTx(ctx, tx); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("committing recall review migration: %w", err) + } + return nil +} +``` + +The prepare SQL drops entry corpus/query/FTS triggers, creates +`recall_entries_review_state_v2` with the complete canonical columns but no +review-state `CHECK`, and copies every column plus `rowid`. Before dropping the +old table, query both counts through the transaction and fail if they differ. +Then execute a separate swap statement that drops the old table and renames the +replacement. After the swap, fail if `PRAGMA foreign_key_check` yields any row. +Do not recreate indexes or triggers inside the migration: the immediately +following `db.init()` owns canonical creation, while preserved rowids keep +external-content FTS attached. + +Call it in `openAndInit`, after legacy-column repair and before `db.init()`: + +```go +db.mu.Lock() +err = migrateRecallReviewStateConstraintLocked(db.getWriter()) +db.mu.Unlock() +if err != nil { + db.Close() + return nil, fmt.Errorf("migrating recall review state: %w", err) +} +``` + +- [ ] **Step 9: Verify GREEN** + +```bash +CGO_ENABLED=1 go test -tags fts5 ./internal/recall ./internal/db -count=1 +``` + +Expected: PASS, including migration preservation, FTS, foreign-key, and +idempotence assertions. + +- [ ] **Step 10: Commit the schema checkpoint** + +Use `kenn:commit`; stage only Task 1 files and commit: + +```text +feat(recall): add durable rejected review state +``` + +--- + +### Task 2: Add the transactional store review operation + +**Files:** + +- Create: `internal/db/recall_review.go` +- Create: `internal/db/recall_review_test.go` +- Modify: `internal/db/store.go` +- Modify: `internal/postgres/store.go` +- Modify: `internal/duckdb/store.go` +- Modify: `internal/db/recall_extract_test.go` + +**Interface:** `ReviewRecallEntry(ctx, id, action)` returns the committed entry +with evidence or one typed validation, not-found, provenance, or conflict error. + +- [ ] **Step 1: Write failing transition tests** + +Create table-driven tests covering: + +```go +tests := []struct { + name string + action RecallReviewAction + provenance bool + wantStatus string + wantReview string + wantErr error +}{ + {"approve", RecallReviewApprove, true, + corerecall.StatusAccepted, corerecall.ReviewStateHumanReviewed, nil}, + {"approve revoked", RecallReviewApprove, false, + corerecall.StatusAccepted, corerecall.ReviewStateUnreviewedAuto, + ErrRecallReviewProvenance}, + {"archive", RecallReviewArchive, true, + corerecall.StatusArchived, corerecall.ReviewStateHumanRejected, nil}, + {"archive revoked", RecallReviewArchive, false, + corerecall.StatusArchived, corerecall.ReviewStateHumanRejected, nil}, +} +``` + +For success, assert unchanged content, evidence, transferability, source and +extractor fields, and `created_at`, plus a newer `updated_at`. Add cases for a +missing ID, unknown action, already reviewed/rejected, and initially archived +entry. + +- [ ] **Step 2: Add failing freshness and lifecycle coverage** + +Assert approval advances only `RecallQueryRevision`; archive advances query and +corpus revisions. Extend `TestActivateExtractGenerationSwitchesServedEntries` +with an archived `human_rejected` entry and assert activation leaves it +archived. Exercise digest-reset cleanup and assert it does not delete the row. + +- [ ] **Step 3: Verify RED** + +```bash +CGO_ENABLED=1 go test -tags fts5 ./internal/db \ + -run 'Test(ReviewRecallEntry|ActivateExtractGenerationSwitchesServedEntries)' \ + -count=1 +``` + +Expected: compilation fails because review action types are missing. + +- [ ] **Step 4: Implement typed actions, errors, and transaction** + +Create `internal/db/recall_review.go`: + +```go +type RecallReviewAction string + +const ( + RecallReviewApprove RecallReviewAction = "approve" + RecallReviewArchive RecallReviewAction = "archive" +) + +var ( + ErrInvalidRecallReviewAction = errors.New("invalid recall review action") + ErrRecallEntryNotFound = errors.New("recall entry not found") + ErrRecallReviewConflict = errors.New("recall review conflict") + ErrRecallReviewProvenance = errors.New("recall provenance is revoked") +) + +func (a RecallReviewAction) Validate() error { + switch a { + case RecallReviewApprove, RecallReviewArchive: + return nil + default: + return fmt.Errorf("%w: %q", ErrInvalidRecallReviewAction, a) + } +} +``` + +`ReviewRecallEntry` trims and validates inputs, takes `db.mu`, opens one writer +transaction, reads current status/review/provenance, classifies precondition +errors, and executes a guarded update repeating +`status='accepted' AND review_state='unreviewed_auto'`. Approval additionally +guards `provenance_ok != 0`. Assign: + +```go +nextStatus := corerecall.StatusAccepted +nextReview := corerecall.ReviewStateHumanReviewed +if action == RecallReviewArchive { + nextStatus = corerecall.StatusArchived + nextReview = corerecall.ReviewStateHumanRejected +} +``` + +Read the updated base row and its evidence through the same transaction, then +commit and return it. Do not use a post-commit lookup: a response-read failure +after commit would make the caller believe a durable mutation failed. + +- [ ] **Step 5: Extend the store contract and read-only stores** + +Add to `internal/db/store.go`: + +```go +ReviewRecallEntry( + ctx context.Context, + id string, + action RecallReviewAction, +) (RecallEntry, error) +``` + +Add matching PostgreSQL and DuckDB methods returning an empty entry and +`db.ErrReadOnly`. + +- [ ] **Step 6: Verify GREEN** + +```bash +CGO_ENABLED=1 go test -tags fts5 \ + ./internal/db ./internal/postgres ./internal/duckdb -count=1 +``` + +- [ ] **Step 7: Commit the storage checkpoint** + +Use `kenn:commit`; stage only Task 2 files and commit: + +```text +feat(recall): persist individual review decisions +``` + +--- + +### Task 3: Expose a dedicated review HTTP action + +**Files:** + +- Modify: `internal/server/server.go` +- Modify: `internal/server/recall.go` +- Modify: `internal/server/recall_test.go` + +**Interface:** `POST /api/v1/recall/entries/{id}/review` accepts one JSON action +and returns the updated entry. + +- [ ] **Step 1: Write failing success tests** + +Use the real SQLite server test environment: + +```go +func TestReviewRecallEntryApproveAndArchive(t *testing.T) { + for _, tc := range []struct { + name string + action string + wantStatus string + wantReview string + }{ + {"approve", "approve", "accepted", "human_reviewed"}, + {"archive", "archive", "archived", "human_rejected"}, + } { + t.Run(tc.name, func(t *testing.T) { + te := setup(t) + seedReviewableRecallEntry(t, te, "review-me", true) + w := te.post(t, "/api/v1/recall/entries/review-me/review", + `{"action":"`+tc.action+`"}`) + assertStatus(t, w, http.StatusOK) + got := decode[db.RecallEntry](t, w) + assert.Equal(t, tc.wantStatus, got.Status) + assert.Equal(t, tc.wantReview, got.ReviewState) + require.Len(t, got.Evidence, 1) + }) + } +} +``` + +Install `WithRecallCorpusMutationNotifier` and assert one notification after +each committed action. + +- [ ] **Step 2: Write failing error-mapping tests** + +Add cases for malformed JSON, unknown action, missing entry, already reviewed, +already rejected, initially archived, revoked approval, and revoked archive. +Assert 400 for malformed/unknown, 404 for missing, 409 for stale/repeated and +revoked approval, and 200 for revoked archive. Close the writer before a +request and assert 503 plus `Retry-After`. Use `setupPGMode` and assert the +established 501 response. Every non-2xx case must leave the notifier count zero. + +- [ ] **Step 3: Verify RED** + +```bash +CGO_ENABLED=1 go test -tags fts5 ./internal/server \ + -run 'TestReviewRecallEntry' -count=1 +``` + +Expected: 404 because the route is not registered. + +- [ ] **Step 4: Register and implement the handler** + +Register beside the existing entry routes: + +```go +s.mux.Handle("POST /api/v1/recall/entries/{id}/review", s.withTimeout( + "POST /api/v1/recall/entries/{id}/review", + s.handleReviewRecallEntry, +)) +``` + +Use a strict request body and the typed store action: + +```go +type reviewRecallEntryRequest struct { + Action db.RecallReviewAction `json:"action"` +} + +func (s *Server) handleReviewRecallEntry(w http.ResponseWriter, r *http.Request) { + var req reviewRecallEntryRequest + decoder := json.NewDecoder(r.Body) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&req); err != nil { + writeError(w, http.StatusBadRequest, "invalid JSON body") + return + } + if err := req.Action.Validate(); err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + + entry, err := s.db.ReviewRecallEntry( + r.Context(), strings.TrimSpace(r.PathValue("id")), req.Action, + ) + if err != nil { + s.handleRecallReviewError(w, err) + return + } + s.notifyRecallCorpusMutation() + writeJSON(w, http.StatusOK, entry) +} +``` + +`handleRecallReviewError` must delegate context/read-only handling first, then +map `ErrRecallEntryNotFound` to 404 and the two review-precondition errors to +409. Reuse `handleReadOnly` so `ErrWriterClosed` produces 503 with +`Retry-After`; unclassified storage failures remain 500. + +- [ ] **Step 5: Verify GREEN** + +```bash +CGO_ENABLED=1 go test -tags fts5 ./internal/server -count=1 +``` + +- [ ] **Step 6: Commit the HTTP checkpoint** + +Use `kenn:commit`; stage only Task 3 files and commit: + +```text +feat(recall): expose entry review action +``` + +--- + +### Task 4: Add the typed frontend API and localized contract + +**Files:** + +- Modify: `frontend/src/lib/api/types/recall.ts` +- Modify: `frontend/src/lib/api/recall.ts` +- Modify: `frontend/src/lib/api/recall.test.ts` +- Modify: `frontend/messages/en.json` +- Modify: `frontend/messages/de.json` +- Modify: `frontend/messages/es.json` +- Modify: `frontend/messages/fr.json` +- Modify: `frontend/messages/ja.json` + +**Interface:** `reviewRecallEntry(id, action)` posts a decision and returns a +typed `RecallEntry`; every review state and action has localized copy. + +- [ ] **Step 1: Load frontend guidance and dependencies** + +Read and follow `localization-paraglide`, `kenn:test-scope-discipline`, and +`testing-without-tautologies`. Run: + +```bash +cd frontend +vp install +``` + +- [ ] **Step 2: Write the failing API test** + +Add to `frontend/src/lib/api/recall.test.ts`: + +```ts +describe("reviewRecallEntry", () => { + it("posts one encoded review action and returns the updated entry", async () => { + const updated = { + id: "entry one", + status: "archived", + review_state: "human_rejected", + }; + const fetchMock = vi.fn().mockResolvedValue(new Response( + JSON.stringify(updated), + { status: 200, headers: { "Content-Type": "application/json" } }, + )); + vi.stubGlobal("fetch", fetchMock); + + await expect(reviewRecallEntry("entry one", "archive")) + .resolves.toEqual(updated); + expect(fetchMock).toHaveBeenCalledWith( + "/api/v1/recall/entries/entry%20one/review", + expect.objectContaining({ + method: "POST", + body: JSON.stringify({ action: "archive" }), + }), + ); + }); +}); +``` + +- [ ] **Step 3: Verify RED** + +```bash +cd frontend +vp test run src/lib/api/recall.test.ts +``` + +Expected: compilation fails because `reviewRecallEntry` is absent. + +- [ ] **Step 4: Add the typed action and API function** + +In `frontend/src/lib/api/types/recall.ts`: + +```ts +export type RecallReviewAction = "approve" | "archive"; +``` + +In `frontend/src/lib/api/recall.ts`: + +```ts +export async function reviewRecallEntry( + id: string, + action: RecallReviewAction, +): Promise { + const response = await fetch( + `${getBase()}/recall/entries/${encodeURIComponent(id)}/review`, + authHeaders({ + method: "POST", + body: JSON.stringify({ action }), + }), + ); + if (!response.ok) { + throw new ApiError(response.status, await responseErrorMessage(response)); + } + return (await response.json()) as RecallEntry; +} +``` + +Import the new type with `import type`. Run the API test and expect PASS. + +- [ ] **Step 5: Add synchronized localized copy** + +Add these keys with real translations to all five locale files: + +```text +recall_page_review_state_human_reviewed +recall_page_review_state_human_rejected +recall_page_review_state_unreviewed_auto +recall_page_review_state_calibrated_auto +recall_page_review_state_eval_raw +recall_page_review_approve +recall_page_review_archive +recall_page_review_approve_disabled +recall_page_review_archive_title +recall_page_review_archive_message +recall_page_review_cancel +recall_page_review_close +recall_page_review_error +``` + +English source copy: + +```json +"recall_page_review_state_human_reviewed": "Human approved", +"recall_page_review_state_human_rejected": "Human rejected", +"recall_page_review_state_unreviewed_auto": "Unreviewed automatic", +"recall_page_review_state_calibrated_auto": "Calibrated automatic", +"recall_page_review_state_eval_raw": "Evaluation raw", +"recall_page_review_approve": "Approve", +"recall_page_review_archive": "Archive", +"recall_page_review_approve_disabled": "Approval is unavailable because the source evidence was revoked.", +"recall_page_review_archive_title": "Archive Recall entry", +"recall_page_review_archive_message": "Archive “{title}” as rejected? It will remain outside the served Recall corpus.", +"recall_page_review_cancel": "Cancel", +"recall_page_review_close": "Close archive confirmation", +"recall_page_review_error": "Could not review this Recall entry: {error}" +``` + +Compile and type-check: + +```bash +cd frontend +vp run i18n:compile +vp check +``` + +- [ ] **Step 6: Commit the frontend contract checkpoint** + +Use `kenn:commit`; stage only Task 4 files and commit: + +```text +feat(recall): add localized entry review contract +``` + +--- + +### Task 5: Wire review controls into the expanded Corpus row + +**Files:** + +- Modify: `frontend/src/lib/components/recall/RecallCorpusPanel.svelte` +- Modify: `frontend/src/lib/components/recall/RecallCorpusPanel.test.ts` + +**Interface:** Expanded accepted `unreviewed_auto` rows offer Approve and +Archive; the response updates or removes only that local row. + +- [ ] **Step 1: Write failing visibility and provenance tests** + +Extend the component fixture with a valid reviewable entry, a revoked +reviewable entry, and a `human_reviewed` entry. Expand each and assert: + +- the valid automatic row has enabled Approve and Archive buttons; +- the revoked automatic row has disabled Approve, the localized explanation, + and enabled Archive; and +- the human-reviewed row has neither action. + +Select controls by accessible name rather than CSS implementation details. + +- [ ] **Step 2: Write failing approve interaction tests** + +Click Approve and assert the exact POST body, retained expanded row, localized +`human_reviewed` label, and unchanged scroll position. Activate the +`unreviewed_auto` filter in a second test, approve, and assert the row disappears +without another list request. + +- [ ] **Step 3: Write failing archive, busy, and error tests** + +Assert the first Archive click opens the shared modal without sending a +request, Cancel closes it, and confirmation sends `{"action":"archive"}`. +Success removes the row because the browser defaults to accepted entries. +While the promise is pending, assert both row actions are disabled. Return a +409 and assert the expanded row remains with a localized inline error. + +- [ ] **Step 4: Verify RED** + +```bash +cd frontend +vp test run src/lib/components/recall/RecallCorpusPanel.test.ts +``` + +Expected: review controls and confirmation are absent. + +- [ ] **Step 5: Add localized review-state labels** + +Include `human_rejected` in `REVIEW_STATES` and add: + +```ts +function reviewStateLabel(state: string): string { + switch (state) { + case "human_reviewed": + return m.recall_page_review_state_human_reviewed(); + case "human_rejected": + return m.recall_page_review_state_human_rejected(); + case "unreviewed_auto": + return m.recall_page_review_state_unreviewed_auto(); + case "calibrated_auto": + return m.recall_page_review_state_calibrated_auto(); + case "eval_raw": + return m.recall_page_review_state_eval_raw(); + default: + return state; + } +} +``` + +Use it in filter options and table cells. + +- [ ] **Step 6: Implement local review state and mutation flow** + +Import `reviewRecallEntry` and `RecallReviewAction`. Add row-scoped state: + +```ts +let reviewingEntryIds = $state([]); +let reviewErrors = $state>({}); +let archiveEntry = $state(null); + +function isReviewable(entry: RecallEntry): boolean { + return entry.status === "accepted" && + entry.review_state === "unreviewed_auto"; +} + +function keepAfterReview(entry: RecallEntry): boolean { + return entry.status === "accepted" && + (!reviewState || entry.review_state === reviewState); +} + +async function submitReview( + entry: RecallEntry, + action: RecallReviewAction, +) { + if (reviewingEntryIds.includes(entry.id)) return; + reviewingEntryIds = [...reviewingEntryIds, entry.id]; + reviewErrors = { ...reviewErrors, [entry.id]: "" }; + try { + const updated = await reviewRecallEntry(entry.id, action); + const keep = keepAfterReview(updated); + entries = keep + ? entries.map((item) => item.id === updated.id ? updated : item) + : entries.filter((item) => item.id !== updated.id); + if (!keep) { + expandedEntryIds = expandedEntryIds.filter((id) => id !== updated.id); + } + archiveEntry = null; + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + reviewErrors = { + ...reviewErrors, + [entry.id]: m.recall_page_review_error({ error: detail }), + }; + } finally { + reviewingEntryIds = reviewingEntryIds.filter((id) => id !== entry.id); + } +} +``` + +Do not call `loadEntries` after success. Local replacement/removal preserves +pagination and scroll. + +- [ ] **Step 7: Render actions and the archive modal** + +Inside `.entry-detail`, after evidence, render actions only when +`isReviewable(entry)`. Approve calls `submitReview` immediately and is disabled +when provenance is invalid or `reviewingEntryIds` contains the row. Archive +sets `archiveEntry` and remains enabled for revoked provenance. Render the +localized provenance reason and row error beside the controls. + +Add one `Modal` outside the table following the existing generation modal. Its +confirmation calls: + +```ts +if (archiveEntry) void submitReview(archiveEntry, "archive"); +``` + +Use kit-ui `Button` and `Modal`. Add only layout CSS using existing spacing, +border, and danger tokens; add no native control or one-off button chrome. + +- [ ] **Step 8: Verify GREEN** + +```bash +cd frontend +vp test run src/lib/api/recall.test.ts \ + src/lib/components/recall/RecallCorpusPanel.test.ts +vp check +``` + +- [ ] **Step 9: Commit the UI checkpoint** + +Use `kenn:commit`; stage only Task 5 files and commit: + +```text +feat(recall): review entries from the corpus table +``` + +--- + +### Task 6: Document the workflow and run final verification + +**Files:** + +- Modify: `docs/recall.md` +- Modify: `docs/internal/recall-extraction.md` +- Modify as required by formatters: touched Go and frontend files only + +- [ ] **Step 1: Update public Recall documentation** + +In `docs/recall.md`, add `human_rejected` and correct the state meanings: + +```markdown +| `human_reviewed` | Explicitly approved by a human | +| `human_rejected` | Explicitly rejected and archived by a human | +| `unreviewed_auto` | Generated or omitted review decision | +| `calibrated_auto` | Automated output from a calibrated future policy | +| `eval_raw` | Quarantined evaluation material | +``` + +Add a concise “Review extracted entries” subsection explaining expanded-row +Approve/Archive behavior, provenance gating, confirmation, and the absence of +editing, bulk actions, and undo. + +- [ ] **Step 2: Update extraction lifecycle documentation** + +In `docs/internal/recall-extraction.md`, state that `human_reviewed` and +`human_rejected` are human-touched states excluded from activation, retirement, +retraction, and digest-reset cleanup. Rejected entries stay archived across +later generation activation. + +- [ ] **Step 3: Format and run focused verification** + +```bash +mdformat --wrap 80 docs/recall.md docs/internal/recall-extraction.md \ + docs/superpowers/specs/2026-08-08-recall-entry-review-design.md \ + docs/superpowers/plans/2026-08-08-recall-entry-review.md +go fmt ./... +CGO_ENABLED=1 go test -tags fts5 \ + ./internal/recall ./internal/db ./internal/postgres \ + ./internal/duckdb ./internal/server -count=1 +go vet ./... +make lint-golangci-ci +cd frontend +vp run i18n:compile +vp test run src/lib/api/recall.test.ts \ + src/lib/components/recall/RecallCorpusPanel.test.ts +vp check +``` + +Expected: every command exits zero. If `mdformat` is unavailable, preserve the +80-column style manually and report that limitation. + +- [ ] **Step 4: Inspect the final diff and public-data boundary** + +```bash +git diff --check +git status --short +git diff --stat origin/main...HEAD +git diff origin/main...HEAD +``` + +Confirm there are no private hostnames, personal paths, lab data, generated site +output, `.superpowers/` files, unrelated changes, or attribution trailers. + +- [ ] **Step 5: Commit the documentation checkpoint** + +Use `kenn:commit`; commit only documentation or formatter changes not already +committed: + +```text +docs(recall): explain entry review decisions +``` + +- [ ] **Step 6: Record kata evidence** + +Comment on `9bds` with focused commands, commit hashes, and migration +preservation evidence. Keep it open until a pull request exists and final +verification has passed. + +- [ ] **Step 7: Prepare delivery without watching CI** + +Use `superpowers:verification-before-completion`, then +`superpowers:finishing-a-development-branch`. Do not push or open a pull request +unless requested. Never poll GitHub Actions or use `gh api` to watch jobs unless +explicitly requested. From 050bf1eb5b3649018c909a8ec3d8bef8464f84b0 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Sat, 8 Aug 2026 22:16:58 -0500 Subject: [PATCH 03/10] feat(recall): add durable rejected review state Recall review needs a terminal rejection state that survives reopen and future state additions. Keeping the allowed review-state enum in SQLite made each new business state require a table change, so policy now lives at the shared Go write boundary.\n\nExisting archives are rebuilt transactionally before schema initialization while preserving row IDs and relationships needed by evidence, supersession, and FTS. --- internal/db/db.go | 10 ++ internal/db/recall_review_migration.go | 170 ++++++++++++++++++++ internal/db/recall_review_migration_test.go | 168 +++++++++++++++++++ internal/db/schema.sql | 5 +- internal/recall/types.go | 2 + internal/recall/types_test.go | 6 + 6 files changed, 357 insertions(+), 4 deletions(-) create mode 100644 internal/db/recall_review_migration.go create mode 100644 internal/db/recall_review_migration_test.go diff --git a/internal/db/db.go b/internal/db/db.go index e9cdbcb77..b46a3d99c 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -3719,6 +3719,16 @@ func openAndInit(path string, schemaRepairNeeded bool) (*DB, error) { } } + db.mu.Lock() + err = migrateRecallReviewStateConstraintLocked(db.getWriter()) + db.mu.Unlock() + if err != nil { + db.Close() + return nil, fmt.Errorf( + "migrating recall review state: %w", err, + ) + } + if err := db.init(); err != nil { db.Close() return nil, fmt.Errorf("initializing schema: %w", err) diff --git a/internal/db/recall_review_migration.go b/internal/db/recall_review_migration.go new file mode 100644 index 000000000..3293a1f99 --- /dev/null +++ b/internal/db/recall_review_migration.go @@ -0,0 +1,170 @@ +package db + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" +) + +// migrateRecallReviewStateConstraintLocked removes the legacy SQL enum from +// recall_entries. Review-state policy belongs to the shared Go write boundary; +// keeping it in the table would require a table migration for every new state. +// The caller must hold db.mu and invoke this before schema initialization so +// schema.sql can recreate the dropped indexes and triggers canonically. +func migrateRecallReviewStateConstraintLocked( + w *writerHandle, +) (retErr error) { + var tableSQL string + err := w.QueryRow(` + SELECT sql FROM sqlite_master + WHERE type = 'table' AND name = 'recall_entries' + `).Scan(&tableSQL) + if errors.Is(err, sql.ErrNoRows) { + return nil + } + if err != nil { + return fmt.Errorf( + "probing recall_entries review constraint: %w", err, + ) + } + if !strings.Contains(tableSQL, "CHECK (review_state IN") { + return nil + } + + ctx := context.Background() + conn, err := w.Conn(ctx) + if err != nil { + return fmt.Errorf( + "acquiring recall review migration connection: %w", err, + ) + } + defer func() { + if err := conn.Close(); err != nil { + retErr = errors.Join(retErr, err) + } + }() + + var foreignKeys int + if err := conn.QueryRowContext( + ctx, `PRAGMA foreign_keys`, + ).Scan(&foreignKeys); err != nil { + return fmt.Errorf("reading foreign-key mode: %w", err) + } + if _, err := conn.ExecContext( + ctx, `PRAGMA foreign_keys = OFF`, + ); err != nil { + return fmt.Errorf("disabling foreign keys: %w", err) + } + defer func() { + if foreignKeys == 0 { + return + } + if _, err := conn.ExecContext( + ctx, `PRAGMA foreign_keys = ON`, + ); err != nil { + retErr = errors.Join(retErr, + fmt.Errorf("restoring foreign keys: %w", err)) + } + }() + + tx, err := conn.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("beginning recall review migration: %w", err) + } + defer func() { _ = tx.Rollback() }() + + if _, err := tx.ExecContext( + ctx, recallReviewStateMigrationPrepareSQL, + ); err != nil { + return fmt.Errorf("preparing recall review migration: %w", err) + } + var sourceCount, replacementCount int64 + if err := tx.QueryRowContext(ctx, ` + SELECT + (SELECT count(*) FROM recall_entries), + (SELECT count(*) FROM recall_entries_review_state_v2) + `).Scan(&sourceCount, &replacementCount); err != nil { + return fmt.Errorf("counting migrated recall entries: %w", err) + } + if sourceCount != replacementCount { + return fmt.Errorf( + "migrating recall review state copied %d of %d entries", + replacementCount, sourceCount, + ) + } + if _, err := tx.ExecContext( + ctx, recallReviewStateMigrationSwapSQL, + ); err != nil { + return fmt.Errorf("swapping migrated recall entries: %w", err) + } + + rows, err := tx.QueryContext(ctx, `PRAGMA foreign_key_check`) + if err != nil { + return fmt.Errorf("checking migrated recall foreign keys: %w", err) + } + broken := rows.Next() + if err := rows.Close(); err != nil { + return fmt.Errorf("closing recall foreign-key check: %w", err) + } + if broken { + return errors.New("migrated recall entries failed foreign-key check") + } + + if err := tx.Commit(); err != nil { + return fmt.Errorf("committing recall review migration: %w", err) + } + return nil +} + +const recallReviewStateMigrationPrepareSQL = ` +CREATE TABLE recall_entries_review_state_v2 ( + id TEXT PRIMARY KEY, + type TEXT NOT NULL, + scope TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'accepted', + review_state TEXT NOT NULL DEFAULT 'unreviewed_auto', + title TEXT NOT NULL, + body TEXT NOT NULL, + trigger TEXT NOT NULL DEFAULT '', + confidence REAL, + uncertainty TEXT NOT NULL DEFAULT '', + project TEXT NOT NULL DEFAULT '', + cwd TEXT NOT NULL DEFAULT '', + git_branch TEXT NOT NULL DEFAULT '', + agent TEXT NOT NULL DEFAULT '', + source_session_id TEXT NOT NULL REFERENCES sessions(id) ON DELETE CASCADE, + source_episode_id TEXT NOT NULL DEFAULT '', + source_run_id TEXT NOT NULL DEFAULT '', + extractor_method TEXT NOT NULL DEFAULT '', + model TEXT NOT NULL DEFAULT '', + transferable INTEGER NOT NULL DEFAULT 0, + provenance_ok INTEGER NOT NULL DEFAULT 0, + supersedes_entry_id TEXT NOT NULL DEFAULT '', + superseded_by_entry_id TEXT NOT NULL DEFAULT '', + created_at TEXT NOT NULL + DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), + updated_at TEXT NOT NULL + DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')) +); +INSERT INTO recall_entries_review_state_v2 ( + rowid, id, type, scope, status, review_state, title, body, trigger, + confidence, uncertainty, project, cwd, git_branch, agent, + source_session_id, source_episode_id, source_run_id, extractor_method, + model, transferable, provenance_ok, supersedes_entry_id, + superseded_by_entry_id, created_at, updated_at +) +SELECT + rowid, id, type, scope, status, review_state, title, body, trigger, + confidence, uncertainty, project, cwd, git_branch, agent, + source_session_id, source_episode_id, source_run_id, extractor_method, + model, transferable, provenance_ok, supersedes_entry_id, + superseded_by_entry_id, created_at, updated_at +FROM recall_entries; +` + +const recallReviewStateMigrationSwapSQL = ` +DROP TABLE recall_entries; +ALTER TABLE recall_entries_review_state_v2 RENAME TO recall_entries; +` diff --git a/internal/db/recall_review_migration_test.go b/internal/db/recall_review_migration_test.go new file mode 100644 index 000000000..41bbaf224 --- /dev/null +++ b/internal/db/recall_review_migration_test.go @@ -0,0 +1,168 @@ +package db + +import ( + "context" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + corerecall "go.kenn.io/agentsview/internal/recall" +) + +func TestOpenMigratesLegacyRecallReviewConstraint(t *testing.T) { + d := testDB(t) + path := d.Path() + insertSession(t, d, "review-migration-session", "agentsview") + + _, err := d.InsertRecallEntry(RecallEntry{ + ID: "legacy-old", Type: "fact", Scope: "project", + Status: corerecall.StatusArchived, + ReviewState: corerecall.ReviewStateHumanReviewed, + Title: "Old preserved entry", Body: "Preserved before replacement.", + SourceSessionID: "review-migration-session", + SupersededByEntryID: "legacy-new", + }) + require.NoError(t, err) + _, err = d.InsertRecallEntry(RecallEntry{ + ID: "legacy-new", Type: "fact", Scope: "project", + Status: corerecall.StatusAccepted, + ReviewState: corerecall.ReviewStateUnreviewedAuto, + Title: "Migration marker", Body: "preservedmarker remains searchable", + SourceSessionID: "review-migration-session", + SupersedesEntryID: "legacy-old", + ProvenanceOK: true, + Evidence: []RecallEvidence{{ + SessionID: "review-migration-session", + MessageStartOrdinal: 2, + MessageEndOrdinal: 4, + Snippet: "preserved migration evidence", + }}, + }) + require.NoError(t, err) + + wantRowIDs := make(map[string]int64, 2) + for _, id := range []string{"legacy-old", "legacy-new"} { + var rowID int64 + require.NoError(t, d.getReader().QueryRow( + `SELECT rowid FROM recall_entries WHERE id = ?`, id, + ).Scan(&rowID)) + wantRowIDs[id] = rowID + } + + conn, err := d.getWriter().Conn(context.Background()) + require.NoError(t, err) + _, err = conn.ExecContext(context.Background(), `PRAGMA foreign_keys = OFF`) + require.NoError(t, err) + tx, err := conn.BeginTx(context.Background(), nil) + require.NoError(t, err) + _, err = tx.Exec(` + CREATE TABLE recall_entries_legacy_review_check ( + id TEXT PRIMARY KEY, + type TEXT NOT NULL, + scope TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'accepted', + review_state TEXT NOT NULL DEFAULT 'unreviewed_auto' + CHECK (review_state IN ( + 'human_reviewed', 'unreviewed_auto', + 'calibrated_auto', 'eval_raw' + )), + title TEXT NOT NULL, + body TEXT NOT NULL, + trigger TEXT NOT NULL DEFAULT '', + confidence REAL, + uncertainty TEXT NOT NULL DEFAULT '', + project TEXT NOT NULL DEFAULT '', + cwd TEXT NOT NULL DEFAULT '', + git_branch TEXT NOT NULL DEFAULT '', + agent TEXT NOT NULL DEFAULT '', + source_session_id TEXT NOT NULL + REFERENCES sessions(id) ON DELETE CASCADE, + source_episode_id TEXT NOT NULL DEFAULT '', + source_run_id TEXT NOT NULL DEFAULT '', + extractor_method TEXT NOT NULL DEFAULT '', + model TEXT NOT NULL DEFAULT '', + transferable INTEGER NOT NULL DEFAULT 0, + provenance_ok INTEGER NOT NULL DEFAULT 0, + supersedes_entry_id TEXT NOT NULL DEFAULT '', + superseded_by_entry_id TEXT NOT NULL DEFAULT '', + created_at TEXT NOT NULL + DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), + updated_at TEXT NOT NULL + DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')) + ); + INSERT INTO recall_entries_legacy_review_check ( + rowid, ` + recallBaseCols + ` + ) SELECT rowid, ` + recallBaseCols + ` FROM recall_entries; + DROP TABLE recall_entries; + ALTER TABLE recall_entries_legacy_review_check RENAME TO recall_entries; + `) + require.NoError(t, err) + require.NoError(t, tx.Commit()) + _, err = conn.ExecContext(context.Background(), `PRAGMA foreign_keys = ON`) + require.NoError(t, err) + require.NoError(t, conn.Close()) + require.NoError(t, d.Close()) + + for pass := 0; pass < 2; pass++ { + reopened, err := Open(path) + require.NoError(t, err) + + for id, wantRowID := range wantRowIDs { + var gotRowID int64 + require.NoError(t, reopened.getReader().QueryRow( + `SELECT rowid FROM recall_entries WHERE id = ?`, id, + ).Scan(&gotRowID)) + assert.Equal(t, wantRowID, gotRowID) + } + + got, err := reopened.GetRecallEntry(context.Background(), "legacy-new") + require.NoError(t, err) + require.NotNil(t, got) + require.Len(t, got.Evidence, 1) + assert.Equal(t, "legacy-old", got.SupersedesEntryID) + assert.Equal(t, "preserved migration evidence", got.Evidence[0].Snippet) + + matches, err := reopened.QueryRecallEntries(context.Background(), RecallQuery{ + Text: "preservedmarker", + Limit: 10, + }) + require.NoError(t, err) + require.Len(t, matches.RecallEntries, 1) + assert.Equal(t, "legacy-new", matches.RecallEntries[0].ID) + + var tableSQL string + require.NoError(t, reopened.getReader().QueryRow(` + SELECT sql FROM sqlite_master + WHERE type = 'table' AND name = 'recall_entries' + `).Scan(&tableSQL)) + assert.NotContains(t, tableSQL, "CHECK (review_state IN") + + rows, err := reopened.getReader().Query(`PRAGMA foreign_key_check`) + require.NoError(t, err) + assert.False(t, rows.Next()) + require.NoError(t, rows.Close()) + + if pass == 0 { + _, err = reopened.InsertRecallEntry(RecallEntry{ + ID: "rejected", Type: "fact", Scope: "project", + Status: corerecall.StatusArchived, + ReviewState: corerecall.ReviewStateHumanRejected, + Title: "Rejected", Body: "Rejected after review.", + SourceSessionID: "review-migration-session", + }) + require.NoError(t, err) + } + require.NoError(t, reopened.Close()) + } + + reopened, err := Open(path) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, reopened.Close()) }) + got, err := reopened.GetRecallEntry(context.Background(), "rejected") + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, corerecall.ReviewStateHumanRejected, got.ReviewState) + assert.True(t, strings.Contains(got.Body, "Rejected")) +} diff --git a/internal/db/schema.sql b/internal/db/schema.sql index 745a54dd8..02f8cc40e 100644 --- a/internal/db/schema.sql +++ b/internal/db/schema.sql @@ -391,10 +391,7 @@ CREATE TABLE IF NOT EXISTS recall_entries ( type TEXT NOT NULL, scope TEXT NOT NULL, status TEXT NOT NULL DEFAULT 'accepted', - review_state TEXT NOT NULL DEFAULT 'unreviewed_auto' - CHECK (review_state IN ( - 'human_reviewed', 'unreviewed_auto', 'calibrated_auto', 'eval_raw' - )), + review_state TEXT NOT NULL DEFAULT 'unreviewed_auto', title TEXT NOT NULL, body TEXT NOT NULL, trigger TEXT NOT NULL DEFAULT '', diff --git a/internal/recall/types.go b/internal/recall/types.go index e2cea2d21..3adc0c9f9 100644 --- a/internal/recall/types.go +++ b/internal/recall/types.go @@ -34,6 +34,7 @@ const LexicalScorePolicyVersion = "recall-lexical-v1" const ( ReviewStateHumanReviewed = "human_reviewed" + ReviewStateHumanRejected = "human_rejected" ReviewStateUnreviewedAuto = "unreviewed_auto" ReviewStateCalibratedAuto = "calibrated_auto" ReviewStateEvalRaw = "eval_raw" @@ -49,6 +50,7 @@ func NormalizeReviewState(value string) (string, bool) { } switch value { case ReviewStateHumanReviewed, + ReviewStateHumanRejected, ReviewStateUnreviewedAuto, ReviewStateCalibratedAuto, ReviewStateEvalRaw: diff --git a/internal/recall/types_test.go b/internal/recall/types_test.go index 5e2223dab..f0fe5315b 100644 --- a/internal/recall/types_test.go +++ b/internal/recall/types_test.go @@ -31,6 +31,12 @@ func TestNormalizeReviewState(t *testing.T) { wantState: ReviewStateHumanReviewed, wantOK: true, }, + { + name: "human rejected remains explicit", + value: ReviewStateHumanRejected, + wantState: ReviewStateHumanRejected, + wantOK: true, + }, { name: "unknown state is rejected", value: "self_approved", From 289f680661dae8375a2af2f42b880a7d8d2d46aa Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Sat, 8 Aug 2026 22:19:15 -0500 Subject: [PATCH 04/10] feat(recall): persist individual review decisions Machine-generated Recall entries need a one-way human disposition that extraction maintenance cannot reverse. The store now enforces that only accepted, unreviewed automatic entries can be approved or archived, with provenance required for approval.\n\nReturning the hydrated entry from the same transaction keeps the API response aligned with the committed decision, while SQLite triggers continue to invalidate only the query and embedding views affected by each transition. --- internal/db/recall_extract_test.go | 8 + internal/db/recall_review.go | 155 +++++++++++++++++++ internal/db/recall_review_test.go | 231 +++++++++++++++++++++++++++++ internal/db/store.go | 3 + internal/duckdb/store.go | 6 + internal/postgres/store.go | 6 + 6 files changed, 409 insertions(+) create mode 100644 internal/db/recall_review.go create mode 100644 internal/db/recall_review_test.go diff --git a/internal/db/recall_extract_test.go b/internal/db/recall_extract_test.go index 0149ceb63..afeaa9b49 100644 --- a/internal/db/recall_extract_test.go +++ b/internal/db/recall_extract_test.go @@ -1664,6 +1664,7 @@ func TestActivateExtractGenerationSwitchesServedEntries(t *testing.T) { entry("e-old", "fp-old", "archived", "unreviewed_auto"), entry("e-new-staged", "fp-new", "archived", "unreviewed_auto"), entry("e-reviewed", "fp-old", "accepted", "human_reviewed"), + entry("e-rejected", "fp-old", "archived", "human_rejected"), }) require.NoError(t, err) require.NoError(t, d.ActivateExtractGeneration( @@ -1684,6 +1685,8 @@ func TestActivateExtractGenerationSwitchesServedEntries(t *testing.T) { "activation must stop serving the retired generation's entries") assert.Equal(t, "accepted", status("e-reviewed"), "human-reviewed entries are not lifecycle-managed") + assert.Equal(t, "archived", status("e-rejected"), + "human-rejected entries are not lifecycle-managed") } func TestRetireExtractGenerationArchivesServedEntries(t *testing.T) { @@ -3054,6 +3057,7 @@ func TestUpsertExtractProgressDigestChangeRemovesEntriesAtomically(t *testing.T) _, err = d.InsertExtractedRecallEntries(ctx, []RecallEntry{ machineEntry("e-1", "unreviewed_auto"), machineEntry("e-human", "human_reviewed"), + machineEntry("e-rejected", "human_rejected"), }) require.NoError(t, err) @@ -3083,6 +3087,10 @@ func TestUpsertExtractProgressDigestChangeRemovesEntriesAtomically(t *testing.T) human, err := d.GetRecallEntry(ctx, "e-human") require.NoError(t, err) require.NotNil(t, human, "human-touched entries are never machine-deleted") + rejected, err := d.GetRecallEntry(ctx, "e-rejected") + require.NoError(t, err) + require.NotNil(t, rejected, + "human-rejected entries are never machine-deleted") // A refused progress write rolls the entry delete back with it. _, err = d.InsertExtractedRecallEntries(ctx, []RecallEntry{ diff --git a/internal/db/recall_review.go b/internal/db/recall_review.go new file mode 100644 index 000000000..eca05e2d1 --- /dev/null +++ b/internal/db/recall_review.go @@ -0,0 +1,155 @@ +package db + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" + + corerecall "go.kenn.io/agentsview/internal/recall" +) + +type RecallReviewAction string + +const ( + RecallReviewApprove RecallReviewAction = "approve" + RecallReviewArchive RecallReviewAction = "archive" +) + +var ( + ErrInvalidRecallReviewAction = errors.New("invalid recall review action") + ErrRecallEntryNotFound = errors.New("recall entry not found") + ErrRecallReviewConflict = errors.New("recall review conflict") + ErrRecallReviewProvenance = errors.New("recall provenance is revoked") +) + +func (a RecallReviewAction) Validate() error { + switch a { + case RecallReviewApprove, RecallReviewArchive: + return nil + default: + return fmt.Errorf("%w: %q", ErrInvalidRecallReviewAction, a) + } +} + +// ReviewRecallEntry records a terminal human decision for one automatic +// entry. The transition and returned representation share one transaction so +// callers never receive a failure after a decision was durably committed. +func (db *DB) ReviewRecallEntry( + ctx context.Context, + id string, + action RecallReviewAction, +) (RecallEntry, error) { + if err := db.requireWritable(); err != nil { + return RecallEntry{}, err + } + id = strings.TrimSpace(id) + action = RecallReviewAction(strings.TrimSpace(string(action))) + if err := action.Validate(); err != nil { + return RecallEntry{}, err + } + if id == "" { + return RecallEntry{}, ErrRecallEntryNotFound + } + if ctx == nil { + ctx = context.Background() + } + + db.mu.Lock() + defer db.mu.Unlock() + tx, err := db.getWriter().BeginTx(ctx, nil) + if err != nil { + return RecallEntry{}, fmt.Errorf("begin recall review: %w", err) + } + defer func() { _ = tx.Rollback() }() + + var status, reviewState string + var provenanceOK bool + err = tx.QueryRowContext(ctx, ` + SELECT status, review_state, provenance_ok + FROM recall_entries WHERE id = ?`, id, + ).Scan(&status, &reviewState, &provenanceOK) + if errors.Is(err, sql.ErrNoRows) { + return RecallEntry{}, fmt.Errorf("%w: %s", ErrRecallEntryNotFound, id) + } + if err != nil { + return RecallEntry{}, fmt.Errorf("read recall review state: %w", err) + } + if status != corerecall.StatusAccepted || + reviewState != corerecall.ReviewStateUnreviewedAuto { + return RecallEntry{}, fmt.Errorf( + "%w: entry %s is %s/%s", + ErrRecallReviewConflict, id, status, reviewState, + ) + } + if action == RecallReviewApprove && !provenanceOK { + return RecallEntry{}, fmt.Errorf("%w: entry %s", ErrRecallReviewProvenance, id) + } + + nextStatus := corerecall.StatusAccepted + nextReview := corerecall.ReviewStateHumanReviewed + guard := "" + if action == RecallReviewArchive { + nextStatus = corerecall.StatusArchived + nextReview = corerecall.ReviewStateHumanRejected + } else { + guard = " AND provenance_ok != 0" + } + result, err := tx.ExecContext(ctx, ` + UPDATE recall_entries + SET status = ?, review_state = ?, + updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') + WHERE id = ? AND status = ? AND review_state = ?`+guard, + nextStatus, nextReview, id, corerecall.StatusAccepted, + corerecall.ReviewStateUnreviewedAuto, + ) + if err != nil { + return RecallEntry{}, fmt.Errorf("update recall review: %w", err) + } + affected, err := result.RowsAffected() + if err != nil { + return RecallEntry{}, fmt.Errorf("count recall review update: %w", err) + } + if affected != 1 { + return RecallEntry{}, fmt.Errorf( + "%w: entry %s changed during review", ErrRecallReviewConflict, id, + ) + } + + entry, err := scanRecallEntryRow(tx.QueryRowContext(ctx, + "SELECT "+recallBaseCols+" FROM recall_entries WHERE id = ?", id)) + if err != nil { + return RecallEntry{}, fmt.Errorf("read reviewed recall entry: %w", err) + } + rows, err := tx.QueryContext(ctx, ` + SELECT id, entry_id, session_id, message_start_ordinal, + message_end_ordinal, message_start_source_uuid, + message_end_source_uuid, content_digest, tool_use_id, snippet + FROM recall_evidence + WHERE entry_id = ? ORDER BY id ASC`, id) + if err != nil { + return RecallEntry{}, fmt.Errorf("read reviewed recall evidence: %w", err) + } + for rows.Next() { + evidence, scanErr := scanRecallEvidenceRow(rows) + if scanErr != nil { + _ = rows.Close() + return RecallEntry{}, fmt.Errorf( + "scan reviewed recall evidence: %w", scanErr) + } + entry.Evidence = append(entry.Evidence, evidence) + } + if err := rows.Err(); err != nil { + _ = rows.Close() + return RecallEntry{}, fmt.Errorf("read reviewed recall evidence: %w", err) + } + if err := rows.Close(); err != nil { + return RecallEntry{}, fmt.Errorf("close reviewed recall evidence: %w", err) + } + + if err := tx.Commit(); err != nil { + return RecallEntry{}, fmt.Errorf("commit recall review: %w", err) + } + return entry, nil +} diff --git a/internal/db/recall_review_test.go b/internal/db/recall_review_test.go new file mode 100644 index 000000000..25331189b --- /dev/null +++ b/internal/db/recall_review_test.go @@ -0,0 +1,231 @@ +package db + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + corerecall "go.kenn.io/agentsview/internal/recall" +) + +func TestReviewRecallEntryTransitions(t *testing.T) { + tests := []struct { + name string + action RecallReviewAction + provenance bool + wantStatus string + wantReview string + wantErr error + }{ + { + name: "approve", action: RecallReviewApprove, provenance: true, + wantStatus: corerecall.StatusAccepted, + wantReview: corerecall.ReviewStateHumanReviewed, + }, + { + name: "approve revoked", action: RecallReviewApprove, + wantStatus: corerecall.StatusAccepted, + wantReview: corerecall.ReviewStateUnreviewedAuto, + wantErr: ErrRecallReviewProvenance, + }, + { + name: "archive", action: RecallReviewArchive, provenance: true, + wantStatus: corerecall.StatusArchived, + wantReview: corerecall.ReviewStateHumanRejected, + }, + { + name: "archive revoked", action: RecallReviewArchive, + wantStatus: corerecall.StatusArchived, + wantReview: corerecall.ReviewStateHumanRejected, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := testDB(t) + ctx := context.Background() + insertSession(t, d, "review-session", "agentsview") + confidence := 0.73 + _, err := d.InsertRecallEntry(RecallEntry{ + ID: "review-entry", Type: "preference", Scope: "project", + Status: corerecall.StatusAccepted, + ReviewState: corerecall.ReviewStateUnreviewedAuto, + Title: "Keep commands concise", + Body: "The user prefers short operational commands.", + Trigger: "when suggesting shell commands", + Confidence: &confidence, + Uncertainty: "May be task specific", + Project: "agentsview", CWD: "/work/agentsview", + GitBranch: "feature", Agent: "codex", + SourceSessionID: "review-session", SourceEpisodeID: "episode-1", + SourceRunID: "run-1", ExtractorMethod: "turns-v1", Model: "model-1", + Transferable: true, ProvenanceOK: tt.provenance, + Evidence: []RecallEvidence{{ + SessionID: "review-session", MessageStartOrdinal: 3, + MessageEndOrdinal: 4, Snippet: "Use the shorter command.", + }}, + }) + require.NoError(t, err) + _, err = d.getWriter().Exec(` + UPDATE recall_entries + SET created_at = '2026-01-02T03:04:05.000Z', + updated_at = '2026-01-02T03:04:05.000Z' + WHERE id = 'review-entry'`) + require.NoError(t, err) + + before, err := d.GetRecallEntry(ctx, "review-entry") + require.NoError(t, err) + require.NotNil(t, before) + got, err := d.ReviewRecallEntry(ctx, "review-entry", tt.action) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + after, getErr := d.GetRecallEntry(ctx, "review-entry") + require.NoError(t, getErr) + require.NotNil(t, after) + assert.Equal(t, tt.wantStatus, after.Status) + assert.Equal(t, tt.wantReview, after.ReviewState) + assert.Equal(t, before.UpdatedAt, after.UpdatedAt) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, got.Status) + assert.Equal(t, tt.wantReview, got.ReviewState) + assert.Equal(t, before.Title, got.Title) + assert.Equal(t, before.Body, got.Body) + assert.Equal(t, before.Trigger, got.Trigger) + assert.Equal(t, before.Confidence, got.Confidence) + assert.Equal(t, before.Uncertainty, got.Uncertainty) + assert.Equal(t, before.Project, got.Project) + assert.Equal(t, before.CWD, got.CWD) + assert.Equal(t, before.GitBranch, got.GitBranch) + assert.Equal(t, before.Agent, got.Agent) + assert.Equal(t, before.SourceSessionID, got.SourceSessionID) + assert.Equal(t, before.SourceEpisodeID, got.SourceEpisodeID) + assert.Equal(t, before.SourceRunID, got.SourceRunID) + assert.Equal(t, before.ExtractorMethod, got.ExtractorMethod) + assert.Equal(t, before.Model, got.Model) + assert.Equal(t, before.Transferable, got.Transferable) + assert.Equal(t, before.ProvenanceOK, got.ProvenanceOK) + assert.Equal(t, before.CreatedAt, got.CreatedAt) + assert.NotEqual(t, before.UpdatedAt, got.UpdatedAt) + require.Len(t, got.Evidence, 1) + assert.Equal(t, "Use the shorter command.", got.Evidence[0].Snippet) + }) + } +} + +func TestReviewRecallEntryRejectsInvalidTransitions(t *testing.T) { + tests := []struct { + name string + id string + action RecallReviewAction + status string + review string + want error + }{ + { + name: "missing", id: "missing", action: RecallReviewApprove, + status: corerecall.StatusAccepted, + review: corerecall.ReviewStateUnreviewedAuto, + want: ErrRecallEntryNotFound, + }, + { + name: "unknown action", id: "entry", action: "publish", + status: corerecall.StatusAccepted, + review: corerecall.ReviewStateUnreviewedAuto, + want: ErrInvalidRecallReviewAction, + }, + { + name: "already approved", id: "entry", action: RecallReviewArchive, + status: corerecall.StatusAccepted, + review: corerecall.ReviewStateHumanReviewed, + want: ErrRecallReviewConflict, + }, + { + name: "already rejected", id: "entry", action: RecallReviewApprove, + status: corerecall.StatusArchived, + review: corerecall.ReviewStateHumanRejected, + want: ErrRecallReviewConflict, + }, + { + name: "archived automatic", id: "entry", action: RecallReviewArchive, + status: corerecall.StatusArchived, + review: corerecall.ReviewStateUnreviewedAuto, + want: ErrRecallReviewConflict, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := testDB(t) + insertSession(t, d, "review-session", "agentsview") + if tt.id != "missing" { + _, err := d.InsertRecallEntry(RecallEntry{ + ID: tt.id, Type: "fact", Scope: "project", + Status: tt.status, ReviewState: tt.review, + Title: "Entry", Body: "Body", ProvenanceOK: true, + SourceSessionID: "review-session", + }) + require.NoError(t, err) + } + _, err := d.ReviewRecallEntry(context.Background(), tt.id, tt.action) + require.ErrorIs(t, err, tt.want) + }) + } +} + +func TestReviewRecallEntryAdvancesRelevantRevisions(t *testing.T) { + d := testDB(t) + ctx := context.Background() + insertSession(t, d, "review-session", "agentsview") + insert := func(id string) { + t.Helper() + _, err := d.InsertRecallEntry(RecallEntry{ + ID: id, Type: "fact", Scope: "project", + Status: corerecall.StatusAccepted, + ReviewState: corerecall.ReviewStateUnreviewedAuto, + Title: "Entry", Body: "Body", ProvenanceOK: true, + SourceSessionID: "review-session", + }) + require.NoError(t, err) + } + + insert("approve-entry") + queryBefore, err := d.RecallQueryRevision(ctx) + require.NoError(t, err) + corpusBefore, err := d.RecallCorpusRevision(ctx) + require.NoError(t, err) + _, err = d.ReviewRecallEntry(ctx, "approve-entry", RecallReviewApprove) + require.NoError(t, err) + queryAfter, err := d.RecallQueryRevision(ctx) + require.NoError(t, err) + corpusAfter, err := d.RecallCorpusRevision(ctx) + require.NoError(t, err) + assert.NotEqual(t, queryBefore, queryAfter) + assert.Equal(t, corpusBefore, corpusAfter) + + insert("archive-entry") + queryBefore, err = d.RecallQueryRevision(ctx) + require.NoError(t, err) + corpusBefore, err = d.RecallCorpusRevision(ctx) + require.NoError(t, err) + _, err = d.ReviewRecallEntry(ctx, "archive-entry", RecallReviewArchive) + require.NoError(t, err) + queryAfter, err = d.RecallQueryRevision(ctx) + require.NoError(t, err) + corpusAfter, err = d.RecallCorpusRevision(ctx) + require.NoError(t, err) + assert.NotEqual(t, queryBefore, queryAfter) + assert.NotEqual(t, corpusBefore, corpusAfter) +} + +func TestRecallReviewActionValidate(t *testing.T) { + assert.NoError(t, RecallReviewApprove.Validate()) + assert.NoError(t, RecallReviewArchive.Validate()) + assert.ErrorIs(t, RecallReviewAction("publish").Validate(), + ErrInvalidRecallReviewAction) + assert.False(t, errors.Is(ErrRecallReviewConflict, ErrRecallEntryNotFound)) +} diff --git a/internal/db/store.go b/internal/db/store.go index e4f05bbe0..66d6625ed 100644 --- a/internal/db/store.go +++ b/internal/db/store.go @@ -120,6 +120,9 @@ type Store interface { // RecallEntries. ListRecallEntries(ctx context.Context, q RecallQuery) ([]RecallEntry, error) GetRecallEntry(ctx context.Context, id string) (*RecallEntry, error) + ReviewRecallEntry( + ctx context.Context, id string, action RecallReviewAction, + ) (RecallEntry, error) QueryRecallEntries(ctx context.Context, q RecallQuery) (RecallPage, error) RecordRecallQueryEvent( ctx context.Context, event RecallQueryEvent, diff --git a/internal/duckdb/store.go b/internal/duckdb/store.go index 8ff99d346..d71c27845 100644 --- a/internal/duckdb/store.go +++ b/internal/duckdb/store.go @@ -242,6 +242,12 @@ func (s *Store) GetRecallEntry( return nil, db.ErrReadOnly } +func (s *Store) ReviewRecallEntry( + _ context.Context, _ string, _ db.RecallReviewAction, +) (db.RecallEntry, error) { + return db.RecallEntry{}, db.ErrReadOnly +} + func (s *Store) QueryRecallEntries( _ context.Context, _ db.RecallQuery, ) (db.RecallPage, error) { diff --git a/internal/postgres/store.go b/internal/postgres/store.go index 978cabfb3..1d5a8dcfa 100644 --- a/internal/postgres/store.go +++ b/internal/postgres/store.go @@ -354,6 +354,12 @@ func (s *Store) GetRecallEntry( return nil, db.ErrReadOnly } +func (s *Store) ReviewRecallEntry( + _ context.Context, _ string, _ db.RecallReviewAction, +) (db.RecallEntry, error) { + return db.RecallEntry{}, db.ErrReadOnly +} + func (s *Store) QueryRecallEntries( _ context.Context, _ db.RecallQuery, ) (db.RecallPage, error) { From d1042ea783c71ec62c9ddfc14b43e41132c43a35 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Sat, 8 Aug 2026 22:21:05 -0500 Subject: [PATCH 05/10] feat(recall): expose entry review action The Corpus UI needs a narrow mutation boundary for approving or dismissing one machine-generated entry without exposing broader entry editing. The endpoint validates a single explicit action, preserves typed storage conflicts, and returns the committed entry so clients can update one row in place.\n\nRead-only and maintenance states follow the server's existing capability and retry semantics, and vector refresh is scheduled only after a successful decision. --- internal/server/recall.go | 56 ++++++++++ internal/server/recall_test.go | 184 +++++++++++++++++++++++++++++++++ internal/server/server.go | 4 + 3 files changed, 244 insertions(+) diff --git a/internal/server/recall.go b/internal/server/recall.go index 564dc7e4e..88383498e 100644 --- a/internal/server/recall.go +++ b/internal/server/recall.go @@ -8,6 +8,7 @@ import ( "encoding/hex" "encoding/json" "errors" + "io" "net/http" "strings" "time" @@ -698,6 +699,61 @@ func (s *Server) handleGetRecallEntry( writeJSON(w, http.StatusOK, recall) } +type reviewRecallEntryRequest struct { + Action db.RecallReviewAction `json:"action"` +} + +func (s *Server) handleReviewRecallEntry( + w http.ResponseWriter, r *http.Request, +) { + if s.db.ReadOnly() { + handleReadOnly(w, db.ErrReadOnly) + return + } + var req reviewRecallEntryRequest + decoder := json.NewDecoder(r.Body) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&req); err != nil { + writeError(w, http.StatusBadRequest, "invalid JSON body") + return + } + if err := decoder.Decode(&struct{}{}); !errors.Is(err, io.EOF) { + writeError(w, http.StatusBadRequest, "invalid JSON body") + return + } + if err := req.Action.Validate(); err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + + entry, err := s.db.ReviewRecallEntry( + r.Context(), strings.TrimSpace(r.PathValue("id")), req.Action, + ) + if err != nil { + s.handleRecallReviewError(w, err) + return + } + s.notifyRecallCorpusMutation() + writeJSON(w, http.StatusOK, entry) +} + +func (s *Server) handleRecallReviewError(w http.ResponseWriter, err error) { + if handleContextError(w, err) || handleReadOnly(w, err) { + return + } + switch { + case errors.Is(err, db.ErrInvalidRecallReviewAction): + writeError(w, http.StatusBadRequest, err.Error()) + case errors.Is(err, db.ErrRecallEntryNotFound): + writeError(w, http.StatusNotFound, err.Error()) + case errors.Is(err, db.ErrRecallReviewConflict), + errors.Is(err, db.ErrRecallReviewProvenance): + writeError(w, http.StatusConflict, err.Error()) + default: + writeError(w, http.StatusInternalServerError, err.Error()) + } +} + func (s *Server) handleQueryRecallEntries( w http.ResponseWriter, r *http.Request, ) { diff --git a/internal/server/recall_test.go b/internal/server/recall_test.go index 9353bb5a1..d85f7fdb2 100644 --- a/internal/server/recall_test.go +++ b/internal/server/recall_test.go @@ -1096,6 +1096,169 @@ func TestGetRecallEntryFoundAndMissing(t *testing.T) { assertStatus(t, w, http.StatusNotFound) } +func TestReviewRecallEntryApproveAndArchive(t *testing.T) { + for _, tt := range []struct { + name string + action string + wantStatus string + wantReview string + }{ + { + name: "approve", action: "approve", wantStatus: "accepted", + wantReview: corerecall.ReviewStateHumanReviewed, + }, + { + name: "archive", action: "archive", wantStatus: "archived", + wantReview: corerecall.ReviewStateHumanRejected, + }, + } { + t.Run(tt.name, func(t *testing.T) { + var notifications atomic.Int32 + te := setupWithServerOpts(t, []server.Option{ + server.WithRecallCorpusMutationNotifier(func() { + notifications.Add(1) + }), + }) + seedReviewableRecallEntry(t, te, "review-me", true, + corerecall.StatusAccepted, + corerecall.ReviewStateUnreviewedAuto) + + w := te.post(t, "/api/v1/recall/entries/review-me/review", + `{"action":"`+tt.action+`"}`) + + assertStatus(t, w, http.StatusOK) + got := decode[db.RecallEntry](t, w) + assert.Equal(t, tt.wantStatus, got.Status) + assert.Equal(t, tt.wantReview, got.ReviewState) + require.Len(t, got.Evidence, 1) + assert.Equal(t, int32(1), notifications.Load()) + }) + } +} + +func TestReviewRecallEntryMapsRequestAndTransitionErrors(t *testing.T) { + tests := []struct { + name string + id string + body string + provenance bool + status string + review string + seed bool + wantStatus int + }{ + { + name: "malformed JSON", id: "review-me", body: `{"action":`, + provenance: true, status: corerecall.StatusAccepted, + review: corerecall.ReviewStateUnreviewedAuto, seed: true, + wantStatus: http.StatusBadRequest, + }, + { + name: "unknown field", id: "review-me", + body: `{"action":"approve","note":"no"}`, + provenance: true, status: corerecall.StatusAccepted, + review: corerecall.ReviewStateUnreviewedAuto, seed: true, + wantStatus: http.StatusBadRequest, + }, + { + name: "unknown action", id: "review-me", body: `{"action":"publish"}`, + provenance: true, status: corerecall.StatusAccepted, + review: corerecall.ReviewStateUnreviewedAuto, seed: true, + wantStatus: http.StatusBadRequest, + }, + { + name: "missing entry", id: "missing", body: `{"action":"approve"}`, + wantStatus: http.StatusNotFound, + }, + { + name: "already approved", id: "review-me", body: `{"action":"archive"}`, + provenance: true, status: corerecall.StatusAccepted, + review: corerecall.ReviewStateHumanReviewed, seed: true, + wantStatus: http.StatusConflict, + }, + { + name: "already rejected", id: "review-me", body: `{"action":"approve"}`, + status: corerecall.StatusArchived, + review: corerecall.ReviewStateHumanRejected, seed: true, + wantStatus: http.StatusConflict, + }, + { + name: "archived automatic", id: "review-me", body: `{"action":"archive"}`, + provenance: true, status: corerecall.StatusArchived, + review: corerecall.ReviewStateUnreviewedAuto, seed: true, + wantStatus: http.StatusConflict, + }, + { + name: "revoked approval", id: "review-me", body: `{"action":"approve"}`, + status: corerecall.StatusAccepted, + review: corerecall.ReviewStateUnreviewedAuto, seed: true, + wantStatus: http.StatusConflict, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var notifications atomic.Int32 + te := setupWithServerOpts(t, []server.Option{ + server.WithRecallCorpusMutationNotifier(func() { + notifications.Add(1) + }), + }) + if tt.seed { + seedReviewableRecallEntry(t, te, tt.id, tt.provenance, + tt.status, tt.review) + } + + w := te.post(t, "/api/v1/recall/entries/"+tt.id+"/review", tt.body) + + assertStatus(t, w, tt.wantStatus) + assert.Zero(t, notifications.Load()) + }) + } +} + +func TestReviewRecallEntryAllowsRevokedArchive(t *testing.T) { + te := setup(t) + seedReviewableRecallEntry(t, te, "review-me", false, + corerecall.StatusAccepted, corerecall.ReviewStateUnreviewedAuto) + + w := te.post(t, "/api/v1/recall/entries/review-me/review", + `{"action":"archive"}`) + + assertStatus(t, w, http.StatusOK) + got := decode[db.RecallEntry](t, w) + assert.Equal(t, corerecall.ReviewStateHumanRejected, got.ReviewState) +} + +func TestReviewRecallEntryMapsUnavailableWriters(t *testing.T) { + t.Run("maintenance", func(t *testing.T) { + var notifications atomic.Int32 + te := setupWithServerOpts(t, []server.Option{ + server.WithRecallCorpusMutationNotifier(func() { + notifications.Add(1) + }), + }) + seedReviewableRecallEntry(t, te, "review-me", true, + corerecall.StatusAccepted, corerecall.ReviewStateUnreviewedAuto) + require.NoError(t, te.db.CloseWriter()) + t.Cleanup(func() { require.NoError(t, te.db.ReopenWriter()) }) + + w := te.post(t, "/api/v1/recall/entries/review-me/review", + `{"action":"approve"}`) + + assertStatus(t, w, http.StatusServiceUnavailable) + assert.Equal(t, "5", w.Header().Get("Retry-After")) + assert.Zero(t, notifications.Load()) + }) + + t.Run("read only", func(t *testing.T) { + te := setupPGMode(t) + w := te.post(t, "/api/v1/recall/entries/missing/review", + `{"action":"approve"}`) + assertStatus(t, w, http.StatusNotImplemented) + }) +} + func TestQueryRecallEntriesReturnsContext(t *testing.T) { te := setup(t) seedRecallEntrySession(t, te) @@ -1699,3 +1862,24 @@ func seedRecallEntry(t *testing.T, te *testEnv, m db.RecallEntry) { _, err := te.db.InsertRecallEntry(m) require.NoError(t, err, "InsertRecallEntry") } + +func seedReviewableRecallEntry( + t *testing.T, + te *testEnv, + id string, + provenance bool, + status string, + reviewState string, +) { + t.Helper() + seedRecallEntrySession(t, te) + seedRecallEntry(t, te, db.RecallEntry{ + ID: id, Type: "fact", Scope: "project", Status: status, + ReviewState: reviewState, Title: "Review this fact", Body: "Fact body", + SourceSessionID: "recall-session", ProvenanceOK: provenance, + Evidence: []db.RecallEvidence{{ + SessionID: "recall-session", MessageStartOrdinal: 1, + MessageEndOrdinal: 2, Snippet: "Supporting transcript range.", + }}, + }) +} diff --git a/internal/server/server.go b/internal/server/server.go index 7b063254a..a2244269e 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -464,6 +464,10 @@ func (s *Server) routes() { "GET /api/v1/recall/entries/{id}", s.handleGetRecallEntry, )) + s.mux.Handle("POST /api/v1/recall/entries/{id}/review", s.withTimeout( + "POST /api/v1/recall/entries/{id}/review", + s.handleReviewRecallEntry, + )) s.mux.Handle("GET /api/v1/recall/extraction/status", s.withTimeout( "GET /api/v1/recall/extraction/status", s.handleRecallExtractionStatus, From 7e43512ac596027f81cb65d6c225a8fdf04c3def Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Sat, 8 Aug 2026 22:23:00 -0500 Subject: [PATCH 06/10] feat(recall): add localized entry review contract Review controls need one typed client operation and stable user-facing language before the Corpus table can expose mutations. The frontend now models the two allowed decisions, preserves server error details, and names every review state so raw storage values do not leak into the interface.\n\nAll supported locales carry the same confirmation, provenance, and failure messages, keeping the interaction accessible regardless of the active language. --- frontend/messages/en.json | 13 +++++++++++++ frontend/messages/fr.json | 13 +++++++++++++ frontend/messages/ko.json | 13 +++++++++++++ frontend/messages/zh-CN.json | 13 +++++++++++++ frontend/messages/zh-TW.json | 13 +++++++++++++ frontend/src/lib/api/recall.test.ts | 26 ++++++++++++++++++++++++++ frontend/src/lib/api/recall.ts | 21 +++++++++++++++++++++ frontend/src/lib/api/types/recall.ts | 2 ++ 8 files changed, 114 insertions(+) diff --git a/frontend/messages/en.json b/frontend/messages/en.json index ba654ac93..16e633774 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1304,6 +1304,19 @@ "recall_page_all_generations": "All generations", "recall_page_review_filter": "Review state", "recall_page_all_review_states": "All review states", + "recall_page_review_state_human_reviewed": "Human approved", + "recall_page_review_state_human_rejected": "Human rejected", + "recall_page_review_state_unreviewed_auto": "Unreviewed automatic", + "recall_page_review_state_calibrated_auto": "Calibrated automatic", + "recall_page_review_state_eval_raw": "Evaluation raw", + "recall_page_review_approve": "Approve", + "recall_page_review_archive": "Archive", + "recall_page_review_approve_disabled": "Approval is unavailable because the source evidence was revoked.", + "recall_page_review_archive_title": "Archive Recall entry", + "recall_page_review_archive_message": "Archive “{title}” as rejected? It will remain outside the served Recall corpus.", + "recall_page_review_cancel": "Cancel", + "recall_page_review_close": "Close archive confirmation", + "recall_page_review_error": "Could not review this Recall entry: {error}", "recall_page_table_label": "Recall entries", "recall_page_fact_column": "Fact", "recall_page_project_column": "Project", diff --git a/frontend/messages/fr.json b/frontend/messages/fr.json index 16f517e9c..62bb51531 100644 --- a/frontend/messages/fr.json +++ b/frontend/messages/fr.json @@ -1304,6 +1304,19 @@ "recall_page_all_generations": "Toutes les générations", "recall_page_review_filter": "État de révision", "recall_page_all_review_states": "Tous les états de révision", + "recall_page_review_state_human_reviewed": "Approuvé par un humain", + "recall_page_review_state_human_rejected": "Rejeté par un humain", + "recall_page_review_state_unreviewed_auto": "Automatique non vérifié", + "recall_page_review_state_calibrated_auto": "Automatique calibré", + "recall_page_review_state_eval_raw": "Évaluation brute", + "recall_page_review_approve": "Approuver", + "recall_page_review_archive": "Archiver", + "recall_page_review_approve_disabled": "L’approbation est indisponible car les preuves sources ont été révoquées.", + "recall_page_review_archive_title": "Archiver l’entrée Recall", + "recall_page_review_archive_message": "Archiver « {title} » comme rejetée ? Elle restera exclue du corpus Recall servi.", + "recall_page_review_cancel": "Annuler", + "recall_page_review_close": "Fermer la confirmation d’archivage", + "recall_page_review_error": "Impossible de vérifier cette entrée Recall : {error}", "recall_page_table_label": "Entrées de rappel", "recall_page_fact_column": "Fait", "recall_page_project_column": "Projet", diff --git a/frontend/messages/ko.json b/frontend/messages/ko.json index 160b45708..d13f66105 100644 --- a/frontend/messages/ko.json +++ b/frontend/messages/ko.json @@ -1274,6 +1274,19 @@ "recall_page_all_generations": "모든 세대", "recall_page_review_filter": "검토 상태", "recall_page_all_review_states": "모든 검토 상태", + "recall_page_review_state_human_reviewed": "사람이 승인함", + "recall_page_review_state_human_rejected": "사람이 거부함", + "recall_page_review_state_unreviewed_auto": "검토되지 않은 자동 항목", + "recall_page_review_state_calibrated_auto": "보정된 자동 항목", + "recall_page_review_state_eval_raw": "원시 평가", + "recall_page_review_approve": "승인", + "recall_page_review_archive": "보관", + "recall_page_review_approve_disabled": "원본 근거가 철회되어 승인할 수 없습니다.", + "recall_page_review_archive_title": "Recall 항목 보관", + "recall_page_review_archive_message": "“{title}”을(를) 거부됨으로 보관하시겠습니까? 제공되는 Recall 코퍼스에서 계속 제외됩니다.", + "recall_page_review_cancel": "취소", + "recall_page_review_close": "보관 확인 닫기", + "recall_page_review_error": "이 Recall 항목을 검토할 수 없습니다: {error}", "recall_page_table_label": "리콜 항목", "recall_page_fact_column": "사실", "recall_page_project_column": "프로젝트", diff --git a/frontend/messages/zh-CN.json b/frontend/messages/zh-CN.json index acf1acffd..baea9de59 100644 --- a/frontend/messages/zh-CN.json +++ b/frontend/messages/zh-CN.json @@ -1272,6 +1272,19 @@ "recall_page_all_generations": "所有代次", "recall_page_review_filter": "审核状态", "recall_page_all_review_states": "所有审核状态", + "recall_page_review_state_human_reviewed": "人工已批准", + "recall_page_review_state_human_rejected": "人工已拒绝", + "recall_page_review_state_unreviewed_auto": "未审核的自动条目", + "recall_page_review_state_calibrated_auto": "已校准的自动条目", + "recall_page_review_state_eval_raw": "原始评估", + "recall_page_review_approve": "批准", + "recall_page_review_archive": "归档", + "recall_page_review_approve_disabled": "由于来源证据已被撤销，无法批准。", + "recall_page_review_archive_title": "归档 Recall 条目", + "recall_page_review_archive_message": "将“{title}”归档为已拒绝吗？它将继续排除在提供服务的 Recall 语料库之外。", + "recall_page_review_cancel": "取消", + "recall_page_review_close": "关闭归档确认", + "recall_page_review_error": "无法审核此 Recall 条目：{error}", "recall_page_table_label": "召回条目", "recall_page_fact_column": "事实", "recall_page_project_column": "项目", diff --git a/frontend/messages/zh-TW.json b/frontend/messages/zh-TW.json index e56409e48..6e2a3875f 100644 --- a/frontend/messages/zh-TW.json +++ b/frontend/messages/zh-TW.json @@ -1272,6 +1272,19 @@ "recall_page_all_generations": "所有代次", "recall_page_review_filter": "審核狀態", "recall_page_all_review_states": "所有審核狀態", + "recall_page_review_state_human_reviewed": "人工已核准", + "recall_page_review_state_human_rejected": "人工已拒絕", + "recall_page_review_state_unreviewed_auto": "未審核的自動項目", + "recall_page_review_state_calibrated_auto": "已校準的自動項目", + "recall_page_review_state_eval_raw": "原始評估", + "recall_page_review_approve": "核准", + "recall_page_review_archive": "封存", + "recall_page_review_approve_disabled": "來源證據已撤銷，因此無法核准。", + "recall_page_review_archive_title": "封存 Recall 項目", + "recall_page_review_archive_message": "要將「{title}」封存為已拒絕嗎？它仍會排除在提供服務的 Recall 語料庫之外。", + "recall_page_review_cancel": "取消", + "recall_page_review_close": "關閉封存確認", + "recall_page_review_error": "無法審核此 Recall 項目：{error}", "recall_page_table_label": "召回項目", "recall_page_fact_column": "事實", "recall_page_project_column": "專案", diff --git a/frontend/src/lib/api/recall.test.ts b/frontend/src/lib/api/recall.test.ts index 0de648822..ca858b9bd 100644 --- a/frontend/src/lib/api/recall.test.ts +++ b/frontend/src/lib/api/recall.test.ts @@ -9,6 +9,7 @@ import { activateRecallExtractionGeneration, fetchRecallEntries, fetchRecallExtractionProgress, + reviewRecallEntry, retireRecallExtractionGeneration, } from "./recall.js"; @@ -58,6 +59,31 @@ describe("fetchRecallEntries", () => { }); }); +describe("reviewRecallEntry", () => { + it("posts one encoded review action and returns the updated entry", async () => { + const updated = { + id: "entry one", + status: "archived", + review_state: "human_rejected", + }; + const fetchMock = vi.fn().mockResolvedValue(new Response( + JSON.stringify(updated), + { status: 200, headers: { "Content-Type": "application/json" } }, + )); + vi.stubGlobal("fetch", fetchMock); + + await expect(reviewRecallEntry("entry one", "archive")) + .resolves.toEqual(updated); + expect(fetchMock).toHaveBeenCalledWith( + "/api/v1/recall/entries/entry%20one/review", + expect.objectContaining({ + method: "POST", + body: JSON.stringify({ action: "archive" }), + }), + ); + }); +}); + describe("fetchRecallExtractionProgress", () => { it("sends bounded generation, state, and cursor filters", async () => { const fetchMock = vi.fn().mockResolvedValue(new Response( diff --git a/frontend/src/lib/api/recall.ts b/frontend/src/lib/api/recall.ts index df364490a..864401a0a 100644 --- a/frontend/src/lib/api/recall.ts +++ b/frontend/src/lib/api/recall.ts @@ -7,6 +7,7 @@ import type { RecallExtractProgressPage, RecallExtractProgressResponse, RecallExtractionStatus, + RecallReviewAction, } from "./types/recall.js"; import { ApiError, @@ -54,6 +55,26 @@ export async function fetchRecallEntries( }; } +export async function reviewRecallEntry( + id: string, + action: RecallReviewAction, +): Promise { + const response = await fetch( + `${getBase()}/recall/entries/${encodeURIComponent(id)}/review`, + authHeaders({ + method: "POST", + body: JSON.stringify({ action }), + }), + ); + if (!response.ok) { + throw new ApiError( + response.status, + await responseErrorMessage(response), + ); + } + return (await response.json()) as RecallEntry; +} + export async function fetchRecallExtractionStatus( signal?: AbortSignal, ): Promise { diff --git a/frontend/src/lib/api/types/recall.ts b/frontend/src/lib/api/types/recall.ts index 06d861f80..20945af6e 100644 --- a/frontend/src/lib/api/types/recall.ts +++ b/frontend/src/lib/api/types/recall.ts @@ -33,6 +33,8 @@ export interface RecallEntry { evidence?: RecallEvidence[]; } +export type RecallReviewAction = "approve" | "archive"; + export interface RecallEntriesResponse { entries: RecallEntry[]; trusted_only: boolean; From 4220a3720cc689fb5b7b488451e3b24015e8e205 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Sat, 8 Aug 2026 22:26:28 -0500 Subject: [PATCH 07/10] feat(recall): review entries from the corpus table A distilled corpus is only useful if people can turn uncertain automatic output into an explicit decision where they inspect it. Expanded rows now expose immediate approval and confirmed archive actions, while revoked provenance blocks only the trust-increasing transition.\n\nSuccessful responses update the current page locally instead of reloading or disturbing pagination and scroll. Pending and conflict states stay scoped to the affected row so unrelated corpus browsing remains available. --- .../recall/RecallCorpusPanel.svelte | 183 ++- .../recall/RecallCorpusPanel.test.ts | 1129 +++++++++++------ 2 files changed, 915 insertions(+), 397 deletions(-) diff --git a/frontend/src/lib/components/recall/RecallCorpusPanel.svelte b/frontend/src/lib/components/recall/RecallCorpusPanel.svelte index 753351654..0b57273ec 100644 --- a/frontend/src/lib/components/recall/RecallCorpusPanel.svelte +++ b/frontend/src/lib/components/recall/RecallCorpusPanel.svelte @@ -17,6 +17,7 @@ fetchRecallEntries, fetchRecallExtractionProgress, fetchRecallExtractionStatus, + reviewRecallEntry, retireRecallExtractionGeneration, } from "../../api/recall.js"; import type { @@ -26,6 +27,7 @@ RecallExtractProgress, RecallExtractProgressState, RecallExtractionStatus, + RecallReviewAction, } from "../../api/types/recall.js"; import { ApiError, isAbortError } from "../../api/runtime.js"; import { formatDateTime, m } from "../../i18n/index.js"; @@ -47,6 +49,7 @@ ]; const REVIEW_STATES = [ "human_reviewed", + "human_rejected", "unreviewed_auto", "calibrated_auto", "eval_raw", @@ -76,6 +79,9 @@ let generationAction = $state(null); let generationActionLoading = $state(false); let generationActionError = $state(""); + let reviewingEntryIds = $state([]); + let reviewErrors = $state>({}); + let archiveEntry = $state(null); let search = $state(""); let query = $state(""); let project = $state(""); @@ -141,8 +147,8 @@ }, ...REVIEW_STATES.map((name) => ({ name, - label: name, - displayLabel: name, + label: reviewStateLabel(name), + displayLabel: reviewStateLabel(name), })), ]); const progressStateOptions = $derived([ @@ -314,6 +320,66 @@ } } + function reviewStateLabel(state: string): string { + switch (state) { + case "human_reviewed": + return m.recall_page_review_state_human_reviewed(); + case "human_rejected": + return m.recall_page_review_state_human_rejected(); + case "unreviewed_auto": + return m.recall_page_review_state_unreviewed_auto(); + case "calibrated_auto": + return m.recall_page_review_state_calibrated_auto(); + case "eval_raw": + return m.recall_page_review_state_eval_raw(); + default: + return state; + } + } + + function isReviewable(entry: RecallEntry): boolean { + return entry.status === "accepted" && + entry.review_state === "unreviewed_auto"; + } + + function keepAfterReview(entry: RecallEntry): boolean { + return entry.status === "accepted" && + (!reviewState || entry.review_state === reviewState); + } + + async function submitReview( + entry: RecallEntry, + action: RecallReviewAction, + ) { + if (reviewingEntryIds.includes(entry.id)) return; + reviewingEntryIds = [...reviewingEntryIds, entry.id]; + reviewErrors = { ...reviewErrors, [entry.id]: "" }; + try { + const updated = await reviewRecallEntry(entry.id, action); + const keep = keepAfterReview(updated); + entries = keep + ? entries.map((item) => item.id === updated.id ? updated : item) + : entries.filter((item) => item.id !== updated.id); + if (!keep) { + expandedEntryIds = expandedEntryIds.filter((id) => id !== updated.id); + } + archiveEntry = null; + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + reviewErrors = { + ...reviewErrors, + [entry.id]: m.recall_page_review_error({ error: detail }), + }; + } finally { + reviewingEntryIds = reviewingEntryIds.filter((id) => id !== entry.id); + } + } + + function closeArchiveReview() { + if (archiveEntry && reviewingEntryIds.includes(archiveEntry.id)) return; + archiveEntry = null; + } + function progressTimestamp(value: string): string { return formatDateTime(value, { dateStyle: "medium", @@ -392,6 +458,28 @@ {/if} {/snippet} +{#snippet archiveReviewFooter()} + {#if archiveEntry} + {@const busy = reviewingEntryIds.includes(archiveEntry.id)} + +