Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 0 additions & 52 deletions HANDOVER.md

This file was deleted.

133 changes: 133 additions & 0 deletions docs/superpowers/specs/2026-07-19-reference-review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
# Handoff — ReferenceReview slice: design/correctness interrogation brief

**Date:** 2026-07-19
**Context branch:** `polish/v2-ui-shell-integration` (PR #232). The ReferenceReview slice itself is **already on `develop`** (merged via #230).
**Goal of next session:** interrogate the Presentation, Domain/infra, and Backend layers for **user design, API design, correctness, and performance** — then decide whether to redesign-in-place, or replace.

---

## Why this brief exists (the reframing)

I was asked whether the reference-review files are half-baked sprawl worth deleting, this is a critical review of the ReferenceReview vertical slice across three layers to determine if it is worth redesigning.

**Where the value actually sits:** the *presentation* (~360 LOC) is the under-designed, replaceable part. The *domain/infra + backend contract* (~1,200 LOC, well-tested) is the asset a redesign reuses — deleting it means rebuilding it.

---

## The full surface (the "any others")

### Presentation (~360 LOC) — the redesign target
- `components/v2/review/ReviewRecordCard.vue` (137) — one card per schema-record; renders context as raw `<dl>`, **dumps orphaned values as `JSON.stringify`**, per-record submit/save-draft/discard footer.
- `components/v2/review/ProjectionReviewForm.vue` (32) — thin list wrapper over records.
- `components/v2/review/ReviewCellInput.vue` (151) — dispatches to existing annotation widgets by question type.
- `components/v2/review/ReviewProvenance.vue` (44) — agent/score/source line.
- `pages/references/[...reference].vue` (69) — catch-all route (slashed DOIs), shell-wrapped in #232.

### Domain / infra (~1,100 LOC) — the asset
- `v2/domain/entities/review/ReferenceReview.ts` (65) — aggregate: reference → ReviewRecord[] (+ cells, context, orphaned values, draft).
- `v2/domain/entities/review/widget-adapters.ts` (88, +120 test) — **maps v2 questions ↔ existing annotation widget option models** (label/rating/ranking/table). High reuse value.
- `v2/domain/entities/review/widget-mapping.ts` (25, +43 test), `SuggestionHint.ts` (22, +20 test), `response-values.ts` (9, +24 test).
- `v2/domain/usecases/get-reference-review-use-case.ts` (131, **+245 test**) — assembles projection + suggestions + responses + drafts into the review model.
- `v2/domain/usecases/submit-reference-review-use-case.ts` (25, +38 test), `save-review-draft-use-case.ts` (16), `discard-review-use-case.ts` (17).
- `v2/infrastructure/storage/ReferenceReviewsStorage.ts` (27) — Pinia store keyed by reference.
- `v2/infrastructure/repositories/ProjectionRepository.ts` (49) — GET `/v2/projection/references/{encoded}`.
- `v2/di/di.ts` — registrations (partial file).
- `components/v2/schemas/V2RecordsTable.vue` — the **entry-point link** `/references/${encodeURIComponent(reference)}?workspace_id=…` (partial file).

### Backend (~124 LOC + 2 integration tests)
- `extralit-server/src/extralit_server/api/v2/projection.py` (31) — `GET /projection/references/{reference:path}` → `ProjectionView`. Note the deliberate `/projection/...` prefix to avoid the greedy `:path` on `GET /references/{reference:path}` shadowing it.
- `extralit-server/src/extralit_server/contexts/v2/projection.py` (70) — cross-schema assembly.
- `extralit-server/src/extralit_server/api/schemas/v2/projection.py` (23) — `ProjectionView` DTO.
- Tests: `tests/integration/api/v2/test_projection.py`, `tests/integration/contexts/v2/test_projection.py`.

### e2e (depend on the slice)
- `e2e/v2/{review-loop,draft-lifecycle,slashed-reference}.spec.ts` — `goto` the `/references/<encoded>` URL directly; exercise suggestion→response, drafts, slashed-DOI. (These pass **serially**; parallel flakes on shared-schema `rebuild-index` contention — see the PR #232 HANDOVER.)

---

## Interrogation matrix (start here next session)

Concrete hotspots I noticed while mapping — not yet verified as defects, but the places to point the review.

### Presentation — **user design** (the weakest layer)
- Orphaned values rendered via `JSON.stringify` (`ReviewRecordCard`) — placeholder, not a designed affordance.
- Context shown as bare `<dl>`; provenance is a minimal agent/score line — no visual hierarchy for "what am I reviewing / what changed."
- Multi-record references: one card stacked per schema-record with **no overview, no navigation, no bulk/keyboard flow**; submit/save-draft/discard are per-card only.
- Loading/empty/error states are minimal (single `V2Empty`/`BaseLoading`).
- **Question for design:** what *is* the review unit — a reference, a schema, a single projected record? The current model shows all records for a reference flat; is that the intended mental model?

### Domain/infra — **correctness**
- `ReviewRecordCard.cleanValues()` drops `null`/`undefined`/`""` before emit. Interaction with **required-question enforcement** (server-side) and with legitimately-empty answers needs a hard look — can a user intentionally clear a value, and does that round-trip?
- `get-reference-review-use-case.ts` merges projection + suggestions + responses + drafts. Check merge precedence (draft vs saved response vs suggestion), and staleness (Pinia `ReferenceReviewsStorage` cache vs re-fetch).
- `ProjectionRepository` does `encodeURIComponent(reference)` and the backend uses `{reference:path}` — verify no double-encode / slash-decoding mismatch (the `slashed-reference` spec covers the happy path; probe `%2F` vs `/`, `#`, `?` in DOIs).
- `widget-adapters` rebuild option arrays on every `modelValue` change (watch in `ReviewCellInput`) — verify no state loss on external resets (draft restore/discard).

### Domain/infra + Presentation — **performance**
- No pagination on the review page — loads **all** records for a reference at once. Fan-out cost if a reference spans many schemas/records.
- `ReviewCellInput` watch rebuilds label/rating/ranking option arrays per keystroke/change — check re-render cost with many cells.
- Repeated single-record hydration vs batch.

### Backend — **API design + performance**
- `ProjectionView` shape: is one endpoint returning the whole cross-schema projection the right granularity, or should it paginate / stream / split per schema?
- `contexts/v2/projection.py` (70 LOC) assembles across schemas — check query count (N schemas × per-record hydration from Postgres); watch for N+1.
- `workspace_id` is a **required query param** on the projection GET — confirm that's the right scoping contract (vs deriving from auth/reference).
- Suggestion/response/draft are **separate PUT/POST** calls from the client (see `submit`/`save-draft`/`discard` use-cases) — is a batched review-submit endpoint warranted?
- The `/projection/...` vs `/references/{reference:path}` routing split (shadowing workaround) — is the URL design coherent for the SDK (#231) too?

### Backend — **correctness**
- FTS/index eventual consistency already characterized (see PR #232 HANDOVER): reseed leaves a cold index; `:rebuild-index` is needed to warm it. Relevant if projection reads from the index anywhere.
- The `prefilter`-kwarg 500 (FTS + scalar filter) was fixed in #232 (`4fa777343`) — confirm the projection path doesn't have a sibling issue.

---

## Options on the table (decision pending — this is yours)

1. **Leave #232 as-is** — reference-review stays shell-integrated; a later redesign replaces only the presentation (~360 LOC), reusing domain + backend. Lowest friction. The BaseButton fix + 3 e2e specs need the page to render regardless.
2. **Hide the entry point in #232** *(my recommendation if you want it visibly parked)* — remove the `V2RecordsTable → /references` link (and optionally un-shell the page) so it's unreachable/unblessed, but the code + backend + domain remain for the redesign. e2e specs still pass (they `goto` directly).
3. **Revert the whole slice** — separate PR removing frontend + backend projection + e2e, reconciled against SDK #231. Only if the product call is "pull this direction entirely." **Not recommended** — throws away ~1,200 LOC of tested, reusable machinery + a working backend contract you'd rebuild.

**Recommendation:** #2 to park it pending design, #1 if fine leaving it reachable. Avoid #3.

---

## Next steps

1. **Decide the parking option** (#1/#2/#3 above) for PR #232 before it merges. If #2, the change is small: drop the `/references` link in `components/v2/schemas/V2RecordsTable.vue` (and optionally revert the `pages/references/[...reference].vue` shell wrap).
2. **Run the design/brainstorm session** on the *presentation* using `superpowers:brainstorming` — anchor on the "what is the review unit?" question above.
3. **Do the layer interrogation** using the matrix: consider a focused `/code-review` (or the multi-agent `/code-review ultra`) scoped to the review + projection files for correctness/perf, in parallel with the UX design work.
4. **Backend API review** — validate the `ProjectionView` granularity and the projection query fan-out (N+1) before building new UI on top.
5. Keep the domain/infra + backend contract as the stable base; treat the four `components/v2/review/*.vue` + the page as replaceable.

---

## Map of important files

**Presentation (redesign target)**
- `extralit-frontend/components/v2/review/ReviewRecordCard.vue`
- `extralit-frontend/components/v2/review/ProjectionReviewForm.vue`
- `extralit-frontend/components/v2/review/ReviewCellInput.vue`
- `extralit-frontend/components/v2/review/ReviewProvenance.vue`
- `extralit-frontend/pages/references/[...reference].vue`
- `extralit-frontend/pages/references/useReferenceReviewViewModel.ts` (+ `.test.ts`)

**Domain / infra (asset — reuse)**
- `extralit-frontend/v2/domain/entities/review/` — `ReferenceReview.ts`, `widget-adapters.ts`, `widget-mapping.ts`, `SuggestionHint.ts`, `response-values.ts` (each with tests)
- `extralit-frontend/v2/domain/usecases/` — `get-reference-review-use-case.ts`, `submit-reference-review-use-case.ts`, `save-review-draft-use-case.ts`, `discard-review-use-case.ts`
- `extralit-frontend/v2/infrastructure/storage/ReferenceReviewsStorage.ts`
- `extralit-frontend/v2/infrastructure/repositories/ProjectionRepository.ts`
- `extralit-frontend/v2/di/di.ts` (registrations)
- `extralit-frontend/components/v2/schemas/V2RecordsTable.vue` (entry-point link)

**Backend**
- `extralit-server/src/extralit_server/api/v2/projection.py`
- `extralit-server/src/extralit_server/contexts/v2/projection.py`
- `extralit-server/src/extralit_server/api/schemas/v2/projection.py`
- `extralit-server/tests/integration/api/v2/test_projection.py`
- `extralit-server/tests/integration/contexts/v2/test_projection.py`

**e2e**
- `extralit-frontend/e2e/v2/{review-loop,draft-lifecycle,slashed-reference}.spec.ts`

**Cross-references**
- `HANDOVER.md` — PR #232 (v2 UI shell) session handoff, incl. the serial-vs-parallel e2e reliability note.
- #230 (`8343da1f8`) — original merge of this slice. #231 — open SDK slice that shares the v2 API surface.
27 changes: 25 additions & 2 deletions extralit-frontend/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,31 @@ npm run e2e:report # View test report
> `/api/v1/token` + `/api/v1/me` offline, and waits for the home/datasets landing at `/`
> (there is no `/datasets` route). This flow is runtime-verified via the CDP browser. The
> per-page specs still need fresh Extralit screenshot baselines (`--update-snapshots`); the
> inherited ones are Argilla's. The local Playwright browser can't launch on the Orin dev
> host (missing OS libs, no sudo) — run the headless gate in CI.
> inherited ones are Argilla's. The local Playwright chromium **does** launch on the Orin
> host (verified 2026-07-18: `chromium.launch({headless:true})` → Chrome 149 after a plain
> `npx playwright install chromium`; no `install-deps`/sudo needed), so the headless gate
> can run locally as well as in CI.

## v2 e2e suite (`e2e/v2/`, real backend — the v2 slice's integration gate)

Separate Playwright project (`--project=v2`, `testMatch: v2/**/*.spec.ts`); the legacy
Argilla specs above are **not** a v2 gate. No network mocking — it exercises real bearer
auth on `/api/v2`, slashed-DOI encoding, the suggestion→response loop, drafts and search
freshness. Env knobs (see `e2e/v2/fixtures.ts`): `E2E_API_URL` (default `http://localhost:6900`),
`E2E_BASE_URL`/`BASE_URL` (default `http://localhost:3000`), `E2E_USERNAME`/`E2E_PASSWORD`
(default `extralit`/`12345678`), optional `E2E_CDP_URL` to drive a remote chromium.

```bash
npm run e2e:v2:seed # uv run ../extralit-server python e2e/v2/seed/seed_v2_e2e.py
npm run dev -- --host # dev server reachable from the browser
npm run e2e:v2 # playwright test --project=v2 (local chromium)
```

Requires the full stack up with the server on :6900. On the Orin host the backing services
publish to `localhost` (postgres :5432, minio :9000, elasticsearch :9200) but the compose
`redis` is **not** published, and the server `.env` uses docker-network hostnames
(`minio`/`elasticsearch`/`redis`) — running the server on the host needs those overridden
to `localhost` plus a throwaway redis on :6379.

## Code Quality

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
<script>
export default {
name: "BaseButton",
emits: ["click", "on-click", "mouseover", "mouseleave"],
props: {
tabIndex: {
type: Number,
Expand All @@ -66,8 +65,9 @@ export default {
},
disabled: Boolean,
centered: Boolean,
to: { type: String | Object },
to: { type: [String, Object] },
},
emits: ["click", "on-click", "mouseover", "mouseleave"],
computed: {
newRel() {
if (this.target === "_blank") {
Expand Down
30 changes: 30 additions & 0 deletions extralit-frontend/components/v2/common/V2Empty.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<template>
<div class="v2-empty">
<slot>
<p class="v2-empty__message" v-text="message" />
</slot>
</div>
</template>

<script lang="ts">
import { defineComponent } from "vue";

export default defineComponent({
name: "V2Empty",
props: { message: { type: String, default: "" } },
});
</script>

<style lang="scss" scoped>
.v2-empty {
display: flex;
align-items: center;
justify-content: center;
padding: $base-space * 6 $base-space * 2;
&__message {
margin: 0;
color: var(--fg-tertiary);
@include font-size(14px);
}
}
</style>
22 changes: 22 additions & 0 deletions extralit-frontend/components/v2/common/V2StatusBadge.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { describe, expect, it } from "vitest";
import { mount } from "@vue/test-utils";
import V2StatusBadge from "./V2StatusBadge.vue";

const mountBadge = (status: string) =>
mount(V2StatusBadge, {
props: { status },
global: {
stubs: { BaseBadge: { name: "BaseBadge", template: "<span><slot/>{{ text }}</span>", props: ["text", "color"] } },
},
});

describe("V2StatusBadge", () => {
it("passes the status through as the badge text", () => {
expect(mountBadge("published").text()).toContain("published");
});

it("maps known statuses to distinct token colors", () => {
const color = (s: string) => mountBadge(s).findComponent({ name: "BaseBadge" }).props("color");
expect(color("completed")).not.toBe(color("discarded"));
});
});
Loading